API council feedbacks for DnsResolver

To address the API review feedback provided by
the API council.

Bug: 129261432
Test: atest DnsResolverTest
Change-Id: I3de11c913682abf790850b45cd5d50ac28b3fc5c
This commit is contained in:
Luke Huang
2019-04-08 15:16:04 +08:00
parent 0855a3c457
commit 81cec00c55
3 changed files with 168 additions and 128 deletions

View File

@@ -93,6 +93,23 @@ public final class DnsResolver {
public static final int FLAG_NO_CACHE_STORE = 1 << 1; public static final int FLAG_NO_CACHE_STORE = 1 << 1;
public static final int FLAG_NO_CACHE_LOOKUP = 1 << 2; public static final int FLAG_NO_CACHE_LOOKUP = 1 << 2;
@IntDef(prefix = { "ERROR_" }, value = {
ERROR_PARSE,
ERROR_SYSTEM
})
@Retention(RetentionPolicy.SOURCE)
@interface DnsError {}
/**
* Indicates that there was an error parsing the response the query.
* The cause of this error is available via getCause() and is a ParseException.
*/
public static final int ERROR_PARSE = 0;
/**
* Indicates that there was an error sending the query.
* The cause of this error is available via getCause() and is an ErrnoException.
*/
public static final int ERROR_SYSTEM = 1;
private static final int NETID_UNSET = 0; private static final int NETID_UNSET = 0;
private static final DnsResolver sInstance = new DnsResolver(); private static final DnsResolver sInstance = new DnsResolver();
@@ -107,97 +124,57 @@ public final class DnsResolver {
private DnsResolver() {} private DnsResolver() {}
/** /**
* Answer parser for parsing raw answers * Base interface for answer callbacks
* *
* @param <T> The type of the parsed answer * @param <T> The type of the answer
*/ */
public interface AnswerParser<T> { public interface Callback<T> {
/**
* Creates a <T> answer by parsing the given raw answer.
*
* @param rawAnswer the raw answer to be parsed
* @return a parsed <T> answer
* @throws ParseException if parsing failed
*/
@NonNull T parse(@NonNull byte[] rawAnswer) throws ParseException;
}
/**
* Base class for answer callbacks
*
* @param <T> The type of the parsed answer
*/
public abstract static class AnswerCallback<T> {
/** @hide */
public final AnswerParser<T> parser;
public AnswerCallback(@NonNull AnswerParser<T> parser) {
this.parser = parser;
};
/** /**
* Success response to * Success response to
* {@link android.net.DnsResolver#query query()}. * {@link android.net.DnsResolver#query query()} or
* {@link android.net.DnsResolver#rawQuery rawQuery()}.
* *
* Invoked when the answer to a query was successfully parsed. * Invoked when the answer to a query was successfully parsed.
* *
* @param answer parsed answer to the query. * @param answer <T> answer to the query.
* @param rcode The response code in the DNS response.
* *
* {@see android.net.DnsResolver#query query()} * {@see android.net.DnsResolver#query query()}
*/ */
public abstract void onAnswer(@NonNull T answer); void onAnswer(@NonNull T answer, int rcode);
/** /**
* Error response to * Error response to
* {@link android.net.DnsResolver#query query()}. * {@link android.net.DnsResolver#query query()} or
* {@link android.net.DnsResolver#rawQuery rawQuery()}.
* *
* Invoked when there is no valid answer to * Invoked when there is no valid answer to
* {@link android.net.DnsResolver#query query()} * {@link android.net.DnsResolver#query query()}
* {@link android.net.DnsResolver#rawQuery rawQuery()}.
* *
* @param exception a {@link ParseException} object with additional * @param error a {@link DnsException} object with additional
* detail regarding the failure * detail regarding the failure
*/ */
public abstract void onParseException(@NonNull ParseException exception); void onError(@NonNull DnsException error);
/**
* Error response to
* {@link android.net.DnsResolver#query query()}.
*
* Invoked if an error happens when
* issuing the DNS query or receiving the result.
* {@link android.net.DnsResolver#query query()}
*
* @param exception an {@link ErrnoException} object with additional detail
* regarding the failure
*/
public abstract void onQueryException(@NonNull ErrnoException exception);
} }
/** /**
* Callback for receiving raw answers * Class to represent DNS error
*/ */
public abstract static class RawAnswerCallback extends AnswerCallback<byte[]> { public static class DnsException extends Exception {
public RawAnswerCallback() { /**
super(rawAnswer -> rawAnswer); * DNS error code as one of the ERROR_* constants
} */
} @DnsError public final int code;
/** DnsException(@DnsError int code, @Nullable Throwable cause) {
* Callback for receiving parsed {@link InetAddress} answers super(cause);
* this.code = code;
* Note that if the answer does not contain any IP addresses,
* onAnswer will be called with an empty list.
*/
public abstract static class InetAddressAnswerCallback
extends AnswerCallback<List<InetAddress>> {
public InetAddressAnswerCallback() {
super(rawAnswer -> new DnsAddressAnswer(rawAnswer).getAddresses());
} }
} }
/** /**
* Send a raw DNS query. * Send a raw DNS query.
* The answer will be provided asynchronously through the provided {@link AnswerCallback}. * The answer will be provided asynchronously through the provided {@link Callback}.
* *
* @param network {@link Network} specifying which network to query on. * @param network {@link Network} specifying which network to query on.
* {@code null} for query on default network. * {@code null} for query on default network.
@@ -206,13 +183,13 @@ public final class DnsResolver {
* @param executor The {@link Executor} that the callback should be executed on. * @param executor The {@link Executor} that the callback should be executed on.
* @param cancellationSignal used by the caller to signal if the query should be * @param cancellationSignal used by the caller to signal if the query should be
* cancelled. May be {@code null}. * cancelled. May be {@code null}.
* @param callback an {@link AnswerCallback} which will be called to notify the caller * @param callback a {@link Callback} which will be called to notify the caller
* of the result of dns query. * of the result of dns query.
*/ */
public <T> void query(@Nullable Network network, @NonNull byte[] query, @QueryFlag int flags, public void rawQuery(@Nullable Network network, @NonNull byte[] query, @QueryFlag int flags,
@NonNull @CallbackExecutor Executor executor, @NonNull @CallbackExecutor Executor executor,
@Nullable CancellationSignal cancellationSignal, @Nullable CancellationSignal cancellationSignal,
@NonNull AnswerCallback<T> callback) { @NonNull Callback<? super byte[]> callback) {
if (cancellationSignal != null && cancellationSignal.isCanceled()) { if (cancellationSignal != null && cancellationSignal.isCanceled()) {
return; return;
} }
@@ -222,9 +199,7 @@ public final class DnsResolver {
queryfd = resNetworkSend((network != null queryfd = resNetworkSend((network != null
? network.netId : NETID_UNSET), query, query.length, flags); ? network.netId : NETID_UNSET), query, query.length, flags);
} catch (ErrnoException e) { } catch (ErrnoException e) {
executor.execute(() -> { executor.execute(() -> callback.onError(new DnsException(ERROR_SYSTEM, e)));
callback.onQueryException(e);
});
return; return;
} }
@@ -237,7 +212,7 @@ public final class DnsResolver {
/** /**
* Send a DNS query with the specified name, class and query type. * Send a DNS query with the specified name, class and query type.
* The answer will be provided asynchronously through the provided {@link AnswerCallback}. * The answer will be provided asynchronously through the provided {@link Callback}.
* *
* @param network {@link Network} specifying which network to query on. * @param network {@link Network} specifying which network to query on.
* {@code null} for query on default network. * {@code null} for query on default network.
@@ -248,14 +223,14 @@ public final class DnsResolver {
* @param executor The {@link Executor} that the callback should be executed on. * @param executor The {@link Executor} that the callback should be executed on.
* @param cancellationSignal used by the caller to signal if the query should be * @param cancellationSignal used by the caller to signal if the query should be
* cancelled. May be {@code null}. * cancelled. May be {@code null}.
* @param callback an {@link AnswerCallback} which will be called to notify the caller * @param callback a {@link Callback} which will be called to notify the caller
* of the result of dns query. * of the result of dns query.
*/ */
public <T> void query(@Nullable Network network, @NonNull String domain, public void rawQuery(@Nullable Network network, @NonNull String domain,
@QueryClass int nsClass, @QueryType int nsType, @QueryFlag int flags, @QueryClass int nsClass, @QueryType int nsType, @QueryFlag int flags,
@NonNull @CallbackExecutor Executor executor, @NonNull @CallbackExecutor Executor executor,
@Nullable CancellationSignal cancellationSignal, @Nullable CancellationSignal cancellationSignal,
@NonNull AnswerCallback<T> callback) { @NonNull Callback<? super byte[]> callback) {
if (cancellationSignal != null && cancellationSignal.isCanceled()) { if (cancellationSignal != null && cancellationSignal.isCanceled()) {
return; return;
} }
@@ -265,9 +240,7 @@ public final class DnsResolver {
queryfd = resNetworkQuery((network != null queryfd = resNetworkQuery((network != null
? network.netId : NETID_UNSET), domain, nsClass, nsType, flags); ? network.netId : NETID_UNSET), domain, nsClass, nsType, flags);
} catch (ErrnoException e) { } catch (ErrnoException e) {
executor.execute(() -> { executor.execute(() -> callback.onError(new DnsException(ERROR_SYSTEM, e)));
callback.onQueryException(e);
});
return; return;
} }
synchronized (lock) { synchronized (lock) {
@@ -277,27 +250,28 @@ public final class DnsResolver {
} }
} }
private class InetAddressAnswerAccumulator extends InetAddressAnswerCallback { private class InetAddressAnswerAccumulator implements Callback<byte[]> {
private final List<InetAddress> mAllAnswers; private final List<InetAddress> mAllAnswers;
private ParseException mParseException; private int mRcode;
private ErrnoException mErrnoException; private DnsException mDnsException;
private final InetAddressAnswerCallback mUserCallback; private final Callback<? super List<InetAddress>> mUserCallback;
private final int mTargetAnswerCount; private final int mTargetAnswerCount;
private int mReceivedAnswerCount = 0; private int mReceivedAnswerCount = 0;
InetAddressAnswerAccumulator(int size, @NonNull InetAddressAnswerCallback callback) { InetAddressAnswerAccumulator(int size,
@NonNull Callback<? super List<InetAddress>> callback) {
mTargetAnswerCount = size; mTargetAnswerCount = size;
mAllAnswers = new ArrayList<>(); mAllAnswers = new ArrayList<>();
mUserCallback = callback; mUserCallback = callback;
} }
private boolean maybeReportException() { private boolean maybeReportError() {
if (mErrnoException != null) { if (mRcode != 0) {
mUserCallback.onQueryException(mErrnoException); mUserCallback.onAnswer(mAllAnswers, mRcode);
return true; return true;
} }
if (mParseException != null) { if (mDnsException != null) {
mUserCallback.onParseException(mParseException); mUserCallback.onError(mDnsException);
return true; return true;
} }
return false; return false;
@@ -305,34 +279,43 @@ public final class DnsResolver {
private void maybeReportAnswer() { private void maybeReportAnswer() {
if (++mReceivedAnswerCount != mTargetAnswerCount) return; if (++mReceivedAnswerCount != mTargetAnswerCount) return;
if (mAllAnswers.isEmpty() && maybeReportException()) return; if (mAllAnswers.isEmpty() && maybeReportError()) return;
// TODO: Do RFC6724 sort. // TODO: Do RFC6724 sort.
mUserCallback.onAnswer(mAllAnswers); mUserCallback.onAnswer(mAllAnswers, mRcode);
} }
@Override @Override
public void onAnswer(@NonNull List<InetAddress> answer) { public void onAnswer(@NonNull byte[] answer, int rcode) {
mAllAnswers.addAll(answer); // If at least one query succeeded, return an rcode of 0.
// Otherwise, arbitrarily return the first rcode received.
if (mReceivedAnswerCount == 0 || rcode == 0) {
mRcode = rcode;
}
try {
mAllAnswers.addAll(new DnsAddressAnswer(answer).getAddresses());
} catch (ParseException e) {
mDnsException = new DnsException(ERROR_PARSE, e);
}
maybeReportAnswer(); maybeReportAnswer();
} }
@Override @Override
public void onParseException(@NonNull ParseException e) { public void onError(@NonNull DnsException error) {
mParseException = e; mDnsException = error;
maybeReportAnswer();
}
@Override
public void onQueryException(@NonNull ErrnoException e) {
mErrnoException = e;
maybeReportAnswer(); maybeReportAnswer();
} }
} }
/** /**
* Send a DNS query with the specified name, get back a set of InetAddresses asynchronously. * Send a DNS query with the specified name on a network with both IPv4 and IPv6,
* The answer will be provided asynchronously through the provided * get back a set of InetAddresses asynchronously.
* {@link InetAddressAnswerCallback}. *
* This method will examine the connection ability on given network, and query IPv4
* and IPv6 if connection is available.
*
* If at least one query succeeded with valid answer, rcode will be 0
*
* The answer will be provided asynchronously through the provided {@link Callback}.
* *
* @param network {@link Network} specifying which network to query on. * @param network {@link Network} specifying which network to query on.
* {@code null} for query on default network. * {@code null} for query on default network.
@@ -341,13 +324,13 @@ public final class DnsResolver {
* @param executor The {@link Executor} that the callback should be executed on. * @param executor The {@link Executor} that the callback should be executed on.
* @param cancellationSignal used by the caller to signal if the query should be * @param cancellationSignal used by the caller to signal if the query should be
* cancelled. May be {@code null}. * cancelled. May be {@code null}.
* @param callback an {@link InetAddressAnswerCallback} which will be called to notify the * @param callback a {@link Callback} which will be called to notify the
* caller of the result of dns query. * caller of the result of dns query.
*/ */
public void query(@Nullable Network network, @NonNull String domain, @QueryFlag int flags, public void query(@Nullable Network network, @NonNull String domain, @QueryFlag int flags,
@NonNull @CallbackExecutor Executor executor, @NonNull @CallbackExecutor Executor executor,
@Nullable CancellationSignal cancellationSignal, @Nullable CancellationSignal cancellationSignal,
@NonNull InetAddressAnswerCallback callback) { @NonNull Callback<? super List<InetAddress>> callback) {
if (cancellationSignal != null && cancellationSignal.isCanceled()) { if (cancellationSignal != null && cancellationSignal.isCanceled()) {
return; return;
} }
@@ -365,9 +348,7 @@ public final class DnsResolver {
v6fd = resNetworkQuery((network != null v6fd = resNetworkQuery((network != null
? network.netId : NETID_UNSET), domain, CLASS_IN, TYPE_AAAA, flags); ? network.netId : NETID_UNSET), domain, CLASS_IN, TYPE_AAAA, flags);
} catch (ErrnoException e) { } catch (ErrnoException e) {
executor.execute(() -> { executor.execute(() -> callback.onError(new DnsException(ERROR_SYSTEM, e)));
callback.onQueryException(e);
});
return; return;
} }
queryCount++; queryCount++;
@@ -377,7 +358,9 @@ public final class DnsResolver {
// Avoiding gateways drop packets if queries are sent too close together // Avoiding gateways drop packets if queries are sent too close together
try { try {
Thread.sleep(SLEEP_TIME_MS); Thread.sleep(SLEEP_TIME_MS);
} catch (InterruptedException ex) { } } catch (InterruptedException ex) {
Thread.currentThread().interrupt();
}
if (queryIpv4) { if (queryIpv4) {
try { try {
@@ -385,9 +368,7 @@ public final class DnsResolver {
? network.netId : NETID_UNSET), domain, CLASS_IN, TYPE_A, flags); ? network.netId : NETID_UNSET), domain, CLASS_IN, TYPE_A, flags);
} catch (ErrnoException e) { } catch (ErrnoException e) {
if (queryIpv6) resNetworkCancel(v6fd); // Closes fd, marks it invalid. if (queryIpv6) resNetworkCancel(v6fd); // Closes fd, marks it invalid.
executor.execute(() -> { executor.execute(() -> callback.onError(new DnsException(ERROR_SYSTEM, e)));
callback.onQueryException(e);
});
return; return;
} }
queryCount++; queryCount++;
@@ -413,34 +394,89 @@ public final class DnsResolver {
} }
} }
private <T> void registerFDListener(@NonNull Executor executor, /**
@NonNull FileDescriptor queryfd, @NonNull AnswerCallback<T> answerCallback, * Send a DNS query with the specified name and query type, get back a set of
* InetAddresses asynchronously.
*
* The answer will be provided asynchronously through the provided {@link Callback}.
*
* @param network {@link Network} specifying which network to query on.
* {@code null} for query on default network.
* @param domain domain name to query
* @param nsType dns resource record (RR) type as one of the TYPE_* constants
* @param flags flags as a combination of the FLAGS_* constants
* @param executor The {@link Executor} that the callback should be executed on.
* @param cancellationSignal used by the caller to signal if the query should be
* cancelled. May be {@code null}.
* @param callback a {@link Callback} which will be called to notify the caller
* of the result of dns query.
*/
public void query(@Nullable Network network, @NonNull String domain,
@QueryType int nsType, @QueryFlag int flags,
@NonNull @CallbackExecutor Executor executor,
@Nullable CancellationSignal cancellationSignal,
@NonNull Callback<? super List<InetAddress>> callback) {
if (cancellationSignal != null && cancellationSignal.isCanceled()) {
return;
}
final Object lock = new Object();
final FileDescriptor queryfd;
try {
queryfd = resNetworkQuery((network != null
? network.netId : NETID_UNSET), domain, CLASS_IN, nsType, flags);
} catch (ErrnoException e) {
executor.execute(() -> callback.onError(new DnsException(ERROR_SYSTEM, e)));
return;
}
final InetAddressAnswerAccumulator accumulator =
new InetAddressAnswerAccumulator(1, callback);
synchronized (lock) {
registerFDListener(executor, queryfd, accumulator, cancellationSignal, lock);
if (cancellationSignal == null) return;
addCancellationSignal(cancellationSignal, queryfd, lock);
}
}
/**
* Class to retrieve DNS response
*
* @hide
*/
public static final class DnsResponse {
public final @NonNull byte[] answerbuf;
public final int rcode;
public DnsResponse(@NonNull byte[] answerbuf, int rcode) {
this.answerbuf = answerbuf;
this.rcode = rcode;
}
}
private void registerFDListener(@NonNull Executor executor,
@NonNull FileDescriptor queryfd, @NonNull Callback<? super byte[]> answerCallback,
@Nullable CancellationSignal cancellationSignal, @NonNull Object lock) { @Nullable CancellationSignal cancellationSignal, @NonNull Object lock) {
Looper.getMainLooper().getQueue().addOnFileDescriptorEventListener( Looper.getMainLooper().getQueue().addOnFileDescriptorEventListener(
queryfd, queryfd,
FD_EVENTS, FD_EVENTS,
(fd, events) -> { (fd, events) -> {
executor.execute(() -> { executor.execute(() -> {
DnsResponse resp = null;
ErrnoException exception = null;
synchronized (lock) { synchronized (lock) {
if (cancellationSignal != null && cancellationSignal.isCanceled()) { if (cancellationSignal != null && cancellationSignal.isCanceled()) {
return; return;
} }
byte[] answerbuf = null;
try { try {
answerbuf = resNetworkResult(fd); // Closes fd, marks it invalid. resp = resNetworkResult(fd); // Closes fd, marks it invalid.
} catch (ErrnoException e) { } catch (ErrnoException e) {
Log.e(TAG, "resNetworkResult:" + e.toString()); Log.e(TAG, "resNetworkResult:" + e.toString());
answerCallback.onQueryException(e); exception = e;
return;
}
try {
answerCallback.onAnswer(
answerCallback.parser.parse(answerbuf));
} catch (ParseException e) {
answerCallback.onParseException(e);
} }
} }
if (exception != null) {
answerCallback.onError(new DnsException(ERROR_SYSTEM, exception));
return;
}
answerCallback.onAnswer(resp.answerbuf, resp.rcode);
}); });
// Unregister this fd listener // Unregister this fd listener
return 0; return 0;

View File

@@ -140,9 +140,10 @@ public class NetworkUtils {
/** /**
* DNS resolver series jni method. * DNS resolver series jni method.
* Read a result for the query associated with the {@code fd}. * Read a result for the query associated with the {@code fd}.
* @return a byte array containing blob answer * @return DnsResponse containing blob answer and rcode
*/ */
public static native byte[] resNetworkResult(FileDescriptor fd) throws ErrnoException; public static native DnsResolver.DnsResponse resNetworkResult(FileDescriptor fd)
throws ErrnoException;
/** /**
* DNS resolver series jni method. * DNS resolver series jni method.

View File

@@ -270,7 +270,7 @@ static jobject android_net_utils_resNetworkSend(JNIEnv *env, jobject thiz, jint
return jniCreateFileDescriptor(env, fd); return jniCreateFileDescriptor(env, fd);
} }
static jbyteArray android_net_utils_resNetworkResult(JNIEnv *env, jobject thiz, jobject javaFd) { static jobject android_net_utils_resNetworkResult(JNIEnv *env, jobject thiz, jobject javaFd) {
int fd = jniGetFDFromFileDescriptor(env, javaFd); int fd = jniGetFDFromFileDescriptor(env, javaFd);
int rcode; int rcode;
std::vector<uint8_t> buf(MAXPACKETSIZE, 0); std::vector<uint8_t> buf(MAXPACKETSIZE, 0);
@@ -291,7 +291,10 @@ static jbyteArray android_net_utils_resNetworkResult(JNIEnv *env, jobject thiz,
reinterpret_cast<jbyte*>(buf.data())); reinterpret_cast<jbyte*>(buf.data()));
} }
return answer; jclass class_DnsResponse = env->FindClass("android/net/DnsResolver$DnsResponse");
jmethodID ctor = env->GetMethodID(class_DnsResponse, "<init>", "([BI)V");
return env->NewObject(class_DnsResponse, ctor, answer, rcode);
} }
static void android_net_utils_resNetworkCancel(JNIEnv *env, jobject thiz, jobject javaFd) { static void android_net_utils_resNetworkCancel(JNIEnv *env, jobject thiz, jobject javaFd) {
@@ -354,7 +357,7 @@ static const JNINativeMethod gNetworkUtilMethods[] = {
{ "setupRaSocket", "(Ljava/io/FileDescriptor;I)V", (void*) android_net_utils_setupRaSocket }, { "setupRaSocket", "(Ljava/io/FileDescriptor;I)V", (void*) android_net_utils_setupRaSocket },
{ "resNetworkSend", "(I[BII)Ljava/io/FileDescriptor;", (void*) android_net_utils_resNetworkSend }, { "resNetworkSend", "(I[BII)Ljava/io/FileDescriptor;", (void*) android_net_utils_resNetworkSend },
{ "resNetworkQuery", "(ILjava/lang/String;III)Ljava/io/FileDescriptor;", (void*) android_net_utils_resNetworkQuery }, { "resNetworkQuery", "(ILjava/lang/String;III)Ljava/io/FileDescriptor;", (void*) android_net_utils_resNetworkQuery },
{ "resNetworkResult", "(Ljava/io/FileDescriptor;)[B", (void*) android_net_utils_resNetworkResult }, { "resNetworkResult", "(Ljava/io/FileDescriptor;)Landroid/net/DnsResolver$DnsResponse;", (void*) android_net_utils_resNetworkResult },
{ "resNetworkCancel", "(Ljava/io/FileDescriptor;)V", (void*) android_net_utils_resNetworkCancel }, { "resNetworkCancel", "(Ljava/io/FileDescriptor;)V", (void*) android_net_utils_resNetworkCancel },
}; };