From 8380608ed760d1a85c0bb9f44e66ccfd03a7918c Mon Sep 17 00:00:00 2001 From: minwoox Date: Wed, 8 Jun 2022 21:23:55 +0900 Subject: [PATCH 1/4] Remove pseudo headers when converting to Spring headers Motivation: We store HTTP/2 pseudo headers to the map in `HttpHeaders` with other headers. Then, the entries of the map are copied to Spring `HttpHeaders` in Spring integration. However, it can be a problem when the copied Spring `HttpHeaders` is converted to Netty Headers later: https://github.com/spring-cloud/spring-cloud-gateway/blob/v3.1.3/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyRoutingFilter.java#L125-L128 The validation is failed when the pseudo headers are set. Modifications: - Remove HTTP/2 pseudo headers when creating Spring `HttpHeaders` from Armeria `HttpHeaders`. - Convert `:authority` header to `HOST` header. Result: - You no longer see `IllegalArgumentException` indicating prohibited character of a header name in an environment that Spring integration module and Netty client are used togeter.(e.g. Spring Cloud Gateway) --- .../reactive/ArmeriaClientHttpResponse.java | 9 +--- .../web/reactive/ArmeriaHttpHeadersUtil.java | 49 +++++++++++++++++++ .../reactive/ArmeriaServerHttpRequest.java | 13 +++-- .../ArmeriaClientHttpRequestTest.java | 10 ++++ .../ArmeriaClientHttpResponseTest.java | 7 +++ .../ArmeriaServerHttpRequestTest.java | 22 +++++++++ .../ArmeriaServerHttpResponseTest.java | 7 +++ 7 files changed, 103 insertions(+), 14 deletions(-) create mode 100644 spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaHttpHeadersUtil.java diff --git a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java index 8032d0f390b..3ee3339b32f 100644 --- a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java +++ b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java @@ -15,6 +15,7 @@ */ package com.linecorp.armeria.spring.web.reactive; +import static com.linecorp.armeria.spring.web.reactive.ArmeriaHttpHeadersUtil.fromArmeriaHttpHeaders; import static java.util.Objects.requireNonNull; import org.springframework.core.io.buffer.DataBuffer; @@ -92,7 +93,7 @@ public HttpHeaders getHeaders() { if (httpHeaders != null) { return httpHeaders; } - this.httpHeaders = initHttpHeaders(); + this.httpHeaders = fromArmeriaHttpHeaders(headers); return this.httpHeaders; } @@ -119,10 +120,4 @@ private MultiValueMap initCookies() { .build())); return cookies; } - - private HttpHeaders initHttpHeaders() { - final HttpHeaders httpHeaders = new HttpHeaders(); - headers.forEach(entry -> httpHeaders.add(entry.getKey().toString(), entry.getValue())); - return httpHeaders; - } } diff --git a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaHttpHeadersUtil.java b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaHttpHeadersUtil.java new file mode 100644 index 00000000000..f46a3708e92 --- /dev/null +++ b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaHttpHeadersUtil.java @@ -0,0 +1,49 @@ +/* + * Copyright 2022 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.spring.web.reactive; + +import java.util.Map.Entry; + +import org.springframework.http.HttpHeaders; + +import com.linecorp.armeria.common.HttpHeaderNames; + +import io.netty.util.AsciiString; + +final class ArmeriaHttpHeadersUtil { + + static HttpHeaders fromArmeriaHttpHeaders(com.linecorp.armeria.common.HttpHeaders armeriaHeaders) { + final HttpHeaders springHeaders = new HttpHeaders(); + for (Entry e : armeriaHeaders) { + final AsciiString k = e.getKey(); + final String v = e.getValue(); + + if (k.isEmpty()) { + continue; + } + + if (k.charAt(0) != ':') { + springHeaders.add(k.toString(), v); + } else if (HttpHeaderNames.AUTHORITY.equals(k) && !armeriaHeaders.contains(HttpHeaderNames.HOST)) { + // Convert `:authority` to `host`. + springHeaders.add(HttpHeaderNames.HOST.toString(), v); + } + } + return springHeaders; + } + + private ArmeriaHttpHeadersUtil() {} +} diff --git a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java index 0a3a1b1a4dc..168c0a3b16b 100644 --- a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java +++ b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java @@ -15,6 +15,7 @@ */ package com.linecorp.armeria.spring.web.reactive; +import static com.linecorp.armeria.spring.web.reactive.ArmeriaHttpHeadersUtil.fromArmeriaHttpHeaders; import static java.util.Objects.requireNonNull; import java.net.InetSocketAddress; @@ -26,7 +27,6 @@ import org.springframework.core.io.buffer.DataBuffer; import org.springframework.http.HttpCookie; -import org.springframework.http.HttpHeaders; import org.springframework.http.server.reactive.AbstractServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.SslInfo; @@ -76,12 +76,6 @@ private static URI uri(HttpRequest req) { 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())); - return newHttpHeaders; - } - @Override protected MultiValueMap initCookies() { final MultiValueMap cookies = new LinkedMultiValueMap<>(); @@ -122,6 +116,11 @@ public Flux getBody() { return body; } + @Override + public InetSocketAddress getLocalAddress() { + return ctx.localAddress(); + } + @Override public InetSocketAddress getRemoteAddress() { return ctx.remoteAddress(); diff --git a/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpRequestTest.java b/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpRequestTest.java index 8de8ad6e01b..9c9ed4b5239 100644 --- a/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpRequestTest.java +++ b/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpRequestTest.java @@ -22,6 +22,8 @@ import static org.mockito.Mockito.when; import java.net.URI; +import java.util.List; +import java.util.Map.Entry; import org.junit.BeforeClass; import org.junit.Test; @@ -75,6 +77,14 @@ public void completeWithoutBody() { StepVerifier.create(httpRequest).expectComplete().verify(); await().until(() -> httpRequest.whenComplete().isDone()); + + // Spring headers does not have pseudo headers. + for (Entry> header : request.getHeaders().entrySet()) { + assertThat(header.getKey()).doesNotStartWith(":"); + } + assertThat(httpRequest.headers().names()) + .contains(HttpHeaderNames.METHOD, HttpHeaderNames.AUTHORITY, + HttpHeaderNames.SCHEME, HttpHeaderNames.PATH); } @Test diff --git a/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponseTest.java b/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponseTest.java index dd0b25191a2..f44dd73f5fe 100644 --- a/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponseTest.java +++ b/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponseTest.java @@ -19,6 +19,8 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.awaitility.Awaitility.await; +import java.util.List; +import java.util.Map.Entry; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicBoolean; @@ -67,6 +69,11 @@ public void readBodyStream() { .verify(); await().until(() -> httpResponse.whenComplete().isDone()); + + // Spring headers does not have pseudo headers. + for (Entry> header : response.getHeaders().entrySet()) { + assertThat(header.getKey()).doesNotStartWith(":"); + } } @Test diff --git a/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequestTest.java b/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequestTest.java index cb67e089ce3..2a52c4ddd4c 100644 --- a/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequestTest.java +++ b/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequestTest.java @@ -19,6 +19,8 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.awaitility.Awaitility.await; +import java.util.List; +import java.util.Map.Entry; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -85,6 +87,26 @@ void readBodyStream() { await().until(() -> httpRequest.whenComplete().isDone()); } + @Test + void springHeadersDoesNotHavePseudoHeaders() { + 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); + for (Entry> header : req.getHeaders().entrySet()) { + assertThat(header.getKey()).doesNotStartWith(":"); + } + // :authority header is converted to the HOST header. + assertThat(req.getHeaders().getFirst(HttpHeaderNames.HOST.toString())).isEqualTo("127.0.0.1"); + final Object nativeRequest = req.getNativeRequest(); + assertThat(nativeRequest).isInstanceOf(HttpRequest.class).isEqualTo(httpRequest); + assertThat(((HttpRequest) nativeRequest).headers().names()) + .contains(HttpHeaderNames.METHOD, HttpHeaderNames.AUTHORITY, + HttpHeaderNames.SCHEME, HttpHeaderNames.PATH); + } + @Test void getCookies() { final HttpRequest httpRequest = HttpRequest.of(RequestHeaders.builder(HttpMethod.POST, "/") diff --git a/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpResponseTest.java b/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpResponseTest.java index 846b36c7da4..d198a979af9 100644 --- a/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpResponseTest.java +++ b/spring/boot2-webflux-autoconfigure/src/test/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpResponseTest.java @@ -20,6 +20,8 @@ import static org.awaitility.Awaitility.await; import java.time.Duration; +import java.util.List; +import java.util.Map.Entry; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicBoolean; @@ -109,6 +111,11 @@ void returnHeadersOnly() throws Exception { .verify(); await().until(() -> httpResponse.whenComplete().isDone()); + + // Spring headers does not have pseudo headers. + for (Entry> header : response.getHeaders().entrySet()) { + assertThat(header.getKey()).doesNotStartWith(":"); + } } @Test From f1df421cb946ff039ef89e7ab40a0d2b5ba02103 Mon Sep 17 00:00:00 2001 From: minwoox Date: Thu, 9 Jun 2022 20:03:13 +0900 Subject: [PATCH 2/4] Dedup converting header logic --- .../internal/common/ArmeriaHttpUtil.java | 20 ++++++++ .../armeria/server/jetty/JettyService.java | 16 +----- .../reactive/ArmeriaClientHttpResponse.java | 5 +- .../web/reactive/ArmeriaHttpHeadersUtil.java | 49 ------------------- .../reactive/ArmeriaServerHttpRequest.java | 12 ++++- .../armeria/server/tomcat/TomcatService.java | 23 +-------- 6 files changed, 37 insertions(+), 88 deletions(-) delete mode 100644 spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaHttpHeadersUtil.java diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java b/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java index 61a8f59b941..ee59db2d329 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java @@ -1078,5 +1078,25 @@ public static boolean isRequestTimeoutResponse(HttpResponse httpResponse) { "close".equalsIgnoreCase(httpResponse.headers().get(HttpHeaderNames.CONNECTION)); } + /** + * Copies header value pairs of the specified {@linkplain HttpHeaders Armeria headers} to the + * {@link BiConsumer} excluding HTTP/2 pseudo headers that starts with ':'. This also converts + * {@link HttpHeaderNames#AUTHORITY} header to {@link HttpHeaderNames#HOST} header if + * the {@linkplain HttpHeaders Armeria headers} does not have one. + */ + public static void toHttp1Headers(HttpHeaders armeriaHeaders, + BiConsumer outputHeaders) { + for (Entry e : armeriaHeaders) { + final AsciiString k = e.getKey(); + final String v = e.getValue(); + if (k.charAt(0) != ':') { + outputHeaders.accept(k, v); + } else if (HttpHeaderNames.AUTHORITY.equals(k) && !armeriaHeaders.contains(HttpHeaderNames.HOST)) { + // Convert `:authority` to `host`. + outputHeaders.accept(HttpHeaderNames.HOST, v); + } + } + } + private ArmeriaHttpUtil() {} } diff --git a/jetty9/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java b/jetty9/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java index f6b4cbc788d..27b97ab8313 100644 --- a/jetty9/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java +++ b/jetty9/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.server.jetty; +import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.toHttp1Headers; import static java.util.Objects.requireNonNull; import java.lang.invoke.MethodHandle; @@ -69,7 +70,6 @@ import com.linecorp.armeria.server.ServiceRequestContext; import io.netty.buffer.ByteBuf; -import io.netty.util.AsciiString; /** * An {@link HttpService} that dispatches its requests to a web application running in an embedded @@ -375,19 +375,7 @@ private static MetaData.Request toRequestMetadata(ServiceRequestContext ctx, Agg // Convert HttpHeaders to HttpFields final HttpFields jHeaders = new HttpFields(aHeaders.size()); - aHeaders.forEach(e -> { - final AsciiString key = e.getKey(); - if (key.isEmpty()) { - return; - } - - if (key.byteAt(0) != ':') { - jHeaders.add(key.toString(), e.getValue()); - } else if (HttpHeaderNames.AUTHORITY.equals(key) && !aHeaders.contains(HttpHeaderNames.HOST)) { - // Convert `:authority` to `host`. - jHeaders.add(HttpHeaderNames.HOST.toString(), e.getValue()); - } - }); + toHttp1Headers(aHeaders, (key, value) -> jHeaders.add(key.toString(), value)); return new MetaData.Request(aHeaders.get(HttpHeaderNames.METHOD), uri, ctx.sessionProtocol().isMultiplex() ? HttpVersion.HTTP_2 diff --git a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java index 3ee3339b32f..120f9b51e38 100644 --- a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java +++ b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java @@ -15,7 +15,7 @@ */ package com.linecorp.armeria.spring.web.reactive; -import static com.linecorp.armeria.spring.web.reactive.ArmeriaHttpHeadersUtil.fromArmeriaHttpHeaders; +import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.toHttp1Headers; import static java.util.Objects.requireNonNull; import org.springframework.core.io.buffer.DataBuffer; @@ -93,7 +93,8 @@ public HttpHeaders getHeaders() { if (httpHeaders != null) { return httpHeaders; } - this.httpHeaders = fromArmeriaHttpHeaders(headers); + this.httpHeaders = new HttpHeaders(); + toHttp1Headers(headers, (header, value) -> this.httpHeaders.add(header.toString(), value)); return this.httpHeaders; } diff --git a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaHttpHeadersUtil.java b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaHttpHeadersUtil.java deleted file mode 100644 index f46a3708e92..00000000000 --- a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaHttpHeadersUtil.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Copyright 2022 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.spring.web.reactive; - -import java.util.Map.Entry; - -import org.springframework.http.HttpHeaders; - -import com.linecorp.armeria.common.HttpHeaderNames; - -import io.netty.util.AsciiString; - -final class ArmeriaHttpHeadersUtil { - - static HttpHeaders fromArmeriaHttpHeaders(com.linecorp.armeria.common.HttpHeaders armeriaHeaders) { - final HttpHeaders springHeaders = new HttpHeaders(); - for (Entry e : armeriaHeaders) { - final AsciiString k = e.getKey(); - final String v = e.getValue(); - - if (k.isEmpty()) { - continue; - } - - if (k.charAt(0) != ':') { - springHeaders.add(k.toString(), v); - } else if (HttpHeaderNames.AUTHORITY.equals(k) && !armeriaHeaders.contains(HttpHeaderNames.HOST)) { - // Convert `:authority` to `host`. - springHeaders.add(HttpHeaderNames.HOST.toString(), v); - } - } - return springHeaders; - } - - private ArmeriaHttpHeadersUtil() {} -} diff --git a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java index 168c0a3b16b..b4ceb349ad1 100644 --- a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java +++ b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java @@ -15,7 +15,7 @@ */ package com.linecorp.armeria.spring.web.reactive; -import static com.linecorp.armeria.spring.web.reactive.ArmeriaHttpHeadersUtil.fromArmeriaHttpHeaders; +import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.toHttp1Headers; import static java.util.Objects.requireNonNull; import java.net.InetSocketAddress; @@ -27,6 +27,7 @@ import org.springframework.core.io.buffer.DataBuffer; import org.springframework.http.HttpCookie; +import org.springframework.http.HttpHeaders; import org.springframework.http.server.reactive.AbstractServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.SslInfo; @@ -38,6 +39,7 @@ import com.linecorp.armeria.common.HttpData; import com.linecorp.armeria.common.HttpHeaderNames; import com.linecorp.armeria.common.HttpRequest; +import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.server.ServiceRequestContext; @@ -57,7 +59,7 @@ final class ArmeriaServerHttpRequest extends AbstractServerHttpRequest { ArmeriaServerHttpRequest(ServiceRequestContext ctx, HttpRequest req, DataBufferFactoryWrapper factoryWrapper) { - super(uri(req), null, fromArmeriaHttpHeaders(req.headers())); + super(uri(req), null, springHeaders(req.headers())); this.ctx = requireNonNull(ctx, "ctx"); this.req = req; @@ -67,6 +69,12 @@ final class ArmeriaServerHttpRequest extends AbstractServerHttpRequest { .publishOn(Schedulers.fromExecutor(ctx.eventLoop())); } + private static HttpHeaders springHeaders(RequestHeaders headers) { + final HttpHeaders springHeaders = new HttpHeaders(); + toHttp1Headers(headers, (key, value) -> springHeaders.add(key.toString(), value)); + return springHeaders; + } + private static URI uri(HttpRequest req) { final String scheme = req.scheme(); final String authority = req.authority(); diff --git a/tomcat9/src/main/java/com/linecorp/armeria/server/tomcat/TomcatService.java b/tomcat9/src/main/java/com/linecorp/armeria/server/tomcat/TomcatService.java index aa1a7e7c890..098174b8a55 100644 --- a/tomcat9/src/main/java/com/linecorp/armeria/server/tomcat/TomcatService.java +++ b/tomcat9/src/main/java/com/linecorp/armeria/server/tomcat/TomcatService.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.server.tomcat; import static com.google.common.base.Preconditions.checkArgument; +import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.toHttp1Headers; import static java.util.Objects.requireNonNull; import java.io.File; @@ -28,7 +29,6 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayDeque; -import java.util.Map.Entry; import java.util.Queue; import org.apache.catalina.LifecycleState; @@ -528,26 +528,7 @@ private static void convertHeaders(HttpHeaders headers, MimeHeaders cHeaders) { if (headers.isEmpty()) { return; } - - for (Entry e : headers) { - final AsciiString k = e.getKey(); - final String v = e.getValue(); - - if (k.isEmpty()) { - continue; - } - - if (k.byteAt(0) != ':') { - final byte[] valueBytes = v.getBytes(StandardCharsets.US_ASCII); - cHeaders.addValue(k.array(), k.arrayOffset(), k.length()) - .setBytes(valueBytes, 0, valueBytes.length); - } else if (HttpHeaderNames.AUTHORITY.equals(k) && !headers.contains(HttpHeaderNames.HOST)) { - // Convert `:authority` to `host`. - final byte[] valueBytes = v.getBytes(StandardCharsets.US_ASCII); - cHeaders.addValue(HOST_BYTES, 0, HOST_BYTES.length) - .setBytes(valueBytes, 0, valueBytes.length); - } - } + toHttp1Headers(headers, (key, value) -> cHeaders.addValue(key.toString()).setString(value)); } private static ResponseHeaders convertResponse(Response coyoteRes) { From 0b2e5e2cb6956517d64e5c3997814ff488f82a50 Mon Sep 17 00:00:00 2001 From: minwoox Date: Fri, 10 Jun 2022 17:17:45 +0900 Subject: [PATCH 3/4] Address the comment by @ikhoon --- .../armeria/internal/common/ArmeriaHttpUtil.java | 16 +++++++++++----- .../armeria/server/jetty/JettyService.java | 2 +- .../web/reactive/ArmeriaClientHttpResponse.java | 3 ++- .../web/reactive/ArmeriaServerHttpRequest.java | 2 +- .../armeria/server/tomcat/TomcatService.java | 3 ++- 5 files changed, 17 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java b/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java index ee59db2d329..4517b1d38d9 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java @@ -1080,23 +1080,29 @@ public static boolean isRequestTimeoutResponse(HttpResponse httpResponse) { /** * Copies header value pairs of the specified {@linkplain HttpHeaders Armeria headers} to the - * {@link BiConsumer} excluding HTTP/2 pseudo headers that starts with ':'. This also converts + * {@link TriConsumer} excluding HTTP/2 pseudo headers that starts with ':'. This also converts * {@link HttpHeaderNames#AUTHORITY} header to {@link HttpHeaderNames#HOST} header if * the {@linkplain HttpHeaders Armeria headers} does not have one. */ - public static void toHttp1Headers(HttpHeaders armeriaHeaders, - BiConsumer outputHeaders) { + public static void toHttp1Headers(HttpHeaders armeriaHeaders, T output, + TriConsumer writer) { for (Entry e : armeriaHeaders) { final AsciiString k = e.getKey(); final String v = e.getValue(); if (k.charAt(0) != ':') { - outputHeaders.accept(k, v); + writer.accept(output, k, v); } else if (HttpHeaderNames.AUTHORITY.equals(k) && !armeriaHeaders.contains(HttpHeaderNames.HOST)) { // Convert `:authority` to `host`. - outputHeaders.accept(HttpHeaderNames.HOST, v); + writer.accept(output, HttpHeaderNames.HOST, v); } } } + // TODO(minwoox): Will provide this interface to public API + @FunctionalInterface + public interface TriConsumer { + void accept(T t, U u, V v); + } + private ArmeriaHttpUtil() {} } diff --git a/jetty9/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java b/jetty9/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java index 27b97ab8313..95c08e46292 100644 --- a/jetty9/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java +++ b/jetty9/src/main/java/com/linecorp/armeria/server/jetty/JettyService.java @@ -375,7 +375,7 @@ private static MetaData.Request toRequestMetadata(ServiceRequestContext ctx, Agg // Convert HttpHeaders to HttpFields final HttpFields jHeaders = new HttpFields(aHeaders.size()); - toHttp1Headers(aHeaders, (key, value) -> jHeaders.add(key.toString(), value)); + toHttp1Headers(aHeaders, jHeaders, (output, key, value) -> output.add(key.toString(), value)); return new MetaData.Request(aHeaders.get(HttpHeaderNames.METHOD), uri, ctx.sessionProtocol().isMultiplex() ? HttpVersion.HTTP_2 diff --git a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java index 120f9b51e38..9362e5bdef2 100644 --- a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java +++ b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java @@ -94,7 +94,8 @@ public HttpHeaders getHeaders() { return httpHeaders; } this.httpHeaders = new HttpHeaders(); - toHttp1Headers(headers, (header, value) -> this.httpHeaders.add(header.toString(), value)); + toHttp1Headers(headers, this.httpHeaders, + (output, header, value) -> output.add(header.toString(), value)); return this.httpHeaders; } diff --git a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java index b4ceb349ad1..c30e0351e9b 100644 --- a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java +++ b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaServerHttpRequest.java @@ -71,7 +71,7 @@ final class ArmeriaServerHttpRequest extends AbstractServerHttpRequest { private static HttpHeaders springHeaders(RequestHeaders headers) { final HttpHeaders springHeaders = new HttpHeaders(); - toHttp1Headers(headers, (key, value) -> springHeaders.add(key.toString(), value)); + toHttp1Headers(headers, springHeaders, (output, key, value) -> output.add(key.toString(), value)); return springHeaders; } diff --git a/tomcat9/src/main/java/com/linecorp/armeria/server/tomcat/TomcatService.java b/tomcat9/src/main/java/com/linecorp/armeria/server/tomcat/TomcatService.java index 098174b8a55..170a13acd74 100644 --- a/tomcat9/src/main/java/com/linecorp/armeria/server/tomcat/TomcatService.java +++ b/tomcat9/src/main/java/com/linecorp/armeria/server/tomcat/TomcatService.java @@ -528,7 +528,8 @@ private static void convertHeaders(HttpHeaders headers, MimeHeaders cHeaders) { if (headers.isEmpty()) { return; } - toHttp1Headers(headers, (key, value) -> cHeaders.addValue(key.toString()).setString(value)); + toHttp1Headers(headers, cHeaders, + (output, key, value) -> output.addValue(key.toString()).setString(value)); } private static ResponseHeaders convertResponse(Response coyoteRes) { From 128b420f9b2772085bccb489b1e2c98011503547 Mon Sep 17 00:00:00 2001 From: minwoox Date: Mon, 13 Jun 2022 10:12:12 +0900 Subject: [PATCH 4/4] Rename to key --- .../armeria/spring/web/reactive/ArmeriaClientHttpResponse.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java index 9362e5bdef2..09355fd38d7 100644 --- a/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java +++ b/spring/boot2-webflux-autoconfigure/src/main/java/com/linecorp/armeria/spring/web/reactive/ArmeriaClientHttpResponse.java @@ -95,7 +95,7 @@ public HttpHeaders getHeaders() { } this.httpHeaders = new HttpHeaders(); toHttp1Headers(headers, this.httpHeaders, - (output, header, value) -> output.add(header.toString(), value)); + (output, key, value) -> output.add(key.toString(), value)); return this.httpHeaders; }