Skip to content

Commit

Permalink
Loosen the validation rule for URI authority (#5854)
Browse files Browse the repository at this point in the history
Motivation:

The authority part in a URI was validated by
`URI.parseServerAuthority()` which only allows alphanumeric characters,
`.` and `-`.

https://github.com/openjdk/jdk/blob/dc35f3e8a84c8f622a4cabb8aee0f96de2e2ea30/src/java.base/share/classes/java/net/URI.java#L3513-L3515

As a result, if underscore (`_`) is set in an authority,
`URISyntaxException` is raised. We think the rule is too strict because
a request can also be sent to an instance when CSLB is used. `_` is a
valid character in a DNS record. Users may want to send a request to a
host whose name is `beta_api.cloud.instance-123.somewhere.com`.

Related: #5814

Modifications:

- Remove the usage of `URI.parseServerAuthority()` in
`SchemeAndAuthority`
- Parse a hostname and a port from a raw authority.

Result:

- Validation is relaxed to permit underscores (_) in URI's authority.
- Closes #5814
  • Loading branch information
ikhoon authored Aug 8, 2024
1 parent cdfad4f commit fde4260
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.linecorp.armeria.internal.common;

import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.findAuthority;
import static com.linecorp.armeria.internal.common.util.BitSetUtil.toBitSet;
import static io.netty.util.internal.StringUtil.decodeHexNibble;
import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -119,14 +120,6 @@ public final class DefaultRequestTarget implements RequestTarget {
*/
private static final BitSet FRAGMENT_MUST_PRESERVE_ENCODING = PATH_MUST_PRESERVE_ENCODING;

private static BitSet toBitSet(String chars) {
final BitSet bitSet = new BitSet();
for (int i = 0; i < chars.length(); i++) {
bitSet.set(chars.charAt(i));
}
return bitSet;
}

private enum ComponentType {
CLIENT_PATH(PATH_ALLOWED, PATH_ALLOWED_IF_ENCODED, PATH_MUST_PRESERVE_ENCODING),
SERVER_PATH(PATH_ALLOWED, PATH_ALLOWED_IF_ENCODED, PATH_MUST_PRESERVE_ENCODING),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,23 @@

package com.linecorp.armeria.internal.common;

import static com.linecorp.armeria.internal.common.util.BitSetUtil.toBitSet;
import static java.util.Objects.requireNonNull;

import java.net.IDN;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.BitSet;

import com.google.common.base.Strings;
import com.google.common.net.HostAndPort;

import com.linecorp.armeria.common.annotation.Nullable;

public final class SchemeAndAuthority {

private static final BitSet RESERVED_CHARS = toBitSet("/?#");

/**
* Attempts to parse this URI's authority component and return {@link SchemeAndAuthority}.
* The authority part may have one of the following formats (The userinfo part will be ignored.):
Expand All @@ -49,14 +58,53 @@ public static SchemeAndAuthority of(@Nullable String scheme, String authority) {
}

final String authorityWithoutUserInfo = removeUserInfo(authority);

for (int i = 0; i < authorityWithoutUserInfo.length(); i++) {
final char c = authorityWithoutUserInfo.charAt(i);
if (c < 0x80 && RESERVED_CHARS.get(c)) {
throw new IllegalArgumentException("The authority contains reserved character: " +
authority + " (" + c + ')');
}
}

try {
final URI uri = new URI(null, authorityWithoutUserInfo, null, null, null).parseServerAuthority();
return new SchemeAndAuthority(scheme, uri.getRawAuthority(), uri.getHost(), uri.getPort());
final URI uri = new URI(null, authorityWithoutUserInfo, null, null, null);
String rawAuthority = uri.getRawAuthority();
if (Strings.isNullOrEmpty(rawAuthority)) {
throw new IllegalArgumentException("Invalid authority: " + authority);
}
rawAuthority = IDN.toASCII(rawAuthority, IDN.ALLOW_UNASSIGNED);

final boolean isIpv6 = rawAuthority.charAt(0) == '[';
if (isIpv6) {
rawAuthority = removeIpv6ScopeId(rawAuthority);
}

final HostAndPort hostAndPort = HostAndPort.fromString(rawAuthority);
String host = hostAndPort.getHost();
if (isIpv6) {
host = '[' + host + ']';
}
final int port = hostAndPort.getPortOrDefault(-1);

return new SchemeAndAuthority(scheme, rawAuthority, host, port);
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
}

private static String removeIpv6ScopeId(String ipV6Authority) {
final int percentPos = ipV6Authority.indexOf('%');
if (percentPos == -1) {
return ipV6Authority;
}

final int endBracket = ipV6Authority.indexOf(']');
// An incomplete IPv6 address is rejected by URI constructor.
assert endBracket > 0 : "endBracket: " + endBracket + " (expected: > 0)";
return ipV6Authority.substring(0, percentPos) + ipV6Authority.substring(endBracket);
}

private static String removeUserInfo(String authority) {
final int indexOfDelimiter = authority.lastIndexOf('@');
if (indexOfDelimiter == -1) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright 2024 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

package com.linecorp.armeria.internal.common.util;

import java.util.BitSet;

public final class BitSetUtil {

public static BitSet toBitSet(String chars) {
final BitSet bitSet = new BitSet();
for (int i = 0; i < chars.length(); i++) {
bitSet.set(chars.charAt(i));
}
return bitSet;
}

private BitSetUtil() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* Copyright 2024 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

package com.linecorp.armeria.client;

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import com.linecorp.armeria.common.AggregatedHttpResponse;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.HttpStatus;
import com.linecorp.armeria.common.logging.RequestLog;
import com.linecorp.armeria.internal.testing.MockAddressResolverGroup;
import com.linecorp.armeria.server.ServerBuilder;
import com.linecorp.armeria.testing.junit5.server.ServerExtension;

class UnderscoredAuthorityTest {

@RegisterExtension
static ServerExtension server = new ServerExtension() {
@Override
protected void configure(ServerBuilder sb) {
sb.service("/", (ctx, req) -> HttpResponse.of("Hello, world!"));
}
};

@Test
void shouldAllowUnderscoreInAuthority() {
try (ClientFactory factory = ClientFactory.builder()
.addressResolverGroupFactory(unused -> {
return MockAddressResolverGroup.localhost();
})
.build()) {
final BlockingWebClient client =
WebClient.builder("http://my_key.z1.armeria.dev:" + server.httpPort())
.factory(factory)
.build()
.blocking();

try (ClientRequestContextCaptor captor = Clients.newContextCaptor()) {
final AggregatedHttpResponse response = client.get("/");
final ClientRequestContext ctx = captor.get();
assertThat(response.status()).isEqualTo(HttpStatus.OK);
assertThat(response.contentUtf8()).isEqualTo("Hello, world!");
final RequestLog log = ctx.log().whenComplete().join();

assertThat(log.requestHeaders().authority())
.isEqualTo("my_key.z1.armeria.dev:" + server.httpPort());
}
}
}

@Test
void shouldAllowUnderscoreInEndpoint() {
final Endpoint endpoint = Endpoint.parse("my_key.z1.armeria.dev:" + server.httpPort());
assertThat(endpoint.authority()).isEqualTo("my_key.z1.armeria.dev:" + server.httpPort());
assertThat(endpoint.host()).isEqualTo("my_key.z1.armeria.dev");
assertThat(endpoint.port()).isEqualTo(server.httpPort());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,20 @@
class SchemeAndAuthorityTest {
@ParameterizedTest
@CsvSource({
"0.0.0.0:80, 0.0.0.0:80, 0.0.0.0, 80", // IPv4
"[::1]:8080, [::1]:8080, [::1], 8080", // IPv6
"unix%3Afoo.sock, unix%3Afoo.sock, unix%3Afoo.sock, -1", // Domain socket
"foo.bar, foo.bar, foo.bar, -1", // Only host
"foo:, foo:, foo, -1", // Empty port
"bar:80, bar:80, bar, 80", // Host and port
"foo@bar:80, bar:80, bar, 80", // Userinfo and host and port
"0.0.0.0, 0.0.0.0, 0.0.0.0, -1", // IPv4
"0.0.0.0:, 0.0.0.0:, 0.0.0.0, -1", // IPv4 with empty port
"0.0.0.0:80, 0.0.0.0:80, 0.0.0.0, 80", // IPv4 with port
"[::1], [::1], [::1], -1", // IPv6
"[::1]:, [::1]:, [::1], -1", // IPv6 with empty port
"[::1]:8080, [::1]:8080, [::1], 8080", // IPv6 with port
"[::1%eth0]:8080, [::1]:8080, [::1], 8080", // IPv6 with port and scope
"unix%3Afoo.sock, unix%3Afoo.sock, unix%3Afoo.sock, -1", // Domain socket
"foo.bar, foo.bar, foo.bar, -1", // Only host
"foo:, foo:, foo, -1", // Empty port
"bar:80, bar:80, bar, 80", // Host and port
"foo@bar:80, bar:80, bar, 80", // Userinfo and host and port
"foo_bar:80, foo_bar:80, foo_bar, 80", // Underscore in host
"한글.com:80, xn--bj0bj06e.com:80, xn--bj0bj06e.com, 80", // IDN
})
void fromAuthority(String authority, String expectedAuthority, String expectedHost,
int expectedPort) {
Expand All @@ -45,8 +52,16 @@ void fromAuthority(String authority, String expectedAuthority, String expectedHo

@ParameterizedTest
@CsvSource({
"foo:bar", "http://foo:80", "foo/bar", "foo?bar=1", "foo#bar",
"[192.168.0.1]", "[::1", "::1]", "[::1]%eth0", "unix:foo.sock"
"foo:bar", // Invalid port
"http://foo:80", // Scheme included
"foo/bar", // Authority with path
"foo?bar=1", // Authority with query
"foo#bar", // Authority with fragment
"[192.168.0.1]", // Bracketed IPv4
"[::1", "::1]", // Incomplete IPv6
"[::1%eth0", "::1%eth0]", // Incomplete IPv6 with scope
"[::1]%eth0", // Invalid IPv6 scope
"unix:foo.sock" // Invalid domain socket
})
void fromBadAuthority(String badAuthority) {
assertThatThrownBy(() -> SchemeAndAuthority.of(null, badAuthority))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* under the License.
*/

package com.linecorp.armeria;
package com.linecorp.armeria.server;

import static org.assertj.core.api.Assertions.assertThat;

Expand All @@ -28,11 +28,6 @@
import com.linecorp.armeria.common.HttpStatus;
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.metric.MeterIdPrefixFunction;
import com.linecorp.armeria.server.HttpService;
import com.linecorp.armeria.server.Server;
import com.linecorp.armeria.server.ServerBuilder;
import com.linecorp.armeria.server.ServiceRequestContext;
import com.linecorp.armeria.server.SimpleDecoratingHttpService;
import com.linecorp.armeria.server.cors.CorsService;
import com.linecorp.armeria.server.encoding.EncodingService;
import com.linecorp.armeria.server.logging.LoggingService;
Expand Down

0 comments on commit fde4260

Please sign in to comment.