From c908ec19378f3c6e8aac30d877fbf230fe2bc3c4 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 8 May 2020 14:11:37 +0100 Subject: [PATCH] Wrappers for native headers on the client side Native server request headers have been wrapped rather than copied since 5.1. This commit applies the same change to the client side where, to make matters worse, headers were copied repeatedly on every access, see also previous commit ca897b95. For Netty and Jetty the wrappers are the same as on the server side but duplicated in order to remain package private. See gh-24680 --- .../HttpComponentsClientHttpResponse.java | 11 +- .../HttpComponentsHeadersAdapter.java | 240 ++++++++++++++++++ .../reactive/JettyClientHttpResponse.java | 13 +- .../client/reactive/JettyHeadersAdapter.java | 230 +++++++++++++++++ .../client/reactive/NettyHeadersAdapter.java | 229 +++++++++++++++++ .../reactive/ReactorClientHttpResponse.java | 13 +- .../server/reactive/JettyHeadersAdapter.java | 4 +- .../server/reactive/NettyHeadersAdapter.java | 4 +- 8 files changed, 727 insertions(+), 17 deletions(-) create mode 100644 spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsHeadersAdapter.java create mode 100644 spring-web/src/main/java/org/springframework/http/client/reactive/JettyHeadersAdapter.java create mode 100644 spring-web/src/main/java/org/springframework/http/client/reactive/NettyHeadersAdapter.java diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpResponse.java index 2ae593121e..7be76e78bd 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpResponse.java @@ -17,7 +17,6 @@ package org.springframework.http.client.reactive; import java.nio.ByteBuffer; -import java.util.Arrays; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.hc.client5.http.cookie.Cookie; @@ -49,6 +48,8 @@ class HttpComponentsClientHttpResponse implements ClientHttpResponse { private final Message> message; + private final HttpHeaders headers; + private final HttpClientContext context; private final AtomicBoolean rejectSubscribers = new AtomicBoolean(); @@ -61,6 +62,9 @@ class HttpComponentsClientHttpResponse implements ClientHttpResponse { this.dataBufferFactory = dataBufferFactory; this.message = message; this.context = context; + + MultiValueMap adapter = new HttpComponentsHeadersAdapter(message.getHead()); + this.headers = HttpHeaders.readOnlyHttpHeaders(adapter); } @@ -107,9 +111,6 @@ class HttpComponentsClientHttpResponse implements ClientHttpResponse { @Override public HttpHeaders getHeaders() { - return Arrays.stream(this.message.getHead().getHeaders()) - .collect(HttpHeaders::new, - (httpHeaders, header) -> httpHeaders.add(header.getName(), header.getValue()), - HttpHeaders::putAll); + return this.headers; } } diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsHeadersAdapter.java b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsHeadersAdapter.java new file mode 100644 index 0000000000..7880f6e0e4 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsHeadersAdapter.java @@ -0,0 +1,240 @@ +/* + * 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. + * 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 org.springframework.http.client.reactive; + +import java.util.AbstractSet; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpResponse; + +import org.springframework.http.HttpHeaders; +import org.springframework.lang.Nullable; +import org.springframework.util.MultiValueMap; + +/** + * {@code MultiValueMap} implementation for wrapping Apache HttpComponents + * HttpClient headers. + * + * @author Rossen Stoyanchev + * @since 5.3 + */ +class HttpComponentsHeadersAdapter implements MultiValueMap { + + private final HttpResponse response; + + + HttpComponentsHeadersAdapter(HttpResponse response) { + this.response = response; + } + + + @Override + public String getFirst(String key) { + Header header = this.response.getFirstHeader(key); + return (header != null ? header.getValue() : null); + } + + @Override + public void add(String key, @Nullable String value) { + this.response.addHeader(key, value); + } + + @Override + public void addAll(String key, List values) { + values.forEach(value -> add(key, value)); + } + + @Override + public void addAll(MultiValueMap values) { + values.forEach(this::addAll); + } + + @Override + public void set(String key, @Nullable String value) { + this.response.setHeader(key, value); + } + + @Override + public void setAll(Map values) { + values.forEach(this::set); + } + + @Override + public Map toSingleValueMap() { + Map map = new LinkedHashMap<>(size()); + this.response.headerIterator().forEachRemaining(h -> map.putIfAbsent(h.getName(), h.getValue())); + return map; + } + + @Override + public int size() { + return this.response.getHeaders().length; + } + + @Override + public boolean isEmpty() { + return (this.response.getHeaders().length == 0); + } + + @Override + public boolean containsKey(Object key) { + return (key instanceof String && this.response.containsHeader((String) key)); + } + + @Override + public boolean containsValue(Object value) { + return (value instanceof String && + Arrays.stream(this.response.getHeaders()).anyMatch(h -> h.getValue().equals(value))); + } + + @Nullable + @Override + public List get(Object key) { + List values = null; + if (containsKey(key)) { + Header[] headers = this.response.getHeaders((String) key); + values = new ArrayList<>(headers.length); + for (Header header : headers) { + values.add(header.getValue()); + } + } + return values; + } + + @Nullable + @Override + public List put(String key, List values) { + List oldValues = remove(key); + values.forEach(value -> add(key, value)); + return oldValues; + } + + @Nullable + @Override + public List remove(Object key) { + if (key instanceof String) { + List oldValues = get(key); + this.response.removeHeaders((String) key); + return oldValues; + } + return null; + } + + @Override + public void putAll(Map> map) { + map.forEach(this::put); + } + + @Override + public void clear() { + this.response.setHeaders(); + } + + @Override + public Set keySet() { + Set keys = new LinkedHashSet<>(size()); + for (Header header : this.response.getHeaders()) { + keys.add(header.getName()); + } + return keys; + } + + @Override + public Collection> values() { + Collection> values = new ArrayList<>(size()); + for (Header header : this.response.getHeaders()) { + values.add(get(header.getName())); + } + return values; + } + + @Override + public Set>> entrySet() { + return new AbstractSet>>() { + @Override + public Iterator>> iterator() { + return new EntryIterator(); + } + + @Override + public int size() { + return HttpComponentsHeadersAdapter.this.size(); + } + }; + } + + + @Override + public String toString() { + return HttpHeaders.formatHeaders(this); + } + + + private class EntryIterator implements Iterator>> { + + private Iterator
iterator = response.headerIterator(); + + @Override + public boolean hasNext() { + return this.iterator.hasNext(); + } + + @Override + public Entry> next() { + return new HeaderEntry(this.iterator.next().getName()); + } + } + + + private class HeaderEntry implements Entry> { + + private final String key; + + HeaderEntry(String key) { + this.key = key; + } + + @Override + public String getKey() { + return this.key; + } + + @Override + public List getValue() { + List values = HttpComponentsHeadersAdapter.this.get(this.key); + return values != null ? values : Collections.emptyList(); + } + + @Override + public List setValue(List value) { + List previousValues = getValue(); + HttpComponentsHeadersAdapter.this.put(this.key, value); + return previousValues; + } + } + +} diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java index 67c6a7aced..29683498e8 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java @@ -36,7 +36,8 @@ import org.springframework.util.MultiValueMap; * * @author Sebastien Deleuze * @since 5.1 - * @see Jetty ReactiveStreams HttpClient + * @see + * Jetty ReactiveStreams HttpClient */ class JettyClientHttpResponse implements ClientHttpResponse { @@ -44,10 +45,15 @@ class JettyClientHttpResponse implements ClientHttpResponse { private final Flux content; + private final HttpHeaders headers; + public JettyClientHttpResponse(ReactiveResponse reactiveResponse, Publisher content) { this.reactiveResponse = reactiveResponse; this.content = Flux.from(content); + + MultiValueMap adapter = new JettyHeadersAdapter(reactiveResponse.getHeaders()); + this.headers = HttpHeaders.readOnlyHttpHeaders(adapter); } @@ -86,10 +92,7 @@ class JettyClientHttpResponse implements ClientHttpResponse { @Override public HttpHeaders getHeaders() { - HttpHeaders headers = new HttpHeaders(); - this.reactiveResponse.getHeaders().stream() - .forEach(field -> headers.add(field.getName(), field.getValue())); - return headers; + return this.headers; } } diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyHeadersAdapter.java b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyHeadersAdapter.java new file mode 100644 index 0000000000..dd2544f585 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyHeadersAdapter.java @@ -0,0 +1,230 @@ +/* + * 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. + * 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 org.springframework.http.client.reactive; + +import java.util.AbstractSet; +import java.util.Collection; +import java.util.Enumeration; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpFields; + +import org.springframework.http.HttpHeaders; +import org.springframework.lang.Nullable; +import org.springframework.util.MultiValueMap; + +/** + * {@code MultiValueMap} implementation for wrapping Jetty HTTP headers. + * + *

There is a duplicate of this class in the server package! + * + * @author Rossen Stoyanchev + * @since 5.3 + */ +class JettyHeadersAdapter implements MultiValueMap { + + private final HttpFields headers; + + + JettyHeadersAdapter(HttpFields headers) { + this.headers = headers; + } + + + @Override + public String getFirst(String key) { + return this.headers.get(key); + } + + @Override + public void add(String key, @Nullable String value) { + this.headers.add(key, value); + } + + @Override + public void addAll(String key, List values) { + values.forEach(value -> add(key, value)); + } + + @Override + public void addAll(MultiValueMap values) { + values.forEach(this::addAll); + } + + @Override + public void set(String key, @Nullable String value) { + this.headers.put(key, value); + } + + @Override + public void setAll(Map values) { + values.forEach(this::set); + } + + @Override + public Map toSingleValueMap() { + Map singleValueMap = new LinkedHashMap<>(this.headers.size()); + Iterator iterator = this.headers.iterator(); + iterator.forEachRemaining(field -> { + if (!singleValueMap.containsKey(field.getName())) { + singleValueMap.put(field.getName(), field.getValue()); + } + }); + return singleValueMap; + } + + @Override + public int size() { + return this.headers.getFieldNamesCollection().size(); + } + + @Override + public boolean isEmpty() { + return (this.headers.size() == 0); + } + + @Override + public boolean containsKey(Object key) { + return (key instanceof String && this.headers.containsKey((String) key)); + } + + @Override + public boolean containsValue(Object value) { + return (value instanceof String && + this.headers.stream().anyMatch(field -> field.contains((String) value))); + } + + @Nullable + @Override + public List get(Object key) { + if (containsKey(key)) { + return this.headers.getValuesList((String) key); + } + return null; + } + + @Nullable + @Override + public List put(String key, List value) { + List oldValues = get(key); + this.headers.put(key, value); + return oldValues; + } + + @Nullable + @Override + public List remove(Object key) { + if (key instanceof String) { + List oldValues = get(key); + this.headers.remove((String) key); + return oldValues; + } + return null; + } + + @Override + public void putAll(Map> map) { + map.forEach(this::put); + } + + @Override + public void clear() { + this.headers.clear(); + } + + @Override + public Set keySet() { + return this.headers.getFieldNamesCollection(); + } + + @Override + public Collection> values() { + return this.headers.getFieldNamesCollection().stream() + .map(this.headers::getValuesList).collect(Collectors.toList()); + } + + @Override + public Set>> entrySet() { + return new AbstractSet>>() { + @Override + public Iterator>> iterator() { + return new EntryIterator(); + } + + @Override + public int size() { + return headers.size(); + } + }; + } + + + @Override + public String toString() { + return HttpHeaders.formatHeaders(this); + } + + + private class EntryIterator implements Iterator>> { + + private Enumeration names = headers.getFieldNames(); + + @Override + public boolean hasNext() { + return this.names.hasMoreElements(); + } + + @Override + public Entry> next() { + return new HeaderEntry(this.names.nextElement()); + } + } + + + private class HeaderEntry implements Entry> { + + private final String key; + + HeaderEntry(String key) { + this.key = key; + } + + @Override + public String getKey() { + return this.key; + } + + @Override + public List getValue() { + return headers.getValuesList(this.key); + } + + @Override + public List setValue(List value) { + List previousValues = headers.getValuesList(this.key); + headers.put(this.key, value); + return previousValues; + } + } + +} diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/NettyHeadersAdapter.java b/spring-web/src/main/java/org/springframework/http/client/reactive/NettyHeadersAdapter.java new file mode 100644 index 0000000000..811d72837a --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/NettyHeadersAdapter.java @@ -0,0 +1,229 @@ +/* + * Copyright 2002-2018 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. + * 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 org.springframework.http.client.reactive; + +import java.util.AbstractSet; +import java.util.Collection; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +import io.netty.handler.codec.http.HttpHeaders; + +import org.springframework.lang.Nullable; +import org.springframework.util.MultiValueMap; + +/** + * {@code MultiValueMap} implementation for wrapping Netty HTTP headers. + * + *

There is a duplicate of this class in the server package! + * + * @author Rossen Stoyanchev + * @since 5.3 + */ +class NettyHeadersAdapter implements MultiValueMap { + + private final HttpHeaders headers; + + + NettyHeadersAdapter(HttpHeaders headers) { + this.headers = headers; + } + + + @Override + @Nullable + public String getFirst(String key) { + return this.headers.get(key); + } + + @Override + public void add(String key, @Nullable String value) { + this.headers.add(key, value); + } + + @Override + public void addAll(String key, List values) { + this.headers.add(key, values); + } + + @Override + public void addAll(MultiValueMap values) { + values.forEach(this.headers::add); + } + + @Override + public void set(String key, @Nullable String value) { + this.headers.set(key, value); + } + + @Override + public void setAll(Map values) { + values.forEach(this.headers::set); + } + + @Override + public Map toSingleValueMap() { + Map singleValueMap = new LinkedHashMap<>(this.headers.size()); + this.headers.entries() + .forEach(entry -> { + if (!singleValueMap.containsKey(entry.getKey())) { + singleValueMap.put(entry.getKey(), entry.getValue()); + } + }); + return singleValueMap; + } + + @Override + public int size() { + return this.headers.names().size(); + } + + @Override + public boolean isEmpty() { + return this.headers.isEmpty(); + } + + @Override + public boolean containsKey(Object key) { + return (key instanceof String && this.headers.contains((String) key)); + } + + @Override + public boolean containsValue(Object value) { + return (value instanceof String && + this.headers.entries().stream() + .anyMatch(entry -> value.equals(entry.getValue()))); + } + + @Override + @Nullable + public List get(Object key) { + if (containsKey(key)) { + return this.headers.getAll((String) key); + } + return null; + } + + @Nullable + @Override + public List put(String key, @Nullable List value) { + List previousValues = this.headers.getAll(key); + this.headers.set(key, value); + return previousValues; + } + + @Nullable + @Override + public List remove(Object key) { + if (key instanceof String) { + List previousValues = this.headers.getAll((String) key); + this.headers.remove((String) key); + return previousValues; + } + return null; + } + + @Override + public void putAll(Map> map) { + map.forEach(this.headers::add); + } + + @Override + public void clear() { + this.headers.clear(); + } + + @Override + public Set keySet() { + return this.headers.names(); + } + + @Override + public Collection> values() { + return this.headers.names().stream() + .map(this.headers::getAll).collect(Collectors.toList()); + } + + @Override + public Set>> entrySet() { + return new AbstractSet>>() { + @Override + public Iterator>> iterator() { + return new EntryIterator(); + } + + @Override + public int size() { + return headers.size(); + } + }; + } + + + @Override + public String toString() { + return org.springframework.http.HttpHeaders.formatHeaders(this); + } + + + private class EntryIterator implements Iterator>> { + + private Iterator names = headers.names().iterator(); + + @Override + public boolean hasNext() { + return this.names.hasNext(); + } + + @Override + public Entry> next() { + return new HeaderEntry(this.names.next()); + } + } + + + private class HeaderEntry implements Entry> { + + private final String key; + + HeaderEntry(String key) { + this.key = key; + } + + @Override + public String getKey() { + return this.key; + } + + @Override + public List getValue() { + return headers.getAll(this.key); + } + + @Override + public List setValue(List value) { + List previousValues = headers.getAll(this.key); + headers.set(this.key, value); + return previousValues; + } + } + +} diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpResponse.java index d8a1ef0398..00a6a0dc3d 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpResponse.java @@ -42,12 +42,14 @@ import org.springframework.util.MultiValueMap; */ class ReactorClientHttpResponse implements ClientHttpResponse { - private final NettyDataBufferFactory bufferFactory; - private final HttpClientResponse response; private final NettyInbound inbound; + private final NettyDataBufferFactory bufferFactory; + + private final HttpHeaders headers; + private final AtomicBoolean rejectSubscribers = new AtomicBoolean(); @@ -55,6 +57,9 @@ class ReactorClientHttpResponse implements ClientHttpResponse { this.response = response; this.inbound = inbound; this.bufferFactory = new NettyDataBufferFactory(alloc); + + MultiValueMap adapter = new NettyHeadersAdapter(response.responseHeaders()); + this.headers = HttpHeaders.readOnlyHttpHeaders(adapter); } @@ -81,9 +86,7 @@ class ReactorClientHttpResponse implements ClientHttpResponse { @Override public HttpHeaders getHeaders() { - HttpHeaders headers = new HttpHeaders(); - this.response.responseHeaders().entries().forEach(e -> headers.add(e.getKey(), e.getValue())); - return headers; + return this.headers; } @Override diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/JettyHeadersAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/JettyHeadersAdapter.java index 198e549bb6..98fd9db212 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/JettyHeadersAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/JettyHeadersAdapter.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,8 @@ import org.springframework.util.MultiValueMap; /** * {@code MultiValueMap} implementation for wrapping Jetty HTTP headers. * + *

There is a duplicate of this class in the client package! + * * @author Brian Clozel * @since 5.1.1 */ diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/NettyHeadersAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/NettyHeadersAdapter.java index c32cb5426f..d69795b086 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/NettyHeadersAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/NettyHeadersAdapter.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. @@ -33,6 +33,8 @@ import org.springframework.util.MultiValueMap; /** * {@code MultiValueMap} implementation for wrapping Netty HTTP headers. * + *

There is a duplicate of this class in the client package! + * * @author Brian Clozel * @since 5.1.1 */