From 3276f818519f9bde187c4ccb83653c21c2e0e7fb Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 8 May 2020 10:34:26 +0100 Subject: [PATCH] Avoid one layer of HttpHeaders wrapping See gh-24680 --- .../org/springframework/http/HttpEntity.java | 8 ++--- .../reactive/AbstractServerHttpRequest.java | 4 +-- .../reactive/JettyHttpHandlerAdapter.java | 7 ++-- .../reactive/ReactorServerHttpRequest.java | 8 +---- .../reactive/ServletServerHttpRequest.java | 35 +++++++++++-------- .../reactive/TomcatHttpHandlerAdapter.java | 6 ++-- .../reactive/UndertowServerHttpRequest.java | 7 +--- .../result/view/DefaultRendering.java | 2 +- 8 files changed, 34 insertions(+), 43 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/HttpEntity.java b/spring-web/src/main/java/org/springframework/http/HttpEntity.java index 35b2b97caf..7b7fe9af60 100644 --- a/spring-web/src/main/java/org/springframework/http/HttpEntity.java +++ b/spring-web/src/main/java/org/springframework/http/HttpEntity.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -99,11 +99,7 @@ public class HttpEntity { */ public HttpEntity(@Nullable T body, @Nullable MultiValueMap headers) { this.body = body; - HttpHeaders tempHeaders = new HttpHeaders(); - if (headers != null) { - tempHeaders.putAll(headers); - } - this.headers = HttpHeaders.readOnlyHttpHeaders(tempHeaders); + this.headers = HttpHeaders.readOnlyHttpHeaders(headers != null ? headers : new HttpHeaders()); } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java index 4f4d628c68..f9d7b54a22 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -76,7 +76,7 @@ public abstract class AbstractServerHttpRequest implements ServerHttpRequest { * @param contextPath the context path for the request * @param headers the headers for the request */ - public AbstractServerHttpRequest(URI uri, @Nullable String contextPath, HttpHeaders headers) { + public AbstractServerHttpRequest(URI uri, @Nullable String contextPath, MultiValueMap headers) { this.uri = uri; this.path = RequestPath.parse(uri, contextPath); this.headers = HttpHeaders.readOnlyHttpHeaders(headers); diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/JettyHttpHandlerAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/JettyHttpHandlerAdapter.java index e0c538bc47..22842c3554 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/JettyHttpHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/JettyHttpHandlerAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,6 +36,7 @@ import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.util.Assert; +import org.springframework.util.MultiValueMap; /** * {@link ServletHttpHandlerAdapter} extension that uses Jetty APIs for writing @@ -79,9 +80,9 @@ public class JettyHttpHandlerAdapter extends ServletHttpHandlerAdapter { super(createHeaders(request), request, asyncContext, servletPath, bufferFactory, bufferSize); } - private static HttpHeaders createHeaders(HttpServletRequest request) { + private static MultiValueMap createHeaders(HttpServletRequest request) { HttpFields fields = ((Request) request).getMetaData().getFields(); - return new HttpHeaders(new JettyHeadersAdapter(fields)); + return new JettyHeadersAdapter(fields); } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java index 9cd57c08c0..8935787753 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ReactorServerHttpRequest.java @@ -33,7 +33,6 @@ import reactor.netty.http.server.HttpServerRequest; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.NettyDataBufferFactory; import org.springframework.http.HttpCookie; -import org.springframework.http.HttpHeaders; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; @@ -59,7 +58,7 @@ class ReactorServerHttpRequest extends AbstractServerHttpRequest { public ReactorServerHttpRequest(HttpServerRequest request, NettyDataBufferFactory bufferFactory) throws URISyntaxException { - super(initUri(request), "", initHeaders(request)); + super(initUri(request), "", new NettyHeadersAdapter(request.requestHeaders())); Assert.notNull(bufferFactory, "DataBufferFactory must not be null"); this.request = request; this.bufferFactory = bufferFactory; @@ -127,11 +126,6 @@ class ReactorServerHttpRequest extends AbstractServerHttpRequest { return uri; } - private static HttpHeaders initHeaders(HttpServerRequest channel) { - NettyHeadersAdapter headersMap = new NettyHeadersAdapter(channel.requestHeaders()); - return new HttpHeaders(headersMap); - } - @Override public String getMethodValue() { diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java index 6ea6e9404d..867ac93f3e 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,6 +23,7 @@ import java.net.URISyntaxException; import java.nio.charset.Charset; import java.security.cert.X509Certificate; import java.util.Enumeration; +import java.util.Locale; import java.util.Map; import javax.servlet.AsyncContext; @@ -44,6 +45,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedCaseInsensitiveMap; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; @@ -77,8 +79,8 @@ class ServletServerHttpRequest extends AbstractServerHttpRequest { this(createDefaultHttpHeaders(request), request, asyncContext, servletPath, bufferFactory, bufferSize); } - public ServletServerHttpRequest(HttpHeaders headers, HttpServletRequest request, AsyncContext asyncContext, - String servletPath, DataBufferFactory bufferFactory, int bufferSize) + public ServletServerHttpRequest(MultiValueMap headers, HttpServletRequest request, + AsyncContext asyncContext, String servletPath, DataBufferFactory bufferFactory, int bufferSize) throws IOException, URISyntaxException { super(initUri(request), request.getContextPath() + servletPath, initHeaders(headers, request)); @@ -99,8 +101,9 @@ class ServletServerHttpRequest extends AbstractServerHttpRequest { } - private static HttpHeaders createDefaultHttpHeaders(HttpServletRequest request) { - HttpHeaders headers = new HttpHeaders(); + private static MultiValueMap createDefaultHttpHeaders(HttpServletRequest request) { + MultiValueMap headers = + CollectionUtils.toMultiValueMap(new LinkedCaseInsensitiveMap<>(8, Locale.ENGLISH)); for (Enumeration names = request.getHeaderNames(); names.hasMoreElements(); ) { String name = (String) names.nextElement(); for (Enumeration values = request.getHeaders(name); values.hasMoreElements(); ) { @@ -120,34 +123,36 @@ class ServletServerHttpRequest extends AbstractServerHttpRequest { return new URI(url.toString()); } - private static HttpHeaders initHeaders(HttpHeaders headers, HttpServletRequest request) { - MediaType contentType = headers.getContentType(); - if (contentType == null) { + private static MultiValueMap initHeaders( + MultiValueMap headerValues, HttpServletRequest request) { + + HttpHeaders headers = null; + MediaType contentType = null; + if (!StringUtils.hasLength(headerValues.getFirst(HttpHeaders.CONTENT_TYPE))) { String requestContentType = request.getContentType(); if (StringUtils.hasLength(requestContentType)) { contentType = MediaType.parseMediaType(requestContentType); + headers = new HttpHeaders(headerValues); headers.setContentType(contentType); } } if (contentType != null && contentType.getCharset() == null) { String encoding = request.getCharacterEncoding(); if (StringUtils.hasLength(encoding)) { - Charset charset = Charset.forName(encoding); Map params = new LinkedCaseInsensitiveMap<>(); params.putAll(contentType.getParameters()); - params.put("charset", charset.toString()); - headers.setContentType( - new MediaType(contentType.getType(), contentType.getSubtype(), - params)); + params.put("charset", Charset.forName(encoding).toString()); + headers.setContentType(new MediaType(contentType, params)); } } - if (headers.getContentLength() == -1) { + if (headerValues.getFirst(HttpHeaders.CONTENT_TYPE) == null) { int contentLength = request.getContentLength(); if (contentLength != -1) { + headers = (headers != null ? headers : new HttpHeaders(headerValues)); headers.setContentLength(contentLength); } } - return headers; + return (headers != null ? headers : headerValues); } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java index 783be84a52..7920c7ffd8 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java @@ -44,6 +44,7 @@ import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.util.Assert; +import org.springframework.util.MultiValueMap; import org.springframework.util.ReflectionUtils; /** @@ -105,14 +106,13 @@ public class TomcatHttpHandlerAdapter extends ServletHttpHandlerAdapter { this.bufferSize = bufferSize; } - private static HttpHeaders createTomcatHttpHeaders(HttpServletRequest request) { + private static MultiValueMap createTomcatHttpHeaders(HttpServletRequest request) { RequestFacade requestFacade = getRequestFacade(request); org.apache.catalina.connector.Request connectorRequest = (org.apache.catalina.connector.Request) ReflectionUtils.getField(COYOTE_REQUEST_FIELD, requestFacade); Assert.state(connectorRequest != null, "No Tomcat connector request"); Request tomcatRequest = connectorRequest.getCoyoteRequest(); - TomcatHeadersAdapter headers = new TomcatHeadersAdapter(tomcatRequest.getMimeHeaders()); - return new HttpHeaders(headers); + return new TomcatHeadersAdapter(tomcatRequest.getMimeHeaders()); } private static RequestFacade getRequestFacade(HttpServletRequest request) { diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java index 11b4418951..045670a344 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/UndertowServerHttpRequest.java @@ -38,7 +38,6 @@ import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.core.io.buffer.DataBufferWrapper; import org.springframework.core.io.buffer.PooledDataBuffer; import org.springframework.http.HttpCookie; -import org.springframework.http.HttpHeaders; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; @@ -63,7 +62,7 @@ class UndertowServerHttpRequest extends AbstractServerHttpRequest { public UndertowServerHttpRequest(HttpServerExchange exchange, DataBufferFactory bufferFactory) throws URISyntaxException { - super(initUri(exchange), "", initHeaders(exchange)); + super(initUri(exchange), "", new UndertowHeadersAdapter(exchange.getRequestHeaders())); this.exchange = exchange; this.body = new RequestBodyPublisher(exchange, bufferFactory); this.body.registerListeners(exchange); @@ -77,10 +76,6 @@ class UndertowServerHttpRequest extends AbstractServerHttpRequest { return new URI(requestUriAndQuery); } - private static HttpHeaders initHeaders(HttpServerExchange exchange) { - return new HttpHeaders(new UndertowHeadersAdapter(exchange.getRequestHeaders())); - } - @Override public String getMethodValue() { return this.exchange.getRequestMethod().toString(); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/DefaultRendering.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/DefaultRendering.java index 30a53013db..a05e653c49 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/DefaultRendering.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/DefaultRendering.java @@ -32,7 +32,7 @@ import org.springframework.ui.Model; */ class DefaultRendering implements Rendering { - private static final HttpHeaders EMPTY_HEADERS = HttpHeaders.readOnlyHttpHeaders(new HttpHeaders()); + private static final HttpHeaders EMPTY_HEADERS = HttpHeaders.EMPTY; private final Object view;