Skip to content

fix DoH error when using ip address as hostname #7073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
swankjesse merged 10 commits into from
Feb 16, 2022
Merged

fix DoH error when using ip address as hostname #7073

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f74fa03
fix DoH error when using ip address as hostname
TaoZang Feb 11, 2022
37be7f2
lookup hostname first and try to parse as IP address only when lookup…
TaoZang Feb 11, 2022
f26f1cd
add unit test in DnsOverHttpsTest for the case that Doing DNS lookup …
TaoZang Feb 12, 2022
1adcb89
use simpler ipAddress check instead of complex support functions
TaoZang Feb 14, 2022
2cf4de1
add test case for some special ipv6 addresses
TaoZang Feb 15, 2022
47f50f5
use existing ipAddress check functions instead of adding a new simila…
TaoZang Feb 15, 2022
d0fb591
avoid to do dns lookup when the hostname is already an IP address
TaoZang Feb 16, 2022
8158e31
code style update
TaoZang Feb 16, 2022
d9bb9fd
add test case
TaoZang Feb 16, 2022
9614b50
use loopback address instead of localhost
TaoZang Feb 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import okhttp3.Protocol
import okhttp3.Request
import okhttp3.RequestBody.Companion.toRequestBody
import okhttp3.Response
import okhttp3.internal.canParseAsIpAddress
import okhttp3.internal.platform.Platform
import okhttp3.internal.publicsuffix.PublicSuffixDatabase

Expand All @@ -53,6 +54,7 @@ class DnsOverHttps internal constructor(
@get:JvmName("resolvePrivateAddresses") val resolvePrivateAddresses: Boolean,
@get:JvmName("resolvePublicAddresses") val resolvePublicAddresses: Boolean
) : Dns {

Copy link
Collaborator

@yschimke yschimke Feb 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can you revert this line.

@Throws(UnknownHostException::class)
override fun lookup(hostname: String): List<InetAddress> {
if (!resolvePrivateAddresses || !resolvePublicAddresses) {
Expand All @@ -67,7 +69,24 @@ class DnsOverHttps internal constructor(
}
}

return lookupHttps(hostname)
return try {
lookupHttps(hostname)
} catch (bestFailure: UnknownHostException) {
val address = if (hostname.startsWith("[") && hostname.endsWith("]")) {
hostname.substring(1, hostname.length - 1)
} else {
hostname
}
if (address.canParseAsIpAddress()) {
Copy link
Collaborator

@yschimke yschimke Feb 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find, I forgot this was a thing. Can you invert this now?

Also if that function doesn't worry about [], we can probably skip that here also.

See

okhttp/okhttp/src/jvmTest/java/okhttp3/HttpUrlTest.java

Line 514 in 3ad1912

assertThat(parse("http://[::1]/").host()).isEqualTo("::1");

Which confirms the HttpUrl strips the [].

Copy link
Contributor Author

@TaoZang TaoZang Feb 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean that the HttpUrl will handle the square braces before calling dns lookup, so we don't need to check and strips the [] here and just remove the test case for hostname "[::1]"?

try {
InetAddress.getAllByName(address).toList()
} catch (e: UnknownHostException) {
throw bestFailure
}
} else {
throw bestFailure
}
}
}

@Throws(UnknownHostException::class)
Expand Down
14 changes: 14 additions & 0 deletions okhttp-dnsoverhttps/src/test/java/okhttp3/dnsoverhttps/DnsOverHttpsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,20 @@ public class DnsOverHttpsTest {
"/lookup?ct&dns=AAABAAABAAAAAAAABmdvb2dsZQNjb20AAAEAAQ");
}

@Test public void getIPAddressHost() throws Exception {
dns = buildLocalhost(bootstrapClient, true);

String hostIPv4 = "127.0.0.1";
assertThat(dns.lookup(hostIPv4)).contains(address(hostIPv4));

String hostIPv6 = "2a03:2880:f029:11:face:b00c:0:2";
assertThat(dns.lookup(hostIPv6)).contains(address(hostIPv6));

String hostIPv6Loopback = "::1";
assertThat(dns.lookup(hostIPv6Loopback)).contains(address(hostIPv6Loopback));
assertThat(dns.lookup("[::1]")).contains(address(hostIPv6Loopback));
}

@Test public void getIpv6() throws Exception {
server.enqueue(dnsResponse(
"0000818000010003000000000567726170680866616365626f6f6b03636f6d0000010001c00c00050001"
Expand Down