From 9b3cb15389b3efe225f802aba724256bf19583c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Tue, 22 Oct 2024 16:59:03 +0200 Subject: [PATCH] Introduce HttpHeaders#headerSet to guarantee case-insensitive iteration The `HttpHeaders#headerSet` method is intended as a drop-in replacement for `entrySet` that guarantees a single casing for all header names reported during the iteration, as the cost of some overhead but with support for iterator removal and entry value-setting. The `formatHeaders` static method is also altered to do a similar deduplication of casing variants, but now additionally mentions "with native header names [native name set]" if the native name set contains casing variants. Closes gh-33823 --- .../http/support/HeadersAdapterBenchmark.java | 124 ++ .../http/support/HeadersAdaptersBaseline.java | 1044 +++++++++++++++++ .../org/springframework/http/HttpHeaders.java | 208 +++- .../server/reactive/TomcatHeadersAdapter.java | 24 +- .../support/HttpComponentsHeadersAdapter.java | 86 +- .../http/support/JettyHeadersAdapter.java | 9 +- .../http/support/Netty4HeadersAdapter.java | 7 +- .../http/support/Netty5HeadersAdapter.java | 7 +- .../server/reactive/HeadersAdaptersTests.java | 166 ++- 9 files changed, 1623 insertions(+), 52 deletions(-) create mode 100644 spring-web/src/jmh/java/org/springframework/http/support/HeadersAdapterBenchmark.java create mode 100644 spring-web/src/jmh/java/org/springframework/http/support/HeadersAdaptersBaseline.java diff --git a/spring-web/src/jmh/java/org/springframework/http/support/HeadersAdapterBenchmark.java b/spring-web/src/jmh/java/org/springframework/http/support/HeadersAdapterBenchmark.java new file mode 100644 index 0000000000..e2b3946cf3 --- /dev/null +++ b/spring-web/src/jmh/java/org/springframework/http/support/HeadersAdapterBenchmark.java @@ -0,0 +1,124 @@ +/* + * Copyright 2002-2024 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.support; + +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; + +import io.netty.handler.codec.http.DefaultHttpHeaders; +import org.apache.hc.client5.http.classic.methods.HttpGet; +import org.eclipse.jetty.http.HttpFields; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.infra.Blackhole; + +import org.springframework.http.HttpHeaders; +import org.springframework.util.MultiValueMap; + +/** + * Benchmark for implementations of MultiValueMap adapters over native HTTP + * headers implementations. + *

