Skip to content

Commit

Permalink
Reuse VirtualHostBuilder on the same hostnamePattern (#5418)
Browse files Browse the repository at this point in the history
Motivation:

I registered each virtual host with the same hostname, the registered
routes cannot be not found. Because `VirtualHostBuilder` creates new
builder even if hostname is the same. However, it does reuse them on the
same port.
So, I think it's better to reuse them on the same hostname.

Modifications:

- `ServerBuilder.virtualHost(hostnamePattern)` and
`ServerBuilder.virtualHost(defaultHostname, hostnamePattern)` checks
whether there is `builders` with the same hostname. If there is, returns
this builder.

Result:

- Reuse `VirtualHostBuilder` on the same hostname. 
- Even if you register each one on a virtual host with the same
hostname, they will be merged.

<!--
Visit this URL to learn more about how to write a pull request
description:

https://armeria.dev/community/developer-guide#how-to-write-pull-request-description
-->

---------

Co-authored-by: jrhee17 <guins_j@guins.org>
Co-authored-by: Ikhun Um <ih.pert@gmail.com>
Co-authored-by: minwoox <songmw725@gmail.com>
  • Loading branch information
4 people committed Apr 4, 2024
1 parent 66cfaa4 commit dad02d6
Show file tree
Hide file tree
Showing 7 changed files with 214 additions and 42 deletions.
39 changes: 37 additions & 2 deletions core/src/main/java/com/linecorp/armeria/server/ServerBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import static com.linecorp.armeria.server.DefaultServerConfig.validateIdleTimeoutMillis;
import static com.linecorp.armeria.server.DefaultServerConfig.validateMaxNumConnections;
import static com.linecorp.armeria.server.DefaultServerConfig.validateNonNegative;
import static com.linecorp.armeria.server.VirtualHost.normalizeHostnamePattern;
import static com.linecorp.armeria.server.VirtualHost.validateHostnamePattern;
import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND;
import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_FRAME_SIZE_UPPER_BOUND;
import static java.util.Objects.requireNonNull;
Expand Down Expand Up @@ -1549,23 +1551,37 @@ public VirtualHostBuilder defaultVirtualHost() {

/**
* Configures a {@link VirtualHost} with the {@code customizer}.
*
* @deprecated Use {@link #withVirtualHost(String, Consumer)} instead.
*/
@Deprecated
public ServerBuilder withVirtualHost(Consumer<? super VirtualHostBuilder> customizer) {
final VirtualHostBuilder virtualHostBuilder = new VirtualHostBuilder(this, false);
customizer.accept(virtualHostBuilder);
virtualHostBuilders.add(virtualHostBuilder);
return this;
}

/**
* Configures a {@link VirtualHost} with the {@code customizer}.
*/
public ServerBuilder withVirtualHost(String hostnamePattern,
Consumer<? super VirtualHostBuilder> customizer) {
final VirtualHostBuilder virtualHostBuilder = findOrCreateVirtualHostBuilder(hostnamePattern);
requireNonNull(customizer, "customizer");
customizer.accept(virtualHostBuilder);
virtualHostBuilders.add(virtualHostBuilder);
return this;
}

/**
* Adds the <a href="https://en.wikipedia.org/wiki/Virtual_hosting#Name-based">name-based virtual host</a>.
*
* @param hostnamePattern virtual host name regular expression
* @return {@link VirtualHostBuilder} for building the virtual host
*/
public VirtualHostBuilder virtualHost(String hostnamePattern) {
final VirtualHostBuilder virtualHostBuilder =
new VirtualHostBuilder(this, false).hostnamePattern(hostnamePattern);
final VirtualHostBuilder virtualHostBuilder = findOrCreateVirtualHostBuilder(hostnamePattern);
virtualHostBuilders.add(virtualHostBuilder);
return virtualHostBuilder;
}
Expand All @@ -1576,7 +1592,10 @@ public VirtualHostBuilder virtualHost(String hostnamePattern) {
* @param defaultHostname default hostname of this virtual host
* @param hostnamePattern virtual host name regular expression
* @return {@link VirtualHostBuilder} for building the virtual host
*
* @deprecated prefer {@link #virtualHost(String)} with {@link VirtualHostBuilder#defaultHostname(String)}.
*/
@Deprecated
public VirtualHostBuilder virtualHost(String defaultHostname, String hostnamePattern) {
final VirtualHostBuilder virtualHostBuilder = new VirtualHostBuilder(this, false)
.defaultHostname(defaultHostname)
Expand Down Expand Up @@ -1613,6 +1632,22 @@ public VirtualHostBuilder virtualHost(int port) {
return virtualHostBuilder;
}

private VirtualHostBuilder findOrCreateVirtualHostBuilder(String hostnamePattern) {
requireNonNull(hostnamePattern, "hostnamePattern");
final HostAndPort hostAndPort = HostAndPort.fromString(hostnamePattern);
validateHostnamePattern(hostAndPort.getHost());

final String normalizedHostnamePattern = normalizeHostnamePattern(hostAndPort.getHost());
final int port = hostAndPort.getPortOrDefault(-1);
for (VirtualHostBuilder virtualHostBuilder : virtualHostBuilders) {
if (!virtualHostBuilder.defaultVirtualHost() &&
virtualHostBuilder.equalsHostnamePattern(normalizedHostnamePattern, port)) {
return virtualHostBuilder;
}
}
return new VirtualHostBuilder(this, false).hostnamePattern(normalizedHostnamePattern, port);
}

/**
* Decorates all {@link HttpService}s with the specified {@code decorator}.
* The specified decorator(s) is/are executed in reverse order of the insertion.
Expand Down
16 changes: 16 additions & 0 deletions core/src/main/java/com/linecorp/armeria/server/VirtualHost.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.linecorp.armeria.server;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Objects.requireNonNull;
Expand Down Expand Up @@ -205,6 +206,21 @@ static String normalizeHostnamePattern(String hostnamePattern) {
return Ascii.toLowerCase(hostnamePattern);
}

static void validateHostnamePattern(String hostnamePattern) {
final boolean validHostnamePattern;
if (hostnamePattern.charAt(0) == '*') {
validHostnamePattern =
hostnamePattern.length() >= 3 &&
hostnamePattern.charAt(1) == '.' &&
HOSTNAME_WITH_NO_PORT_PATTERN.matcher(hostnamePattern.substring(2)).matches();
} else {
validHostnamePattern = HOSTNAME_WITH_NO_PORT_PATTERN.matcher(hostnamePattern).matches();
}

checkArgument(validHostnamePattern,
"hostnamePattern: %s (expected: *.<hostname> or <hostname>)", hostnamePattern);
}

/**
* Ensure that 'hostnamePattern' matches 'defaultHostname'.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@
import static com.linecorp.armeria.server.ServerSslContextUtil.validateSslContext;
import static com.linecorp.armeria.server.ServiceConfig.validateMaxRequestLength;
import static com.linecorp.armeria.server.ServiceConfig.validateRequestTimeoutMillis;
import static com.linecorp.armeria.server.VirtualHost.HOSTNAME_WITH_NO_PORT_PATTERN;
import static com.linecorp.armeria.server.VirtualHost.ensureHostnamePatternMatchesDefaultHostname;
import static com.linecorp.armeria.server.VirtualHost.normalizeDefaultHostname;
import static com.linecorp.armeria.server.VirtualHost.normalizeHostnamePattern;
import static com.linecorp.armeria.server.VirtualHost.validateHostnamePattern;
import static io.netty.handler.codec.http2.Http2Headers.PseudoHeaderName.isPseudoHeader;
import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -232,7 +232,11 @@ public VirtualHostBuilder baseContextPath(String baseContextPath) {
* will be bound to the {@code 8080} port. Otherwise, the virtual host will allow all active ports.
*
* @throws UnsupportedOperationException if this is the default {@link VirtualHostBuilder}
*
* @deprecated prefer specifying the hostnamePattern using {@link ServerBuilder#virtualHost(String)}
* or {@link ServerBuilder#withVirtualHost(String, Consumer)}
*/
@Deprecated
public VirtualHostBuilder hostnamePattern(String hostnamePattern) {
if (defaultVirtualHost) {
throw new UnsupportedOperationException(
Expand All @@ -248,23 +252,25 @@ public VirtualHostBuilder hostnamePattern(String hostnamePattern) {
hostnamePattern = hostAndPort.getHost();
}

final boolean validHostnamePattern;
if (hostnamePattern.charAt(0) == '*') {
validHostnamePattern =
hostnamePattern.length() >= 3 &&
hostnamePattern.charAt(1) == '.' &&
HOSTNAME_WITH_NO_PORT_PATTERN.matcher(hostnamePattern.substring(2)).matches();
} else {
validHostnamePattern = HOSTNAME_WITH_NO_PORT_PATTERN.matcher(hostnamePattern).matches();
}

checkArgument(validHostnamePattern,
"hostnamePattern: %s (expected: *.<hostname> or <hostname>)", hostnamePattern);
validateHostnamePattern(hostnamePattern);

this.hostnamePattern = normalizeHostnamePattern(hostnamePattern);
return this;
}

VirtualHostBuilder hostnamePattern(String hostnamePattern, int port) {
if (defaultVirtualHost) {
throw new UnsupportedOperationException(
"Cannot set hostnamePattern for the default virtual host builder");
}

this.hostnamePattern = hostnamePattern;
if (port >= 1 && port <= 65535) {
this.port = port;
}
return this;
}

@Override
public VirtualHostBuilder tls(File keyCertChainFile, File keyFile) {
return (VirtualHostBuilder) TlsSetters.super.tls(keyCertChainFile, keyFile);
Expand Down Expand Up @@ -1525,6 +1531,15 @@ private SelfSignedCertificate selfSignedCertificate() throws CertificateExceptio
return selfSignedCertificate;
}

boolean equalsHostnamePattern(String validHostnamePattern, int port) {
checkArgument(!validHostnamePattern.isEmpty(), "hostnamePattern is empty.");

if (this.port != port) {
return false;
}
return validHostnamePattern.equals(hostnamePattern);
}

int port() {
return port;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* 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.server;

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

import java.net.ServerSocket;

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

import com.linecorp.armeria.client.ClientFactory;
import com.linecorp.armeria.client.WebClient;
import com.linecorp.armeria.common.AggregatedHttpResponse;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.internal.testing.MockAddressResolverGroup;
import com.linecorp.armeria.testing.junit5.server.ServerExtension;

class HostnameBasedVirtualHostTest {

private static int fooHostPort;

@RegisterExtension
static ServerExtension serverWithPortMapping = new ServerExtension() {
@Override
protected void configure(ServerBuilder sb) throws Exception {

try (ServerSocket ss = new ServerSocket(0)) {
fooHostPort = ss.getLocalPort();
}

sb.http(fooHostPort)
.virtualHost("foo.com:" + fooHostPort)
.service("/foo", (ctx, req) -> HttpResponse.of("foo with port"))
.and()
.virtualHost("foo.com:" + fooHostPort)
.service("/bar", (ctx, req) -> HttpResponse.of("bar with port"))
.and()
.virtualHost("foo.bar.com")
.service("/foo-bar", (ctx, req) -> HttpResponse.of("foo bar"))
.and()
.build();
}
};

@Test
void testHostnamePattern() {
try (ClientFactory factory = ClientFactory.builder()
.addressResolverGroupFactory(
unused -> MockAddressResolverGroup.localhost())
.build()) {

final WebClient client = WebClient.builder("http://foo.com:" + fooHostPort)
.factory(factory)
.build();
AggregatedHttpResponse response = client.get("/foo").aggregate().join();
assertThat(response.contentUtf8()).isEqualTo("foo with port");

response = client.get("/bar").aggregate().join();
assertThat(response.contentUtf8()).isEqualTo("bar with port");
}
}

@Test
void shouldReturnSameInstanceForHostnameBasedVirtualHost() {
final ServerBuilder serverBuilder = Server.builder();
final VirtualHostBuilder virtualHost1 = serverBuilder.virtualHost("foo.com");
final VirtualHostBuilder virtualHost2 = serverBuilder.virtualHost("foo.com");
assertThat(virtualHost1).isSameAs(virtualHost2);
final VirtualHostBuilder virtualHost3 = serverBuilder.virtualHost("foo.com:18080");
assertThat(virtualHost2).isNotSameAs(virtualHost3);
final VirtualHostBuilder virtualHost4 = serverBuilder.virtualHost("bar.com");
assertThat(virtualHost2).isNotSameAs(virtualHost4);
}

@Test
void shouldReturnSameInstanceForHostnameBasedVirtualHostWithPort() {
final ServerBuilder serverBuilder = Server.builder();
final VirtualHostBuilder virtualHost1 = serverBuilder.virtualHost("foo.com:18080");
final VirtualHostBuilder virtualHost2 = serverBuilder.virtualHost("foo.com:18080");
assertThat(virtualHost1).isSameAs(virtualHost2);
final VirtualHostBuilder virtualHost3 = serverBuilder.virtualHost("foo.com:18081");
assertThat(virtualHost2).isNotSameAs(virtualHost3);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -227,4 +227,12 @@ void portBasedVirtualHostWithTls() {
assertThat(server.config().virtualHosts().stream().map(VirtualHost::originalHostnamePattern))
.containsExactly("*", "foo.com", "*");
}

@Test
void notAllowSettingHostnameWhenDefaultVirtualHost() {
final VirtualHostBuilder virtualHostBuilder = Server.builder()
.virtualHost(8080);
assertThatThrownBy(() -> virtualHostBuilder.hostnamePattern("foo.com"))
.isInstanceOf(UnsupportedOperationException.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,8 @@ void serviceConfigurationPriority() {
Duration.ofMillis(250),
ctx.eventLoop())))
.withVirtualHost(
h -> h.hostnamePattern("foo.com")
.service("/custom_virtual_host",
"foo.com",
h -> h.service("/custom_virtual_host",
(ctx, req) -> HttpResponse.delayed(
HttpResponse.of(HttpStatus.OK),
Duration.ofMillis(150),
Expand Down

0 comments on commit dad02d6

Please sign in to comment.