Skip to content

Commit

Permalink
Loosen the validation rule for URI authority
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 host
whose name is `beta_api.cloud.somewhere.com`.

Related: line#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 line#5814
  • Loading branch information
ikhoon committed Aug 2, 2024
1 parent 1145d45 commit 3ab60c9
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@

import static java.util.Objects.requireNonNull;

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

import com.google.common.net.HostAndPort;

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

public final class SchemeAndAuthority {
Expand Down Expand Up @@ -50,8 +53,21 @@ public static SchemeAndAuthority of(@Nullable String scheme, String authority) {

final String authorityWithoutUserInfo = removeUserInfo(authority);
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 (rawAuthority == null) {
throw new IllegalArgumentException("Invalid authority: " + authority);
}
rawAuthority = IDN.toASCII(rawAuthority, IDN.ALLOW_UNASSIGNED);

final boolean isIpv6 = rawAuthority.startsWith("[");
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);
}
Expand Down
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,15 @@
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: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
"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 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 3ab60c9

Please sign in to comment.