Run JMH with {@code -p implementation=Netty,Netty5,HttpComponents,Jetty} + * to cover all implementations + * @author Simon Baslé + */ +@BenchmarkMode(Mode.Throughput) +public class HeadersAdapterBenchmark { + + @Benchmark + public void iterateEntries(BenchmarkData data, Blackhole bh) { + for (Map.Entry> entry : data.entriesProvider.apply(data.headers)) { + bh.consume(entry.getKey()); + for (String s : entry.getValue()) { + bh.consume(s); + } + } + } + + @Benchmark + public void toString(BenchmarkData data, Blackhole bh) { + bh.consume(data.headers.toString()); + } + + @State(Scope.Benchmark) + public static class BenchmarkData { + + @Param({"NONE"}) + public String implementation; + + @Param({"true"}) + public boolean duplicate; + + public MultiValueMap headers; + public Function, Set>>> entriesProvider; + + //Uncomment the following line and comment the similar line for setupImplementationBaseline below + //to benchmark current implementations + @Setup(Level.Trial) + public void initImplementationNew() { + this.entriesProvider = map -> new HttpHeaders(map).headerSet(); + + this.headers = switch (this.implementation) { + case "Netty" -> new Netty4HeadersAdapter(new DefaultHttpHeaders()); + case "HttpComponents" -> new HttpComponentsHeadersAdapter(new HttpGet("https://example.com")); + case "Netty5" -> new Netty5HeadersAdapter(io.netty5.handler.codec.http.headers.HttpHeaders.newHeaders()); + case "Jetty" -> new JettyHeadersAdapter(HttpFields.build()); + //FIXME tomcat/undertow implementations (in another package) +// case "Tomcat" -> new TomcatHeadersAdapter(new MimeHeaders()); +// case "Undertow" -> new UndertowHeadersAdapter(new HeaderMap()); + default -> throw new IllegalArgumentException("Unsupported implementation: " + this.implementation); + }; + initHeaders(); + } + + //Uncomment the following line and comment the similar line for setupImplementationNew above + //to benchmark old implementations +// @Setup(Level.Trial) + public void setupImplementationBaseline() { + this.entriesProvider = MultiValueMap::entrySet; + + this.headers = switch (this.implementation) { + case "Netty" -> new HeadersAdaptersBaseline.Netty4(new DefaultHttpHeaders()); + case "HttpComponents" -> new HeadersAdaptersBaseline.HttpComponents(new HttpGet("https://example.com")); + case "Netty5" -> new HeadersAdaptersBaseline.Netty5(io.netty5.handler.codec.http.headers.HttpHeaders.newHeaders()); + case "Jetty" -> new HeadersAdaptersBaseline.Jetty(HttpFields.build()); + default -> throw new IllegalArgumentException("Unsupported implementation: " + this.implementation); + }; + initHeaders(); + } + + private void initHeaders() { + this.headers.add("TestHeader", "first"); + this.headers.add("SecondHeader", "value"); + if (this.duplicate) { + this.headers.add("TestHEADER", "second"); + } + else { + this.headers.add("TestHeader", "second"); + } + this.headers.add("TestHeader", "third"); + } + } +} diff --git a/spring-web/src/jmh/java/org/springframework/http/support/HeadersAdaptersBaseline.java b/spring-web/src/jmh/java/org/springframework/http/support/HeadersAdaptersBaseline.java new file mode 100644 index 0000000000..e412ffd74d --- /dev/null +++ b/spring-web/src/jmh/java/org/springframework/http/support/HeadersAdaptersBaseline.java @@ -0,0 +1,1044 @@ +/* + * Copyright 2002-2024 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.support; + +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.LinkedHashSet; +import java.util.List; +import java.util.ListIterator; +import java.util.Map; +import java.util.Set; +import java.util.stream.StreamSupport; + +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpMessage; +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.Assert; +import org.springframework.util.CollectionUtils; +import org.springframework.util.MultiValueMap; + +class HeadersAdaptersBaseline { + + static final class HttpComponents implements MultiValueMap { + + private final HttpMessage message; + + + /** + * Create a new {@code HttpComponentsHeadersAdapter} based on the given + * {@code HttpMessage}. + */ + public HttpComponents(HttpMessage message) { + Assert.notNull(message, "Message must not be null"); + this.message = message; + } + + + @Override + @Nullable + public String getFirst(String key) { + Header header = this.message.getFirstHeader(key); + return (header != null ? header.getValue() : null); + } + + @Override + public void add(String key, @Nullable String value) { + this.message.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.message.setHeader(key, value); + } + + @Override + public void setAll(Map values) { + values.forEach(this::set); + } + + @Override + public Map toSingleValueMap() { + Map map = CollectionUtils.newLinkedHashMap(size()); + this.message.headerIterator().forEachRemaining(h -> map.putIfAbsent(h.getName(), h.getValue())); + return map; + } + + @Override + public int size() { + return this.message.getHeaders().length; + } + + @Override + public boolean isEmpty() { + return (this.message.getHeaders().length == 0); + } + + @Override + public boolean containsKey(Object key) { + return (key instanceof String headerName && this.message.containsHeader(headerName)); + } + + @Override + public boolean containsValue(Object value) { + return (value instanceof String && + Arrays.stream(this.message.getHeaders()).anyMatch(h -> h.getValue().equals(value))); + } + + @Nullable + @Override + public List get(Object key) { + List values = null; + if (containsKey(key)) { + Header[] headers = this.message.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 headerName) { + List oldValues = get(key); + this.message.removeHeaders(headerName); + return oldValues; + } + return null; + } + + @Override + public void putAll(Map> map) { + map.forEach(this::put); + } + + @Override + public void clear() { + this.message.setHeaders(); + } + + @Override + public Set keySet() { + Set keys = new LinkedHashSet<>(size()); + for (Header header : this.message.getHeaders()) { + keys.add(header.getName()); + } + return keys; + } + + @Override + public Collection> values() { + Collection> values = new ArrayList<>(size()); + for (Header header : this.message.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 HttpComponents.this.size(); + } + }; + } + + + @Override + public String toString() { + return HttpHeaders.formatHeaders(this); + } + + + private class EntryIterator implements Iterator>> { + + private final Iterator

iterator = message.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 = HttpComponents.this.get(this.key); + return values != null ? values : Collections.emptyList(); + } + + @Override + public List setValue(List value) { + List previousValues = getValue(); + HttpComponents.this.put(this.key, value); + return previousValues; + } + } + + } + + static final class Jetty implements MultiValueMap { + + private final HttpFields headers; + + @Nullable + private final HttpFields.Mutable mutable; + + + /** + * Creates a new {@code JettyHeadersAdapter} based on the given + * {@code HttpFields} instance. + * @param headers the {@code HttpFields} to base this adapter on + */ + public Jetty(HttpFields headers) { + Assert.notNull(headers, "Headers must not be null"); + this.headers = headers; + this.mutable = headers instanceof HttpFields.Mutable m ? m : null; + } + + + @Override + public String getFirst(String key) { + return this.headers.get(key); + } + + @Override + public void add(String key, @Nullable String value) { + if (value != null) { + HttpFields.Mutable mutableHttpFields = mutableFields(); + mutableHttpFields.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) { + HttpFields.Mutable mutableHttpFields = mutableFields(); + if (value != null) { + mutableHttpFields.put(key, value); + } + else { + mutableHttpFields.remove(key); + } + } + + @Override + public void setAll(Map values) { + values.forEach(this::set); + } + + @Override + public Map toSingleValueMap() { + Map singleValueMap = CollectionUtils.newLinkedHashMap(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 name && this.headers.contains(name)); + } + + @Override + public boolean containsValue(Object value) { + if (value instanceof String searchString) { + for (HttpField field : this.headers) { + if (field.contains(searchString)) { + return true; + } + } + } + return false; + } + + @Nullable + @Override + public List get(Object key) { + List list = null; + if (key instanceof String name) { + for (HttpField f : this.headers) { + if (f.is(name)) { + if (list == null) { + list = new ArrayList<>(); + } + list.add(f.getValue()); + } + } + } + return list; + } + + @Nullable + @Override + public List put(String key, List value) { + HttpFields.Mutable mutableHttpFields = mutableFields(); + List oldValues = get(key); + + if (oldValues == null) { + switch (value.size()) { + case 0 -> {} + case 1 -> mutableHttpFields.add(key, value.get(0)); + default -> mutableHttpFields.add(key, value); + } + } + else { + switch (value.size()) { + case 0 -> mutableHttpFields.remove(key); + case 1 -> mutableHttpFields.put(key, value.get(0)); + default -> mutableHttpFields.put(key, value); + } + } + return oldValues; + } + + @Nullable + @Override + public List remove(Object key) { + HttpFields.Mutable mutableHttpFields = mutableFields(); + List list = null; + if (key instanceof String name) { + for (ListIterator i = mutableHttpFields.listIterator(); i.hasNext(); ) { + HttpField f = i.next(); + if (f.is(name)) { + if (list == null) { + list = new ArrayList<>(); + } + list.add(f.getValue()); + i.remove(); + } + } + } + return list; + } + + @Override + public void putAll(Map> map) { + map.forEach(this::put); + } + + @Override + public void clear() { + HttpFields.Mutable mutableHttpFields = mutableFields(); + mutableHttpFields.clear(); + } + + @Override + public Set keySet() { + return new HeaderNames(); + } + + @Override + public Collection> values() { + return this.headers.getFieldNamesCollection().stream() + .map(this.headers::getValuesList).toList(); + } + + @Override + public Set>> entrySet() { + return new AbstractSet<>() { + @Override + public Iterator>> iterator() { + return new EntryIterator(); + } + + @Override + public int size() { + return headers.size(); + } + }; + } + + private HttpFields.Mutable mutableFields() { + if (this.mutable == null) { + throw new IllegalStateException("Immutable headers"); + } + return this.mutable; + } + + @Override + public String toString() { + return HttpHeaders.formatHeaders(this); + } + + + private class EntryIterator implements Iterator>> { + + private final Iterator names = headers.getFieldNamesCollection().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.getValuesList(this.key); + } + + @Override + public List setValue(List value) { + HttpFields.Mutable mutableHttpFields = mutableFields(); + List previousValues = headers.getValuesList(this.key); + mutableHttpFields.put(this.key, value); + return previousValues; + } + } + + + private class HeaderNames extends AbstractSet { + + @Override + public Iterator iterator() { + return new HeaderNamesIterator(headers.getFieldNamesCollection().iterator()); + } + + @Override + public int size() { + return headers.getFieldNamesCollection().size(); + } + } + + + private final class HeaderNamesIterator implements Iterator { + + private final Iterator iterator; + + @Nullable + private String currentName; + + private HeaderNamesIterator(Iterator iterator) { + this.iterator = iterator; + } + + @Override + public boolean hasNext() { + return this.iterator.hasNext(); + } + + @Override + public String next() { + this.currentName = this.iterator.next(); + return this.currentName; + } + + @Override + public void remove() { + HttpFields.Mutable mutableHttpFields = mutableFields(); + if (this.currentName == null) { + throw new IllegalStateException("No current Header in iterator"); + } + if (!headers.contains(this.currentName)) { + throw new IllegalStateException("Header not present: " + this.currentName); + } + mutableHttpFields.remove(this.currentName); + } + } + + } + + static final class Netty4 implements MultiValueMap { + + private final io.netty.handler.codec.http.HttpHeaders headers; + + /** + * Creates a new {@code Netty4HeadersAdapter} based on the given + * {@code HttpHeaders}. + */ + public Netty4(io.netty.handler.codec.http.HttpHeaders headers) { + Assert.notNull(headers, "Headers must not be null"); + this.headers = headers; + } + + + @Override + @Nullable + public String getFirst(String key) { + return this.headers.get(key); + } + + @Override + public void add(String key, @Nullable String value) { + if (value != null) { + 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) { + if (value != null) { + this.headers.set(key, value); + } + } + + @Override + public void setAll(Map values) { + values.forEach(this.headers::set); + } + + @Override + public Map toSingleValueMap() { + Map singleValueMap = CollectionUtils.newLinkedHashMap(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 headerName && this.headers.contains(headerName)); + } + + @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 headerName) { + List previousValues = this.headers.getAll(headerName); + this.headers.remove(headerName); + return previousValues; + } + return null; + } + + @Override + public void putAll(Map> map) { + map.forEach(this.headers::set); + } + + @Override + public void clear() { + this.headers.clear(); + } + + @Override + public Set keySet() { + return new HeaderNames(); + } + + @Override + public Collection> values() { + return this.headers.names().stream() + .map(this.headers::getAll).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 final 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; + } + } + + + private class HeaderNames extends AbstractSet { + + @Override + public Iterator iterator() { + return new HeaderNamesIterator(headers.names().iterator()); + } + + @Override + public int size() { + return headers.names().size(); + } + } + + private final class HeaderNamesIterator implements Iterator { + + private final Iterator iterator; + + @Nullable + private String currentName; + + private HeaderNamesIterator(Iterator iterator) { + this.iterator = iterator; + } + + @Override + public boolean hasNext() { + return this.iterator.hasNext(); + } + + @Override + public String next() { + this.currentName = this.iterator.next(); + return this.currentName; + } + + @Override + public void remove() { + if (this.currentName == null) { + throw new IllegalStateException("No current Header in iterator"); + } + if (!headers.contains(this.currentName)) { + throw new IllegalStateException("Header not present: " + this.currentName); + } + headers.remove(this.currentName); + } + } + + } + + static final class Netty5 implements MultiValueMap { + + private final io.netty5.handler.codec.http.headers.HttpHeaders headers; + + + /** + * Create a new {@code Netty5HeadersAdapter} based on the given + * {@code HttpHeaders}. + */ + public Netty5(io.netty5.handler.codec.http.headers.HttpHeaders headers) { + Assert.notNull(headers, "Headers must not be null"); + this.headers = headers; + } + + + @Override + @Nullable + public String getFirst(String key) { + CharSequence value = this.headers.get(key); + return (value != null ? value.toString() : null); + } + + @Override + public void add(String key, @Nullable String value) { + if (value != null) { + 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) { + if (value != null) { + this.headers.set(key, value); + } + } + + @Override + public void setAll(Map values) { + values.forEach(this.headers::set); + } + + @Override + public Map toSingleValueMap() { + Map singleValueMap = CollectionUtils.newLinkedHashMap(this.headers.size()); + this.headers.forEach(entry -> singleValueMap.putIfAbsent( + entry.getKey().toString(), entry.getValue().toString())); + 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 headerName && this.headers.contains(headerName)); + } + + @Override + public boolean containsValue(Object value) { + return (value instanceof String && + StreamSupport.stream(this.headers.spliterator(), false) + .anyMatch(entry -> value.equals(entry.getValue()))); + } + + @Override + @Nullable + public List get(Object key) { + Iterator iterator = this.headers.valuesIterator((CharSequence) key); + if (iterator.hasNext()) { + List result = new ArrayList<>(); + iterator.forEachRemaining(value -> result.add(value.toString())); + return result; + } + return null; + } + + @Nullable + @Override + public List put(String key, @Nullable List value) { + List previousValues = get(key); + this.headers.set(key, value); + return previousValues; + } + + @Nullable + @Override + public List remove(Object key) { + if (key instanceof String headerName) { + List previousValues = get(headerName); + this.headers.remove(headerName); + return previousValues; + } + return null; + } + + @Override + public void putAll(Map> map) { + map.forEach(this.headers::set); + } + + @Override + public void clear() { + this.headers.clear(); + } + + @Override + public Set keySet() { + return new HeaderNames(); + } + + @Override + public Collection> values() { + List> result = new ArrayList<>(this.headers.size()); + forEach((key, value) -> result.add(value)); + return result; + } + + @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 final 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 CharSequence key; + + HeaderEntry(CharSequence key) { + this.key = key; + } + + @Override + public String getKey() { + return this.key.toString(); + } + + @Override + public List getValue() { + List values = get(this.key); + return (values != null ? values : Collections.emptyList()); + } + + @Override + public List setValue(List value) { + List previousValues = getValue(); + headers.set(this.key, value); + return previousValues; + } + } + + private class HeaderNames extends AbstractSet { + + @Override + public Iterator iterator() { + return new HeaderNamesIterator(headers.names().iterator()); + } + + @Override + public int size() { + return headers.names().size(); + } + } + + private final class HeaderNamesIterator implements Iterator { + + private final Iterator iterator; + + @Nullable + private CharSequence currentName; + + private HeaderNamesIterator(Iterator iterator) { + this.iterator = iterator; + } + + @Override + public boolean hasNext() { + return this.iterator.hasNext(); + } + + @Override + public String next() { + this.currentName = this.iterator.next(); + return this.currentName.toString(); + } + + @Override + public void remove() { + if (this.currentName == null) { + throw new IllegalStateException("No current Header in iterator"); + } + if (!headers.contains(this.currentName)) { + throw new IllegalStateException("Header not present: " + this.currentName); + } + headers.remove(this.currentName); + } + } + + } +} diff --git a/spring-web/src/main/java/org/springframework/http/HttpHeaders.java b/spring-web/src/main/java/org/springframework/http/HttpHeaders.java index 0c7e1d4260..d7e07d52c7 100644 --- a/spring-web/src/main/java/org/springframework/http/HttpHeaders.java +++ b/spring-web/src/main/java/org/springframework/http/HttpHeaders.java @@ -30,14 +30,17 @@ import java.time.ZoneId; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; import java.time.format.DateTimeParseException; +import java.util.AbstractSet; import java.util.ArrayList; import java.util.Base64; import java.util.Collection; import java.util.Collections; +import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.StringJoiner; import java.util.function.BiConsumer; @@ -63,7 +66,23 @@ import org.springframework.util.StringUtils; *
  • {@link #set(String, String)} sets the header value to a single string value
    • * * - *

    Note that {@code HttpHeaders} generally treats header names in a case-insensitive manner. + *

    Note that {@code HttpHeaders} instances created by the default constructor + * treat header names in a case-insensitive manner. Instances created with the + * {@link #HttpHeaders(MultiValueMap)} constructor like those instantiated + * internally by the framework to adapt to existing HTTP headers data structures + * do guarantee per-header get/set/add operations to be case-insensitive as + * mandated by the HTTP specification. However, it is not necessarily the case + * for operations that deal with the collection as a whole (like {@code size()}, + * {@code values()}, {@code keySet()} and {@code entrySet()}). Prefer using + * {@link #headerSet()} for these cases. + * + *

    Some backing implementations can store header names in a case-sensitive + * manner, which will lead to duplicates during the entrySet() iteration where + * multiple occurrences of a header name can surface depending on letter casing + * but each such entry has the full {@code List} of values. — This can be + * problematic for example when copying headers into a new instance by iterating + * over the old instance's {@code entrySet()} and using + * {@link #addAll(String, List)} rather than {@link #put(String, List)}. * * @author Arjen Poutsma * @author Sebastien Deleuze @@ -71,6 +90,7 @@ import org.springframework.util.StringUtils; * @author Juergen Hoeller * @author Josh Long * @author Sam Brannen + * @author Simon Baslé * @since 3.0 */ public class HttpHeaders implements MultiValueMap, Serializable { @@ -418,8 +438,8 @@ public class HttpHeaders implements MultiValueMap, Serializable /** - * Construct a new, empty instance of the {@code HttpHeaders} object. - *

    This is the common constructor, using a case-insensitive map structure. + * Construct a new, empty instance of the {@code HttpHeaders} object + * using an underlying case-insensitive map. */ public HttpHeaders() { this(CollectionUtils.toMultiValueMap(new LinkedCaseInsensitiveMap<>(8, Locale.ROOT))); @@ -1748,11 +1768,6 @@ public class HttpHeaders implements MultiValueMap, Serializable // Map implementation - @Override - public int size() { - return this.headers.size(); - } - @Override public boolean isEmpty() { return this.headers.isEmpty(); @@ -1794,29 +1809,81 @@ public class HttpHeaders implements MultiValueMap, Serializable this.headers.clear(); } + @Override + public List putIfAbsent(String key, List value) { + return this.headers.putIfAbsent(key, value); + } + + // Map/MultiValueMap methods that can have duplicate header names: size/keySet/values/entrySet/forEach + + /** + * Return the number of headers in the collection. This can be inflated, + * see {@link HttpHeaders class level javadoc}. + */ + @Override + public int size() { + return this.headers.size(); + } + + /** + * Return a {@link Set} view of header names. This can include multiple + * casing variants of a given header name, see + * {@link HttpHeaders class level javadoc}. + */ @Override public Set keySet() { return this.headers.keySet(); } + /** + * Return a {@link Collection} view of all the header values, reconstructed + * from iterating over the {@link #keySet()}. This can include duplicates if + * multiple casing variants of a given header name are tracked, see + * {@link HttpHeaders class level javadoc}. + */ @Override public Collection> values() { return this.headers.values(); } + /** + * Return a {@link Set} views of header entries, reconstructed from + * iterating over the {@link #keySet()}. This can include duplicate entries + * if multiple casing variants of a given header name are tracked, see + * {@link HttpHeaders class level javadoc}. + * @see #headerSet() + */ @Override public Set>> entrySet() { return this.headers.entrySet(); } + /** + * Perform an action over each header, as when iterated via + * {@link #entrySet()}. This can include duplicate entries + * if multiple casing variants of a given header name are tracked, see + * {@link HttpHeaders class level javadoc}. + * @param action the action to be performed for each entry + */ @Override public void forEach(BiConsumer> action) { this.headers.forEach(action); } - @Override - public List putIfAbsent(String key, List value) { - return this.headers.putIfAbsent(key, value); + /** + * Return a view of the headers as an entry {@code Set} of key-list pairs. + * Both {@link Iterator#remove()} and {@link java.util.Map.Entry#setValue} + * are supported and mutate the headers. + *

    This collection is guaranteed to contain one entry per header name + * even if the backing structure stores multiple casing variants of names, + * at the cost of first copying the names into a case-insensitive set for + * filtering the iteration. + * @return a {@code Set} view that iterates over all headers in a + * case-insensitive manner + * @since 6.1.15 + */ + public Set>> headerSet() { + return new CaseInsensitiveEntrySet(this.headers); } @@ -1882,19 +1949,29 @@ public class HttpHeaders implements MultiValueMap, Serializable * Helps to format HTTP header values, as HTTP header values themselves can * contain comma-separated values, can become confusing with regular * {@link Map} formatting that also uses commas between entries. + *

    Additionally, this method displays the native list of header names + * with the mention {@code with native header names} if the underlying + * implementation stores multiple casing variants of header names (see + * {@link HttpHeaders class level javadoc}). * @param headers the headers to format * @return the headers to a String * @since 5.1.4 */ public static String formatHeaders(MultiValueMap headers) { - return headers.entrySet().stream() - .map(entry -> { - List values = entry.getValue(); - return entry.getKey() + ":" + (values.size() == 1 ? + Set headerNames = toCaseInsensitiveSet(headers.keySet()); + String suffix = "]"; + if (headerNames.size() != headers.size()) { + suffix = "] with native header names " + headers.keySet(); + } + + return headerNames.stream() + .map(headerName -> { + List values = headers.get(headerName); + return headerName + ":" + (values.size() == 1 ? "\"" + values.get(0) + "\"" : values.stream().map(s -> "\"" + s + "\"").collect(Collectors.joining(", "))); }) - .collect(Collectors.joining(", ", "[", "]")); + .collect(Collectors.joining(", ", "[", suffix)); } /** @@ -1947,4 +2024,103 @@ public class HttpHeaders implements MultiValueMap, Serializable return DATE_FORMATTER.format(time); } + + private static Set toCaseInsensitiveSet(Set originalSet) { + final Set deduplicatedSet = Collections.newSetFromMap( + new LinkedCaseInsensitiveMap<>(originalSet.size(), Locale.ROOT)); + // add/addAll (put/putAll in LinkedCaseInsensitiveMap) retain the casing of the last occurrence. + // Here we prefer the first. + for (String header : originalSet) { + //noinspection RedundantCollectionOperation + if (!deduplicatedSet.contains(header)) { + deduplicatedSet.add(header); + } + } + return deduplicatedSet; + } + + + private static final class CaseInsensitiveEntrySet extends AbstractSet>> { + + private final MultiValueMap headers; + private final Set deduplicatedNames; + + public CaseInsensitiveEntrySet(MultiValueMap headers) { + this.headers = headers; + this.deduplicatedNames = toCaseInsensitiveSet(headers.keySet()); + } + + @Override + public Iterator>> iterator() { + return new CaseInsensitiveIterator(this.deduplicatedNames.iterator()); + } + + @Override + public int size() { + return this.deduplicatedNames.size(); + } + + private final class CaseInsensitiveIterator implements Iterator>> { + + private final Iterator namesIterator; + + @Nullable + private String currentName; + + private CaseInsensitiveIterator(Iterator namesIterator) { + this.namesIterator = namesIterator; + this.currentName = null; + } + + @Override + public boolean hasNext() { + return this.namesIterator.hasNext(); + } + + @Override + public Map.Entry> next() { + this.currentName = this.namesIterator.next(); + return new CaseInsensitiveEntry(this.currentName); + } + + @Override + public void remove() { + if (this.currentName == null) { + throw new IllegalStateException("No current Header in iterator"); + } + if (!CaseInsensitiveEntrySet.this.headers.containsKey(this.currentName)) { + throw new IllegalStateException("Header not present: " + this.currentName); + } + CaseInsensitiveEntrySet.this.headers.remove(this.currentName); + } + } + + private final class CaseInsensitiveEntry implements Map.Entry> { + + private final String key; + + CaseInsensitiveEntry(String key) { + this.key = key; + } + + @Override + public String getKey() { + return this.key; + } + + @Override + public List getValue() { + return Objects.requireNonNull(CaseInsensitiveEntrySet.this.headers.get(this.key)); + } + + @Override + public List setValue(List value) { + List previousValues = Objects.requireNonNull( + CaseInsensitiveEntrySet.this.headers.get(this.key)); + CaseInsensitiveEntrySet.this.headers.put(this.key, value); + return previousValues; + } + } + } + } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHeadersAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHeadersAdapter.java index 426cd5d275..4a18b40981 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHeadersAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHeadersAdapter.java @@ -21,7 +21,9 @@ import java.util.Collection; import java.util.Collections; import java.util.Enumeration; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; @@ -38,6 +40,7 @@ import org.springframework.util.MultiValueMap; * * @author Brian Clozel * @author Sam Brannen + * @author Simon Baslé * @since 5.1.1 */ class TomcatHeadersAdapter implements MultiValueMap { @@ -90,12 +93,11 @@ class TomcatHeadersAdapter implements MultiValueMap { @Override public int size() { Enumeration names = this.headers.names(); - int size = 0; + Set deduplicated = new LinkedHashSet<>(); while (names.hasMoreElements()) { - size++; - names.nextElement(); + deduplicated.add(names.nextElement().toLowerCase(Locale.ROOT)); } - return size; + return deduplicated.size(); } @Override @@ -185,7 +187,7 @@ class TomcatHeadersAdapter implements MultiValueMap { @Override public int size() { - return headers.size(); + return TomcatHeadersAdapter.this.size(); } }; } @@ -289,11 +291,17 @@ class TomcatHeadersAdapter implements MultiValueMap { if (this.currentName == null) { throw new IllegalStateException("No current Header in iterator"); } - int index = headers.findHeader(this.currentName, 0); - if (index == -1) { + //implement a mix of removeHeader(String) and removeHeader(int) + boolean found = false; + for (int i = 0; i < headers.size(); i++) { + if (headers.getName(i).equalsIgnoreCase(this.currentName)) { + headers.removeHeader(i--); + found = true; + } + } + if (!found) { throw new IllegalStateException("Header not present: " + this.currentName); } - headers.removeHeader(index); } } diff --git a/spring-web/src/main/java/org/springframework/http/support/HttpComponentsHeadersAdapter.java b/spring-web/src/main/java/org/springframework/http/support/HttpComponentsHeadersAdapter.java index 9fd80f6816..8b584c52c7 100644 --- a/spring-web/src/main/java/org/springframework/http/support/HttpComponentsHeadersAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/support/HttpComponentsHeadersAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -22,8 +22,8 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Iterator; -import java.util.LinkedHashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; @@ -33,7 +33,7 @@ import org.apache.hc.core5.http.HttpMessage; import org.springframework.http.HttpHeaders; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; +import org.springframework.util.LinkedCaseInsensitiveMap; import org.springframework.util.MultiValueMap; /** @@ -41,12 +41,13 @@ import org.springframework.util.MultiValueMap; * HttpClient headers. * * @author Rossen Stoyanchev + * @author Simon Baslé * @since 6.1 */ public final class HttpComponentsHeadersAdapter implements MultiValueMap { private final HttpMessage message; - + private final Set headerNames; /** * Create a new {@code HttpComponentsHeadersAdapter} based on the given @@ -55,6 +56,11 @@ public final class HttpComponentsHeadersAdapter implements MultiValueMap( + message.getHeaders().length, Locale.ROOT)); + for (Header header : message.getHeaders()) { + this.headerNames.add(header.getName()); + } } @@ -67,6 +73,7 @@ public final class HttpComponentsHeadersAdapter implements MultiValueMap toSingleValueMap() { - Map map = CollectionUtils.newLinkedHashMap(size()); + Map map = new LinkedCaseInsensitiveMap<>(this.message.getHeaders().length, Locale.ROOT); this.message.headerIterator().forEachRemaining(h -> map.putIfAbsent(h.getName(), h.getValue())); return map; } @Override public int size() { - return this.message.getHeaders().length; + return this.headerNames.size(); } @Override @@ -145,6 +153,7 @@ public final class HttpComponentsHeadersAdapter implements MultiValueMap remove(Object key) { if (key instanceof String headerName) { List oldValues = get(key); + this.headerNames.remove(headerName); this.message.removeHeaders(headerName); return oldValues; } @@ -158,23 +167,20 @@ public final class HttpComponentsHeadersAdapter implements MultiValueMap keySet() { - Set keys = new LinkedHashSet<>(size()); - for (Header header : this.message.getHeaders()) { - keys.add(header.getName()); - } - return keys; + return new HeaderNames(); } @Override public Collection> values() { - Collection> values = new ArrayList<>(size()); - for (Header header : this.message.getHeaders()) { - values.add(get(header.getName())); + Collection> values = new ArrayList<>(this.message.getHeaders().length); + for (String headerName : keySet()) { + values.add(get(headerName)); } return values; } @@ -200,10 +206,22 @@ public final class HttpComponentsHeadersAdapter implements MultiValueMap { + + @Override + public Iterator iterator() { + return new HeaderNamesIterator(headerNames.iterator()); + } + + @Override + public int size() { + return headerNames.size(); + } + } private class EntryIterator implements Iterator>> { - private final Iterator

    iterator = message.headerIterator(); + private final Iterator iterator = keySet().iterator(); @Override public boolean hasNext() { @@ -212,7 +230,7 @@ public final class HttpComponentsHeadersAdapter implements MultiValueMap> next() { - return new HeaderEntry(this.iterator.next().getName()); + return new HeaderEntry(this.iterator.next()); } } @@ -244,4 +262,40 @@ public final class HttpComponentsHeadersAdapter implements MultiValueMap { + + private final Iterator iterator; + + @Nullable + private String currentName; + + private HeaderNamesIterator(Iterator iterator) { + this.iterator = iterator; + } + + @Override + public boolean hasNext() { + return this.iterator.hasNext(); + } + + @Override + public String next() { + this.currentName = this.iterator.next(); + return this.currentName; + } + + @Override + public void remove() { + if (this.currentName == null) { + throw new IllegalStateException("No current Header in iterator"); + } + if (!message.containsHeader(this.currentName)) { + throw new IllegalStateException("Header not present: " + this.currentName); + } + headerNames.remove(this.currentName); + message.removeHeaders(this.currentName); + } + } + + } diff --git a/spring-web/src/main/java/org/springframework/http/support/JettyHeadersAdapter.java b/spring-web/src/main/java/org/springframework/http/support/JettyHeadersAdapter.java index 57f9b7ab89..921efebcee 100644 --- a/spring-web/src/main/java/org/springframework/http/support/JettyHeadersAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/support/JettyHeadersAdapter.java @@ -20,6 +20,7 @@ import java.util.AbstractSet; import java.util.Collection; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; @@ -29,7 +30,7 @@ import org.eclipse.jetty.http.HttpFields; import org.springframework.http.HttpHeaders; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; +import org.springframework.util.LinkedCaseInsensitiveMap; import org.springframework.util.MultiValueMap; /** @@ -38,6 +39,7 @@ import org.springframework.util.MultiValueMap; * @author Rossen Stoyanchev * @author Juergen Hoeller * @author Sam Brannen + * @author Simon Baslé * @since 6.1 */ public final class JettyHeadersAdapter implements MultiValueMap { @@ -97,7 +99,8 @@ public final class JettyHeadersAdapter implements MultiValueMap @Override public Map toSingleValueMap() { - Map singleValueMap = CollectionUtils.newLinkedHashMap(this.headers.size()); + Map singleValueMap = new LinkedCaseInsensitiveMap<>( + this.headers.size(), Locale.ROOT); Iterator iterator = this.headers.iterator(); iterator.forEachRemaining(field -> { if (!singleValueMap.containsKey(field.getName())) { @@ -189,7 +192,7 @@ public final class JettyHeadersAdapter implements MultiValueMap } @Override public int size() { - return headers.size(); + return headers.getFieldNamesCollection().size(); } }; } diff --git a/spring-web/src/main/java/org/springframework/http/support/Netty4HeadersAdapter.java b/spring-web/src/main/java/org/springframework/http/support/Netty4HeadersAdapter.java index 97e853d4b7..14bcff9889 100644 --- a/spring-web/src/main/java/org/springframework/http/support/Netty4HeadersAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/support/Netty4HeadersAdapter.java @@ -20,6 +20,7 @@ import java.util.AbstractSet; import java.util.Collection; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; @@ -27,7 +28,7 @@ import io.netty.handler.codec.http.HttpHeaders; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; +import org.springframework.util.LinkedCaseInsensitiveMap; import org.springframework.util.MultiValueMap; /** @@ -35,6 +36,7 @@ import org.springframework.util.MultiValueMap; * * @author Rossen Stoyanchev * @author Sam Brannen + * @author Simon Baslé * @since 6.1 */ public final class Netty4HeadersAdapter implements MultiValueMap { @@ -89,7 +91,8 @@ public final class Netty4HeadersAdapter implements MultiValueMap @Override public Map toSingleValueMap() { - Map singleValueMap = CollectionUtils.newLinkedHashMap(this.headers.size()); + Map singleValueMap = new LinkedCaseInsensitiveMap<>( + this.headers.size(), Locale.ROOT); this.headers.entries() .forEach(entry -> { if (!singleValueMap.containsKey(entry.getKey())) { diff --git a/spring-web/src/main/java/org/springframework/http/support/Netty5HeadersAdapter.java b/spring-web/src/main/java/org/springframework/http/support/Netty5HeadersAdapter.java index 26d7b7fd7c..869ed15111 100644 --- a/spring-web/src/main/java/org/springframework/http/support/Netty5HeadersAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/support/Netty5HeadersAdapter.java @@ -22,6 +22,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.stream.StreamSupport; @@ -30,13 +31,14 @@ import io.netty5.handler.codec.http.headers.HttpHeaders; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; +import org.springframework.util.LinkedCaseInsensitiveMap; import org.springframework.util.MultiValueMap; /** * {@code MultiValueMap} implementation for wrapping Netty HTTP headers. * * @author Violeta Georgieva + * @author Simon Baslé * @since 6.1 */ public final class Netty5HeadersAdapter implements MultiValueMap { @@ -92,7 +94,8 @@ public final class Netty5HeadersAdapter implements MultiValueMap @Override public Map toSingleValueMap() { - Map singleValueMap = CollectionUtils.newLinkedHashMap(this.headers.size()); + Map singleValueMap = new LinkedCaseInsensitiveMap<>( + this.headers.size(), Locale.ROOT); this.headers.forEach(entry -> singleValueMap.putIfAbsent( entry.getKey().toString(), entry.getValue().toString())); return singleValueMap; diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/HeadersAdaptersTests.java b/spring-web/src/test/java/org/springframework/http/server/reactive/HeadersAdaptersTests.java index 65b3bbc3e5..fd78255644 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/HeadersAdaptersTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/reactive/HeadersAdaptersTests.java @@ -23,16 +23,23 @@ import java.lang.annotation.Target; import java.util.Iterator; import java.util.List; import java.util.Locale; +import java.util.Map; +import java.util.function.BiConsumer; +import java.util.function.Function; import java.util.stream.Stream; import io.netty.handler.codec.http.DefaultHttpHeaders; import io.undertow.util.HeaderMap; +import io.undertow.util.HttpString; +import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.tomcat.util.http.MimeHeaders; import org.eclipse.jetty.http.HttpFields; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.springframework.http.HttpHeaders; +import org.springframework.http.support.HttpComponentsHeadersAdapter; import org.springframework.http.support.JettyHeadersAdapter; import org.springframework.http.support.Netty4HeadersAdapter; import org.springframework.http.support.Netty5HeadersAdapter; @@ -50,15 +57,109 @@ import static org.junit.jupiter.params.provider.Arguments.arguments; * * @author Brian Clozel * @author Sam Brannen + * @author Simon Baslé */ class HeadersAdaptersTests { + @ParameterizedPopulatedHeadersTest + void toSingleValueMapIsCaseInsensitive(MultiValueMap headers) { + assertThat(headers.toSingleValueMap()).as("toSingleValueMap") + .containsEntry("TestHeader", "first") + .containsEntry("SecondHeader", "value") + .hasSize(2); + } + @ParameterizedHeadersTest + void shouldRemoveCaseInsensitiveFromKeySet(MultiValueMap headers) { + headers.add("TestHeader", "first"); + headers.add("TestHEADER", "second"); + headers.add("TestHeader", "third"); + + Iterator iterator = headers.keySet().iterator(); + iterator.next(); + iterator.remove(); + + assertThat(headers) + .doesNotContainKey("TestHeader") + .doesNotContainKey("TestHEADER") + .doesNotContainKey("testheader") + .hasSize(0); + } + + @ParameterizedPopulatedHeadersTest + void toString(MultiValueMap headers) { + String expectedFirstHeader = "TestHeader:\"first\", \"second\", \"third\""; + String expectedSecondHeader = "SecondHeader:\"value\""; + int minimumLength = expectedFirstHeader.length() + expectedSecondHeader.length() + 4; + + String result = headers.toString(); + assertThat(result) + .startsWith("[").endsWith("]") + // Using contains here because some native headers iterate over names in reverse insertion order + .contains(expectedFirstHeader) + .contains(expectedSecondHeader) + .hasSizeGreaterThanOrEqualTo(minimumLength); + + if (result.length() > minimumLength) { + String expectedEnd = " with native header names " + headers.keySet(); + assertThat(result).as("toString() with a dump of native duplicate headers") + .endsWith(expectedEnd) + .hasSize(minimumLength + expectedEnd.length()); + } + } + + @ParameterizedPopulatedHeadersTest + void copyUsingHeaderSetAddAllIsCaseInsensitive(MultiValueMap headers) { + HttpHeaders headers2 = new HttpHeaders(); + for (Map.Entry> entry : new HttpHeaders(headers).headerSet()) { + headers2.addAll(entry.getKey(), entry.getValue()); + } + + assertThat(headers2.get("TestHeader")).as("TestHeader") + .containsExactly("first", "second", "third"); + // Using the headerSet approach, we keep the first encountered casing of any given key + assertThat(headers2.keySet()).as("first casing variant").containsExactlyInAnyOrder("TestHeader", "SecondHeader"); + assertThat(headers2.toString()).as("similar toString, no 'with native headers' dump") + .isEqualTo(headers.toString().substring(0, headers.toString().indexOf(']') + 1)); + } + + @ParameterizedPopulatedHeadersTest + void copyUsingEntrySetPutRemovesDuplicates(MultiValueMap headers) { + HttpHeaders headers2 = new HttpHeaders(); + for (Map.Entry> entry : headers.entrySet()) { + headers2.put(entry.getKey(), entry.getValue()); + } + + assertThat(headers2.get("TestHeader")).as("TestHeader") + .containsExactly("first", "second", "third"); + // Ordering and casing are not guaranteed using the entrySet+put approach + assertThat(headers2).as("two keys") + .containsKey("testheader") + .containsKey("secondheader") + .hasSize(2); + assertThat(headers2.toString()).as("no 'with native headers' dump") + .doesNotContain("with native headers"); + } + + @ParameterizedPopulatedHeadersTest + void copyUsingPutAllRemovesDuplicates(MultiValueMap headers) { + HttpHeaders headers2 = new HttpHeaders(); + headers2.putAll(headers); + + assertThat(headers2.get("TestHeader")).as("TestHeader") + .containsExactly("first", "second", "third"); + // Ordering and casing are not guaranteed using the putAll approach + assertThat(headers2).as("two keys").containsOnlyKeys("testheader", "secondheader"); + assertThat(headers2.toString()).as("similar toString, no 'with native headers' dump") + .isEqualToIgnoringCase(headers.toString().substring(0, headers.toString().indexOf(']') + 1)); + } + + @ParameterizedPopulatedHeadersTest void getWithUnknownHeaderShouldReturnNull(MultiValueMap headers) { assertThat(headers.get("Unknown")).isNull(); } - @ParameterizedHeadersTest + @ParameterizedPopulatedHeadersTest void getFirstWithUnknownHeaderShouldReturnNull(MultiValueMap headers) { assertThat(headers.getFirst("Unknown")).isNull(); } @@ -78,9 +179,8 @@ class HeadersAdaptersTests { assertThat(headers.keySet()).hasSize(2); } - @ParameterizedHeadersTest + @ParameterizedPopulatedHeadersTest void containsKeyShouldBeCaseInsensitive(MultiValueMap headers) { - headers.add("TestHeader", "first"); assertThat(headers.containsKey("testheader")).isTrue(); } @@ -127,6 +227,39 @@ class HeadersAdaptersTests { assertThatThrownBy(names::remove).isInstanceOf(IllegalStateException.class); } + @ParameterizedPopulatedHeadersTest + void headerSetEntryCanSetList(MultiValueMap headers) { + for (Map.Entry> entry : new HttpHeaders(headers).headerSet()) { + entry.setValue(List.of(entry.getKey())); + } + + assertThat(headers).hasSize(2); + assertThat(headers.get("TestHeader")).containsExactly("TestHeader"); + assertThat(headers.get("SecondHeader")).containsExactly("SecondHeader"); + } + + @ParameterizedPopulatedHeadersTest + void headerSetIteratorCanRemove(MultiValueMap headers) { + Iterator>> iterator = new HttpHeaders(headers).headerSet().iterator(); + while (iterator.hasNext()) { + iterator.next(); + iterator.remove(); + } + + assertThat(headers).isEmpty(); + } + + + static T withHeaders(T nativeHeader, Function> addMethod) { + BiConsumer add = addMethod.apply(nativeHeader); + add.accept("TestHeader", "first"); + add.accept("TestHEADER", "second"); + add.accept("SecondHeader", "value"); + add.accept("TestHeader", "third"); + + return nativeHeader; + } + @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) @ParameterizedTest(name = "[{index}] {0}") @@ -138,10 +271,33 @@ class HeadersAdaptersTests { return Stream.of( arguments(named("Map", CollectionUtils.toMultiValueMap(new LinkedCaseInsensitiveMap<>(8, Locale.ENGLISH)))), arguments(named("Netty", new Netty4HeadersAdapter(new DefaultHttpHeaders()))), - arguments(named("Netty", new Netty5HeadersAdapter(io.netty5.handler.codec.http.headers.HttpHeaders.newHeaders()))), + arguments(named("Netty5", new Netty5HeadersAdapter(io.netty5.handler.codec.http.headers.HttpHeaders.newHeaders()))), arguments(named("Tomcat", new TomcatHeadersAdapter(new MimeHeaders()))), arguments(named("Undertow", new UndertowHeadersAdapter(new HeaderMap()))), - arguments(named("Jetty", new JettyHeadersAdapter(HttpFields.build()))) + arguments(named("Jetty", new JettyHeadersAdapter(HttpFields.build()))), + arguments(named("HttpComponents", new HttpComponentsHeadersAdapter(new HttpGet("https://example.com")))) + ); + } + + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.METHOD) + @ParameterizedTest + @MethodSource("nativeHeadersWithCasedEntries") + @interface ParameterizedPopulatedHeadersTest { + } + + static Stream nativeHeadersWithCasedEntries() { + return Stream.of( + arguments(named("Netty", new Netty4HeadersAdapter(withHeaders(new DefaultHttpHeaders(), h -> h::add)))), + arguments(named("Netty5", new Netty5HeadersAdapter(withHeaders(io.netty5.handler.codec.http.headers.HttpHeaders.newHeaders(), + h -> h::add)))), + arguments(named("Tomcat", new TomcatHeadersAdapter(withHeaders(new MimeHeaders(), + h -> (k, v) -> h.addValue(k).setString(v))))), + arguments(named("Undertow", new UndertowHeadersAdapter(withHeaders(new HeaderMap(), + h -> (k, v) -> h.add(HttpString.tryFromString(k), v))))), + arguments(named("Jetty", new JettyHeadersAdapter(withHeaders(HttpFields.build(), h -> h::add)))), + arguments(named("HttpComponents", new HttpComponentsHeadersAdapter(withHeaders(new HttpGet("https://example.com"), + h -> h::addHeader)))) ); }