Fix the uri of ArmeriaServerHttpRequest to contain the base url (line…

…#1904)

Motivation:
The uri of `ServerHttpRequest` in Spring Web flux contains the base URL.
Also, a negative value of "max-age" from "set-cookie" means no "max-age" attribute, we shouldn't set it.

Modification:
- Add the base URL when `ArmeriaServerHttpRequest` is created.
- Do not set max-age from set-cookie when the value is negative

Result:
- Spring integration works as expected

spring/boot-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java

@@ -56,9 +56,7 @@ final class ArmeriaServerHttpRequest extends AbstractServerHttpRequest {
     ArmeriaServerHttpRequest(ServiceRequestContext ctx,
                              HttpRequest req,
                              DataBufferFactoryWrapper<?> factoryWrapper) {
-        super(URI.create(requireNonNull(req, "req").path()),
-              null,
-              fromArmeriaHttpHeaders(req.headers()));
+        super(uri(req), null, fromArmeriaHttpHeaders(req.headers()));
         this.ctx = requireNonNull(ctx, "ctx");
         this.req = req;
 
@@ -68,6 +66,15 @@ final class ArmeriaServerHttpRequest extends AbstractServerHttpRequest {
                    .publishOn(Schedulers.fromExecutor(ctx.contextAwareExecutor()));
     }
 
+    private static URI uri(HttpRequest req) {
+        final String scheme = req.scheme();
+        final String authority = req.authority();
+        // Server side Armeria HTTP request always has the scheme and authority.
+        assert scheme != null;
+        assert authority != null;
+        return URI.create(scheme + "://" + authority + req.path());
+    }
+
     private static HttpHeaders fromArmeriaHttpHeaders(com.linecorp.armeria.common.HttpHeaders httpHeaders) {
         final HttpHeaders newHttpHeaders = new HttpHeaders();
         httpHeaders.forEach(entry -> newHttpHeaders.add(entry.getKey().toString(), entry.getValue()));

spring/boot-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpResponse.java

@@ -283,14 +283,19 @@ public String toString() {
     /**
      * Converts the specified {@link ResponseCookie} to Netty's {@link Cookie} interface.
      */
-    static Cookie toNettyCookie(ResponseCookie resCookie) {
+    private static Cookie toNettyCookie(ResponseCookie resCookie) {
         final DefaultCookie cookie = new DefaultCookie(resCookie.getName(), resCookie.getValue());
-        cookie.setHttpOnly(resCookie.isHttpOnly());
-        cookie.setMaxAge(resCookie.getMaxAge().getSeconds());
+        if (!resCookie.getMaxAge().isNegative()) {
+            cookie.setMaxAge(resCookie.getMaxAge().getSeconds());
+        }
+        if (resCookie.getDomain() != null) {
+            cookie.setDomain(resCookie.getDomain());
+        }
+        if (resCookie.getPath() != null) {
+            cookie.setPath(resCookie.getPath());
+        }
         cookie.setSecure(resCookie.isSecure());
-        // Domain and path are nullable, but the setters allow null as their input.
-        cookie.setDomain(resCookie.getDomain());
-        cookie.setPath(resCookie.getPath());
+        cookie.setHttpOnly(resCookie.isHttpOnly());
         return cookie;
     }
 

spring/boot-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequestTest.java

@@ -22,7 +22,7 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 import org.springframework.http.HttpCookie;
 import org.springframework.util.MultiValueMap;
 
@@ -39,16 +39,30 @@
 import reactor.core.publisher.Flux;
 import reactor.test.StepVerifier;
 
-public class ArmeriaServerHttpRequestTest {
+class ArmeriaServerHttpRequestTest {
 
     private static ArmeriaServerHttpRequest request(ServiceRequestContext ctx) {
         return new ArmeriaServerHttpRequest(ctx, ctx.request(), DataBufferFactoryWrapper.DEFAULT);
     }
 
     @Test
-    public void readBodyStream() throws Exception {
+    void requestUriContainsBaseUrl() {
+        final HttpRequest httpRequest = HttpRequest.of(RequestHeaders.builder(HttpMethod.POST, "/")
+                                                                     .scheme("http")
+                                                                     .authority("127.0.0.1")
+                                                                     .build());
+        final ServiceRequestContext ctx = newRequestContext(httpRequest);
+        final ArmeriaServerHttpRequest req = request(ctx);
+        assertThat(req.getURI().toString()).isEqualTo("http://127.0.0.1/");
+    }
+
+    @Test
+    void readBodyStream() {
         final HttpRequest httpRequest =
-                HttpRequest.of(RequestHeaders.of(HttpMethod.POST, "/"),
+                HttpRequest.of(RequestHeaders.builder(HttpMethod.POST, "/")
+                                             .scheme("http")
+                                             .authority("127.0.0.1")
+                                             .build(),
                                Flux.just("a", "b", "c", "d", "e").map(HttpData::ofUtf8));
 
         final ServiceRequestContext ctx = newRequestContext(httpRequest);
@@ -72,9 +86,12 @@ public void readBodyStream() throws Exception {
     }
 
     @Test
-    public void getCookies() {
-        final HttpRequest httpRequest = HttpRequest.of(RequestHeaders.of(HttpMethod.POST, "/",
-                                                                         HttpHeaderNames.COOKIE, "a=1;b=2"));
+    void getCookies() {
+        final HttpRequest httpRequest = HttpRequest.of(RequestHeaders.builder(HttpMethod.POST, "/")
+                                                                     .scheme("http")
+                                                                     .authority("127.0.0.1")
+                                                                     .add(HttpHeaderNames.COOKIE, "a=1;b=2")
+                                                                     .build());
         final ServiceRequestContext ctx = newRequestContext(httpRequest);
         final ArmeriaServerHttpRequest req = request(ctx);
 
@@ -88,9 +105,12 @@ public void getCookies() {
     }
 
     @Test
-    public void cancel() {
+    void cancel() {
         final HttpRequest httpRequest =
-                HttpRequest.of(RequestHeaders.of(HttpMethod.POST, "/"),
+                HttpRequest.of(RequestHeaders.builder(HttpMethod.POST, "/")
+                                             .scheme("http")
+                                             .authority("127.0.0.1")
+                                             .build(),
                                Flux.just("a", "b", "c", "d", "e").map(HttpData::ofUtf8));
 
         final ServiceRequestContext ctx = newRequestContext(httpRequest);

spring/boot-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpResponseTest.java

@@ -65,6 +65,15 @@ public void returnHeadersOnly() throws Exception {
         final ArmeriaServerHttpResponse response = response(ctx, future);
 
         response.setStatusCode(HttpStatus.NOT_FOUND);
+        response.addCookie(ResponseCookie.from("a", "1")
+                                         .domain("http://localhost")
+                                         .path("/")
+                                         // A negative value means no "Max-Age" attribute in which case
+                                         // the cookie is removed when the browser is closed.
+                                         .maxAge(-1)
+                                         .secure(true)
+                                         .httpOnly(true)
+                                         .build());
         assertThat(future.isDone()).isFalse();
 
         // Create HttpResponse.
@@ -84,6 +93,15 @@ public void returnHeadersOnly() throws Exception {
                         assertThat(o).isInstanceOf(ResponseHeaders.class);
                         final ResponseHeaders headers = (ResponseHeaders) o;
                         assertThat(headers.status().code()).isEqualTo(404);
+                        final Cookie setCookie =
+                                ClientCookieDecoder.LAX.decode(headers.get(HttpHeaderNames.SET_COOKIE));
+                        assertThat(setCookie.name()).isEqualTo("a");
+                        assertThat(setCookie.value()).isEqualTo("1");
+                        assertThat(setCookie.domain()).isEqualTo("http://localhost");
+                        assertThat(setCookie.path()).isEqualTo("/");
+                        assertThat(setCookie.maxAge()).isEqualTo(Cookie.UNDEFINED_MAX_AGE);
+                        assertThat(setCookie.isSecure()).isTrue();
+                        assertThat(setCookie.isHttpOnly()).isTrue();
                     })
                     .expectComplete()
                     .verify();