ClientRequestContext thorws a exception when host, authoriy is null.

line · Nov 7, 2024 · b2eae62 · b2eae62
1 parent c5b3f19
commit b2eae62
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 17 deletions.
diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContext.java b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContext.java
@@ -290,15 +290,11 @@ ClientRequestContext newDerivedContext(RequestId id, @Nullable HttpRequest req,
      *     <li>{@link Endpoint#authority()}.</li>
      * </ol>
      */
-    @Nullable
-    @UnstableApi
     String authority();
 
     /**
      * Returns the host part of {@link #authority()}, without a port number.
      */
-    @Nullable
-    @UnstableApi
     String host();
 
     /**

diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextWrapper.java b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextWrapper.java
@@ -70,13 +70,11 @@ public String fragment() {
         return unwrap().fragment();
     }
 
-    @Nullable
     @Override
     public String authority() {
         return unwrap().authority();
     }
 
-    @Nullable
     @Override
     public String host() {
         return unwrap().host();

diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java
@@ -483,15 +483,20 @@ private void failEarly(Throwable cause) {
 
     // TODO(ikhoon): Consider moving the logic for filling authority to `HttpClientDelegate.exceute()`.
     private void autoFillSchemeAuthorityAndOrigin() {
-        final String authority = authority();
-        if (authority != null && endpoint != null && endpoint.isIpAddrOnly()) {
-            // The connection will be established with the IP address but `host` set to the `Endpoint`
-            // could be used for SNI. It would make users send HTTPS requests with CSLB or configure a reverse
-            // proxy based on an authority.
-            final String host = SchemeAndAuthority.of(null, authority).host();
-            if (!NetUtil.isValidIpV4Address(host) && !NetUtil.isValidIpV6Address(host)) {
-                endpoint = endpoint.withHost(host);
+
+        try {
+            final String authority = authority();
+            if (endpoint != null && endpoint.isIpAddrOnly()) {
+                // The connection will be established with the IP address but `host` set to the `Endpoint`
+                // could be used for SNI. It would make users send HTTPS requests
+                // with CSLB or configure a reverse proxy based on an authority.
+                final String host = SchemeAndAuthority.of(null, authority).host();
+                if (!NetUtil.isValidIpV4Address(host) && !NetUtil.isValidIpV6Address(host)) {
+                    endpoint = endpoint.withHost(host);
+                }
             }
+        } catch (IllegalStateException e) {
+            // Just pass, because it's normal condition.
         }
 
         final HttpHeadersBuilder headersBuilder = internalRequestHeaders.toBuilder();
@@ -750,7 +755,6 @@ public String fragment() {
         return requestTarget().fragment();
     }
 
-    @Nullable
     @Override
     public String authority() {
         final HttpHeaders additionalRequestHeaders = this.additionalRequestHeaders;
@@ -774,6 +778,11 @@ public String authority() {
         if (authority == null) {
             authority = internalRequestHeaders.get(HttpHeaderNames.HOST);
         }
+        if (authority == null) {
+            throw new IllegalStateException(
+                    "ClientRequestContext may be in the process of initialization." +
+                    "In this case, host() or authority() could be null");
+        }
         return authority;
     }
 
@@ -794,12 +803,12 @@ private String origin() {
         return origin;
     }
 
-    @Nullable
     @Override
     public String host() {
         final String authority = authority();
         if (authority == null) {
-            return null;
+            throw new IllegalStateException("ClientRequestContext may be in the process of initialization." +
+                                            "In this case, host() or authority() could be null");
         }
         return SchemeAndAuthority.of(null, authority).host();
     }

diff --git a/core/src/test/java/com/linecorp/armeria/client/ClientRequestContextTest.java b/core/src/test/java/com/linecorp/armeria/client/ClientRequestContextTest.java
@@ -18,6 +18,8 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
 import java.util.function.Function;
 import java.util.stream.Stream;
 
@@ -29,6 +31,7 @@
 import org.junit.jupiter.params.provider.ArgumentsSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
+import com.linecorp.armeria.common.AggregatedHttpResponse;
 import com.linecorp.armeria.common.HttpMethod;
 import com.linecorp.armeria.common.HttpRequest;
 import com.linecorp.armeria.common.RequestContext;
@@ -269,6 +272,31 @@ void hasOwnAttr() {
         }
     }
 
+    @Test
+    void callAuthorityShouldBeThrownDuringPartiallyInit() {
+        final CountDownLatch countDownLatch = new CountDownLatch(1);
+        final WebClient client = WebClient
+                .builder("http://localhost:8080")
+                .contextCustomizer(ctx -> {
+                    countDownLatch.countDown();
+                    assertThatThrownBy(ctx::host)
+                            .isInstanceOf(IllegalStateException.class)
+                            .hasMessage(
+                                    "ClientRequestContext may be in the process of initialization." +
+                                    "In this case, host() or authority() could be null");
+                    assertThatThrownBy(ctx::authority)
+                            .isInstanceOf(IllegalStateException.class)
+                            .hasMessage(
+                                    "ClientRequestContext may be in the process of initialization." +
+                                    "In this case, host() or authority() could be null"
+                            );
+                    }).build();
+
+        final CompletableFuture<AggregatedHttpResponse> aggregate = client.get("/").aggregate();
+        assertThat(countDownLatch.getCount()).isEqualTo(0);
+        aggregate.cancel(true);
+    }
+
     @ParameterizedTest
     @ValueSource(strings = {"%", "http:///", "http://foo.com/bar"})
     void updateRequestWithInvalidPath(String path) {