diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java index 42f6d7207c90..cb6213a8405e 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java @@ -51,19 +51,20 @@ public void beforeDecoding(Response response) HttpResponse httpResponse = (HttpResponse)response; httpResponse.headers(headers -> { - ListIterator iterator = headers.listIterator(); - while (iterator.hasNext()) + boolean seenContentEncoding = false; + for (ListIterator iterator = headers.listIterator(headers.size()); iterator.hasPrevious();) { - HttpField field = iterator.next(); + HttpField field = iterator.previous(); HttpHeader header = field.getHeader(); if (header == HttpHeader.CONTENT_LENGTH) { // Content-Length is not valid anymore while we are decoding. iterator.remove(); } - else if (header == HttpHeader.CONTENT_ENCODING) + else if (header == HttpHeader.CONTENT_ENCODING && !seenContentEncoding) { - // Content-Encoding should be removed/modified as the content will be decoded. + // Last Content-Encoding should be removed/modified as the content will be decoded. + seenContentEncoding = true; String value = field.getValue(); int comma = value.lastIndexOf(","); if (comma < 0) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpReceiver.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpReceiver.java index b0fc67771860..a8808c65abb9 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpReceiver.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpReceiver.java @@ -258,15 +258,13 @@ protected void responseHeaders(HttpExchange exchange) { // Content-Encoding may have multiple values in the order they // are applied, but we only support one decoding pass, the last one. - String contentEncoding = responseHeaders.get(HttpHeader.CONTENT_ENCODING); + String contentEncoding = responseHeaders.getLast(HttpHeader.CONTENT_ENCODING); if (contentEncoding != null) { int comma = contentEncoding.indexOf(","); if (comma > 0) { - QuotedCSV parser = new QuotedCSV(false); - parser.addValue(contentEncoding); - List values = parser.getValues(); + List values = new QuotedCSV(false, contentEncoding).getValues(); contentEncoding = values.get(values.size() - 1); } } diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java index 1b21907c63ac..1da90c1b2fb1 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpField.java @@ -362,6 +362,56 @@ else if (c != '0' && c != '.') return param != ZERO_QUALITY.length() && match == search.length(); } + /** + * Look for a value as the last value in a possible multivalued field + * Parameters and specifically quality parameters are not considered. + * @param search Values to search for (case-insensitive) + * @return True iff the value is contained in the field value entirely or + * as the last element of a quoted comma separated list. + */ + public boolean containsLast(String search) + { + return containsLast(getValue(), search); + } + + /** + * Look for the last value in a possible multivalued field + * Parameters and specifically quality parameters are not considered. + * @param value The field value to search in. + * @param search Values to search for (case-insensitive) + * @return True iff the value is contained in the field value entirely or + * as the last element of a quoted comma separated list. + */ + public static boolean containsLast(String value, String search) + { + if (search == null) + return value == null; + if (search.isEmpty()) + return false; + if (value == null) + return false; + if (search.equalsIgnoreCase(value)) + return true; + + if (value.endsWith(search)) + { + int i = value.length() - search.length() - 1; + while (i >= 0) + { + char c = value.charAt(i--); + if (c == ',') + return true; + if (c != ' ') + return false; + } + return true; + } + + QuotedCSV csv = new QuotedCSV(false, value); + List values = csv.getValues(); + return !values.isEmpty() && search.equalsIgnoreCase(values.get(values.size() - 1)); + } + @Override public int hashCode() { diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java index 93a5a0de572c..44c6aca679b9 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java @@ -161,6 +161,27 @@ default HttpFields get() return this; } + @Override + default Iterator iterator() + { + return listIterator(); + } + + /** + * @return an iterator over the {@link HttpField}s in this {@code HttpFields}. + * @see #listIterator(int) + */ + default ListIterator listIterator() + { + return listIterator(0); + } + + /** + * @return an iterator over the {@link HttpField}s in this {@code HttpFields} starting at the given index. + * @see #listIterator() + */ + ListIterator listIterator(int index); + /** *

Returns an immutable copy of this {@link HttpFields} instance.

* @@ -245,6 +266,27 @@ default boolean contains(HttpHeader header, String value) return false; } + /** + * Look for a value as the last value in a possible multivalued field. + * Parameters and specifically quality parameters are not considered. + * @param header The {@link HttpHeader} type to search for. + * @param value The value to search for (case-insensitive) + * @return True iff the value is contained in the field value entirely or + * as the last element of a quoted comma separated list. + * @see HttpField#containsLast(String) + */ + default boolean containsLast(HttpHeader header, String value) + { + for (ListIterator i = listIterator(size()); i.hasPrevious();) + { + HttpField f = i.previous(); + + if (f.getHeader() == header) + return f.containsLast(value); + } + return false; + } + /** *

Returns whether this instance contains the given field name * with the given value.

@@ -340,6 +382,28 @@ default String get(HttpHeader header) return null; } + /** + *

Returns the encoded value of the last field with the given field name, + * or {@code null} if no such header is present.

+ *

In case of multi-valued fields, the returned value is the encoded + * value, including commas and quotes, as returned by {@link HttpField#getValue()}.

+ * + * @param header the field name to search for + * @return the raw value of the last field with the given field name, + * or {@code null} if no such header is present + * @see HttpField#getValue() + */ + default String getLast(HttpHeader header) + { + for (ListIterator i = listIterator(size()); i.hasPrevious();) + { + HttpField f = i.previous(); + if (f.getHeader() == header) + return f.getValue(); + } + return null; + } + /** *

Returns the encoded value of the first field with the given field name, * or {@code null} if no such field is present.

@@ -904,11 +968,7 @@ default Mutable add(HttpHeader header, long value) */ default Mutable add(HttpField field) { - ListIterator i = listIterator(); - while (i.hasNext()) - { - i.next(); - } + ListIterator i = listIterator(size()); i.add(field); return this; } @@ -1041,20 +1101,6 @@ default void ensureField(HttpField field) } } - /** - * @return an {@link Iterator} over the {@link HttpField}s of this instance - */ - @Override - default Iterator iterator() - { - return listIterator(); - } - - /** - * @return a {@link ListIterator} over the {@link HttpField}s of this instance - */ - ListIterator listIterator(); - /** *

Puts the given {@link HttpField} into this instance.

*

If a fields with the same name is present, the given field @@ -1111,7 +1157,7 @@ default Mutable put(String name, String value) *

This method behaves like {@link #remove(HttpHeader)} when * the given {@code value} is {@code null}, otherwise behaves * like {@link #put(HttpField)}.

- * + * * @param header the name of the field * @param value the value of the field; if {@code null} the field is removed * @return this instance @@ -1590,9 +1636,9 @@ public Mutable clear() } @Override - public ListIterator listIterator() + public ListIterator listIterator(int index) { - ListIterator i = _fields.listIterator(); + ListIterator i = _fields.listIterator(index); return new ListIterator<>() { HttpField last; diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ImmutableHttpFields.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ImmutableHttpFields.java index 58202c2f73ef..ba83642b292b 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ImmutableHttpFields.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ImmutableHttpFields.java @@ -14,7 +14,7 @@ package org.eclipse.jetty.http; import java.util.Arrays; -import java.util.Iterator; +import java.util.ListIterator; import java.util.NoSuchElementException; import java.util.Objects; import java.util.stream.Stream; @@ -134,24 +134,9 @@ public HttpField getField(int index) } @Override - public Iterator iterator() + public ListIterator listIterator(int index) { - return new Iterator<>() - { - int _index = 0; - - @Override - public boolean hasNext() - { - return _index < _size; - } - - @Override - public HttpField next() - { - return _fields[_index++]; - } - }; + return new Listerator(index); } @Override @@ -171,4 +156,77 @@ public String toString() { return asString(); } + + private class Listerator implements ListIterator + { + private int _index; + private int _last = -1; + + Listerator(int index) + { + if (index < 0 || index > _size) + throw new NoSuchElementException(Integer.toString(index)); + _index = index; + } + + @Override + public void add(HttpField field) + { + throw new UnsupportedOperationException(); + } + + @Override + public boolean hasNext() + { + return _index < _size; + } + + @Override + public boolean hasPrevious() + { + return _index > 0; + } + + @Override + public HttpField next() + { + if (_index >= _size) + throw new NoSuchElementException(Integer.toString(_index)); + _last = _index++; + return _fields[_last]; + } + + @Override + public int nextIndex() + { + return _index + 1; + } + + @Override + public HttpField previous() + { + if (_index <= 0) + throw new NoSuchElementException(Integer.toString(_index - 1)); + _last = --_index; + return _fields[_last]; + } + + @Override + public int previousIndex() + { + return _index - 1; + } + + @Override + public void remove() + { + throw new UnsupportedOperationException(); + } + + @Override + public void set(HttpField field) + { + throw new UnsupportedOperationException(); + } + } } diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MutableHttpFields.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MutableHttpFields.java index c500172d2ed5..70776d31cb12 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MutableHttpFields.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MutableHttpFields.java @@ -243,7 +243,13 @@ public void remove() @Override public ListIterator listIterator() { - return new ListItr(); + return new Listerator(0); + } + + @Override + public ListIterator listIterator(int index) + { + return new Listerator(index); } @Override @@ -433,10 +439,17 @@ public String toString() return asString(); } - private class ListItr implements ListIterator + private class Listerator implements ListIterator { - int _cursor; // index of next element to return - int _current = -1; + private int _index; + private int _last = -1; + + Listerator(int index) + { + if (index < 0 || index > _size) + throw new NoSuchElementException(Integer.toString(index)); + _index = index; + } @Override public void add(HttpField field) @@ -447,72 +460,72 @@ public void add(HttpField field) int last = _size++; if (_fields.length < _size) _fields = Arrays.copyOf(_fields, _fields.length + SIZE_INCREMENT); - System.arraycopy(_fields, _cursor, _fields, _cursor + 1, last - _cursor); - _fields[_cursor++] = field; - _current = -1; + System.arraycopy(_fields, _index, _fields, _index + 1, last - _index); + _fields[_index++] = field; + _last = -1; } @Override public boolean hasNext() { - return _cursor != _size; + return _index < _size; } @Override public boolean hasPrevious() { - return _cursor > 0; + return _index > 0; } @Override public HttpField next() { - if (_cursor == _size) - throw new NoSuchElementException(); - _current = _cursor++; - return _fields[_current]; + if (_index >= _size) + throw new NoSuchElementException(Integer.toString(_index)); + _last = _index++; + return _fields[_last]; } @Override public int nextIndex() { - return _cursor + 1; + return _index + 1; } @Override public HttpField previous() { - if (_cursor == 0) - throw new NoSuchElementException(); - _current = --_cursor; - return _fields[_current]; + if (_index <= 0) + throw new NoSuchElementException(Integer.toString(_index - 1)); + _last = --_index; + return _fields[_last]; } @Override public int previousIndex() { - return _cursor - 1; + return _index - 1; } @Override public void remove() { - if (_current < 0) + if (_last < 0) throw new IllegalStateException(); - org.eclipse.jetty.http.MutableHttpFields.this.remove(_current); - _cursor = _current; - _current = -1; + org.eclipse.jetty.http.MutableHttpFields.this.remove(_last); + _index = _last; + _last = -1; } @Override public void set(HttpField field) { - if (_current < 0) + if (_last < 0) throw new IllegalStateException(); if (field == null) remove(); else - _fields[_current] = field; + _fields[_last] = field; } } } diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java index b460a21a6f02..46bf5d58a743 100644 --- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java @@ -60,9 +60,9 @@ public static Stream mutables() private final HttpFields.Mutable fields = HttpFields.build(); @Override - public ListIterator listIterator() + public ListIterator listIterator(int index) { - return fields.listIterator(); + return fields.listIterator(index); } @Override @@ -76,9 +76,9 @@ public String toString() private final HttpFields.Mutable fields = HttpFields.build(); @Override - public ListIterator listIterator() + public ListIterator listIterator(int index) { - return fields.listIterator(); + return fields.listIterator(index); } @Override @@ -90,15 +90,7 @@ public String toString() @Override public HttpFields asImmutable() { - HttpFields f = fields.asImmutable(); - return new HttpFields() - { - @Override - public Iterator iterator() - { - return f.iterator(); - } - }; + return fields.asImmutable(); } } ); @@ -800,10 +792,45 @@ public void testContains(HttpFields.Mutable header) assertFalse(header.contains(new HttpField("N8", "def"))); assertFalse(header.contains(HttpHeader.ACCEPT, "def")); assertFalse(header.contains(HttpHeader.AGE, "abc")); - assertFalse(header.contains("n11")); } + @ParameterizedTest + @MethodSource("mutables") + public void testContainsLast(HttpFields.Mutable header) + { + assertFalse(header.containsLast(HttpHeader.TRANSFER_ENCODING, "gzip")); + + header.add(HttpHeader.TRANSFER_ENCODING, "gzip"); + assertTrue(header.containsLast(HttpHeader.TRANSFER_ENCODING, "gzip")); + + header.add(HttpHeader.TRANSFER_ENCODING, "bz2"); + assertFalse(header.containsLast(HttpHeader.TRANSFER_ENCODING, "gzip")); + assertTrue(header.containsLast(HttpHeader.TRANSFER_ENCODING, "bz2")); + + header.add(HttpHeader.TRANSFER_ENCODING, "foo, bar"); + assertFalse(header.containsLast(HttpHeader.TRANSFER_ENCODING, "foo")); + assertTrue(header.containsLast(HttpHeader.TRANSFER_ENCODING, "bar")); + + header.add(HttpHeader.TRANSFER_ENCODING, "\"x\", \"y\""); + assertFalse(header.containsLast(HttpHeader.TRANSFER_ENCODING, "x")); + assertTrue(header.containsLast(HttpHeader.TRANSFER_ENCODING, "y")); + + header.add(HttpHeader.TRANSFER_ENCODING, "tom,dick,harry"); + assertFalse(header.containsLast(HttpHeader.TRANSFER_ENCODING, "tom")); + assertTrue(header.containsLast(HttpHeader.TRANSFER_ENCODING, "harry")); + + header.add(HttpHeader.TRANSFER_ENCODING, "spongebob"); + assertFalse(header.containsLast(HttpHeader.TRANSFER_ENCODING, "sponge")); + assertFalse(header.containsLast(HttpHeader.TRANSFER_ENCODING, "bob")); + assertTrue(header.containsLast(HttpHeader.TRANSFER_ENCODING, "spongebob")); + + header.add(HttpHeader.TRANSFER_ENCODING, "sponge bob"); + assertFalse(header.containsLast(HttpHeader.TRANSFER_ENCODING, "sponge")); + assertFalse(header.containsLast(HttpHeader.TRANSFER_ENCODING, "bob")); + assertTrue(header.containsLast(HttpHeader.TRANSFER_ENCODING, "sponge bob")); + } + @ParameterizedTest @ValueSource(strings = {"Host", "host", "HOST", "HoSt", "Connection", "CONNECTION", "connection", "CoNnEcTiOn"}) public void testContainsKeyTrue(String keyName) diff --git a/jetty-core/jetty-http3/jetty-http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/metadata/Http3Fields.java b/jetty-core/jetty-http3/jetty-http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/metadata/Http3Fields.java index 7994e9480f4c..8b723c86461f 100644 --- a/jetty-core/jetty-http3/jetty-http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/metadata/Http3Fields.java +++ b/jetty-core/jetty-http3/jetty-http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/metadata/Http3Fields.java @@ -19,6 +19,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.ListIterator; import java.util.Set; import java.util.stream.Stream; @@ -129,6 +130,7 @@ public HttpField getField(int index) @Override public int size() { + // TODO this is very inefficient return Math.toIntExact(stream().count()); } @@ -139,22 +141,7 @@ public Stream stream() if (httpFields == null) return pseudoHeadersStream; - Stream httpFieldStream = httpFields.stream().filter(field -> - { - HttpHeader header = field.getHeader(); - - // If the header is specifically ignored skip it (Connection Specific Headers). - if (header != null && IGNORED_HEADERS.contains(header)) - return false; - - // If this is the TE header field it can only have the value "trailers". - if ((header == HttpHeader.TE) && !field.contains("trailers")) - return false; - - // Remove the headers nominated by the Connection header field. - String name = field.getLowerCaseName(); - return hopHeaders == null || !hopHeaders.contains(name); - }); + Stream httpFieldStream = httpFields.stream().filter(this::filterIgnored); if (contentLengthHeader != null) return Stream.concat(pseudoHeadersStream, Stream.concat(httpFieldStream, Stream.of(contentLengthHeader))); @@ -162,9 +149,33 @@ public Stream stream() return Stream.concat(pseudoHeadersStream, httpFieldStream); } + private boolean filterIgnored(HttpField field) + { + HttpHeader header = field.getHeader(); + + // If the header is specifically ignored skip it (Connection Specific Headers). + if (header != null && IGNORED_HEADERS.contains(header)) + return false; + + // If this is the TE header field it can only have the value "trailers". + if ((header == HttpHeader.TE) && !field.contains("trailers")) + return false; + + // Remove the headers nominated by the Connection header field. + String name = field.getLowerCaseName(); + return hopHeaders == null || !hopHeaders.contains(name); + } + @Override public Iterator iterator() { return stream().iterator(); } + + @Override + public ListIterator listIterator(int index) + { + // TODO this is very inefficient and toList returns an unmodifiable list + return stream().toList().listIterator(index); + } } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java index 476058826208..8f36658961b9 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.server.handler.gzip; +import java.util.ListIterator; import java.util.Set; import java.util.zip.Deflater; @@ -550,14 +551,20 @@ public boolean handle(Request request, Response response, Callback callback) thr boolean inflatable = false; boolean deflatable = false; boolean etagMatches = false; - for (HttpField field : fields) + boolean seenContentEncoding = false; + for (ListIterator i = fields.listIterator(fields.size()); i.hasPrevious();) { + HttpField field = i.previous(); HttpHeader header = field.getHeader(); if (header == null) continue; switch (header) { - case CONTENT_ENCODING -> inflatable = field.contains("gzip"); + case CONTENT_ENCODING -> + { + inflatable |= !seenContentEncoding && field.containsLast("gzip"); + seenContentEncoding = true; + } case ACCEPT_ENCODING -> deflatable = field.contains("gzip"); case IF_MATCH, IF_NONE_MATCH -> etagMatches |= field.getValue().contains(EtagUtils.ETAG_SEPARATOR); } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipRequest.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipRequest.java index 53802d5fac44..424b2d0e677c 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipRequest.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipRequest.java @@ -14,7 +14,7 @@ package org.eclipse.jetty.server.handler.gzip; import java.nio.ByteBuffer; -import java.util.regex.Pattern; +import java.util.ListIterator; import org.eclipse.jetty.http.CompressedContentFormat; import org.eclipse.jetty.http.GZIPContentDecoder; @@ -33,7 +33,6 @@ public class GzipRequest extends Request.Wrapper { private static final HttpField X_CE_GZIP = new PreEncodedHttpField("X-Content-Encoding", "gzip"); - private static final Pattern COMMA_GZIP = Pattern.compile(".*, *gzip"); // TODO: use InflaterPool from somewhere. private static final InflaterPool __inflaterPool = new InflaterPool(-1, true); @@ -58,38 +57,38 @@ public GzipRequest(Request request, int inflateBufferSize) private HttpFields updateRequestFields(Request request, boolean inflatable) { HttpFields fields = request.getHeaders(); - HttpFields.Mutable newFields = HttpFields.build(fields.size()); - for (HttpField field : fields) + HttpFields.Mutable newFields = HttpFields.build(fields); + boolean contentEncodingSeen = false; + + // iterate in reverse to see last content encoding first + for (ListIterator i = newFields.listIterator(newFields.size()); i.hasPrevious();) { + HttpField field = i.previous(); + HttpHeader header = field.getHeader(); if (header == null) - { - newFields.add(field); continue; - } switch (header) { case CONTENT_ENCODING -> { - if (inflatable) + if (inflatable && !contentEncodingSeen) { + contentEncodingSeen = true; + if (field.getValue().equalsIgnoreCase("gzip")) { - newFields.add(X_CE_GZIP); - continue; + i.set(X_CE_GZIP); } - - if (COMMA_GZIP.matcher(field.getValue()).matches()) + else if (field.containsLast("gzip")) { String v = field.getValue(); v = v.substring(0, v.lastIndexOf(',')); - newFields.add(X_CE_GZIP); - newFields.add(new HttpField(HttpHeader.CONTENT_ENCODING, v)); - continue; + i.set(new HttpField(HttpHeader.CONTENT_ENCODING, v)); + i.add(X_CE_GZIP); } } - newFields.add(field); } case IF_MATCH, IF_NONE_MATCH -> { @@ -97,14 +96,15 @@ private HttpFields updateRequestFields(Request request, boolean inflatable) String etagsNoSuffix = CompressedContentFormat.GZIP.stripSuffixes(etags); if (!etagsNoSuffix.equals(etags)) { - newFields.add(new HttpField(field.getHeader(), etagsNoSuffix)); + i.set(new HttpField(field.getHeader(), etagsNoSuffix)); request.setAttribute(GzipHandler.GZIP_HANDLER_ETAGS, etags); - continue; } - newFields.add(field); } - case CONTENT_LENGTH -> newFields.add(inflatable ? new HttpField("X-Content-Length", field.getValue()) : field); - default -> newFields.add(field); + case CONTENT_LENGTH -> + { + if (inflatable) + i.set(new HttpField("X-Content-Length", field.getValue())); + } } } return newFields.asImmutable(); diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/ResponseHttpFields.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/ResponseHttpFields.java index 104c85c63f8d..b6d707b5be66 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/ResponseHttpFields.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/ResponseHttpFields.java @@ -25,7 +25,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.eclipse.jetty.server.internal.ResponseHttpFields.PersistentField.isPersistent; +import static org.eclipse.jetty.server.internal.ResponseHttpFields.Persistent.isPersistent; public class ResponseHttpFields implements HttpFields.Mutable { @@ -94,11 +94,10 @@ public Mutable clear() { if (!_committed.get()) { - // TODO iterate backwards when the list iterator of that form is available - for (Iterator iterator = _fields.iterator(); iterator.hasNext();) + for (ListIterator iterator = _fields.listIterator(_fields.size()); iterator.hasPrevious();) { - HttpField field = iterator.next(); - if (!PersistentField.isPersistent(field)) + HttpField field = iterator.previous(); + if (!Persistent.isPersistent(field)) iterator.remove(); } } @@ -118,7 +117,7 @@ public Iterator iterator() Iterator i = _fields.iterator(); return new Iterator<>() { - HttpField _current; + private HttpField _current; @Override public boolean hasNext() @@ -139,7 +138,9 @@ public void remove() if (_committed.get()) throw new UnsupportedOperationException("Read Only"); if (isPersistent(_current)) - throw new IllegalStateException("Persistent field"); + throw new UnsupportedOperationException("Persistent field"); + if (_current == null) + throw new IllegalStateException("No current field"); i.remove(); _current = null; } @@ -147,12 +148,12 @@ public void remove() } @Override - public ListIterator listIterator() + public ListIterator listIterator(int index) { - ListIterator i = _fields.listIterator(); + ListIterator i = _fields.listIterator(index); return new ListIterator<>() { - HttpField _current; + private HttpField _current; @Override public boolean hasNext() @@ -198,7 +199,9 @@ public void remove() if (_committed.get()) throw new UnsupportedOperationException("Read Only"); if (isPersistent(_current)) - throw new IllegalStateException("Persistent field"); + throw new UnsupportedOperationException("Persistent field"); + if (_current == null) + throw new IllegalStateException("No current field"); i.remove(); _current = null; } @@ -212,7 +215,7 @@ public void set(HttpField field) { // cannot change the field name if (field == null || !field.isSameName(_current)) - throw new IllegalStateException("Persistent field"); + throw new UnsupportedOperationException("Persistent field"); // new field must also be persistent if (!isPersistent(field)) @@ -220,6 +223,8 @@ public void set(HttpField field) ? new PersistentPreEncodedHttpField(field.getHeader(), field.getValue()) : new PersistentHttpField(field); } + if (_current == null) + throw new IllegalStateException("No current field"); if (field == null) i.remove(); else @@ -246,21 +251,21 @@ public String toString() /** * A marker interface for {@link HttpField}s that cannot be {@link #remove(HttpHeader) removed} or {@link #clear() cleared} - * from a {@link ResponseHttpFields} instance. Persistent fields are not immutable in the {@link ResponseHttpFields} - * and may be replaced with a different value. + * from a {@link ResponseHttpFields} instance. Persistent fields are not immutable in the {@link ResponseHttpFields} + * and may be replaced with a different value. i.e. A Persistent field cannot be removed but can be overwritten. */ - public interface PersistentField + public interface Persistent { static boolean isPersistent(HttpField field) { - return field instanceof PersistentField; + return field instanceof Persistent; } } /** - * A {@link HttpField} that is a {@link PersistentField}. + * A {@link HttpField} that is a {@link Persistent}. */ - public static class PersistentHttpField extends HttpField implements PersistentField + public static class PersistentHttpField extends HttpField implements Persistent { private final HttpField _field; @@ -284,16 +289,11 @@ public long getLongValue() } /** - * A {@link PreEncodedHttpField} that is a {@link PersistentField}. + * A {@link PreEncodedHttpField} that is a {@link Persistent}. */ - public static class PersistentPreEncodedHttpField extends PreEncodedHttpField implements PersistentField + public static class PersistentPreEncodedHttpField extends PreEncodedHttpField implements Persistent { public PersistentPreEncodedHttpField(HttpHeader header, String value) - { - this(header, value, true); - } - - public PersistentPreEncodedHttpField(HttpHeader header, String value, boolean immutable) { super(header, value); } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java index d792060571ee..616655845777 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java @@ -83,7 +83,7 @@ public boolean handle(Request request, Response response, Callback callback) } @Test - public void testServerDateFieldsFrozen() throws Exception + public void testServerDateFieldsPersistent() throws Exception { server.setHandler(new Handler.Abstract() { @@ -92,36 +92,36 @@ public boolean handle(Request request, Response response, Callback callback) { response.getHeaders().add("Temp", "field"); response.getHeaders().add("Test", "before reset"); - assertThrows(IllegalStateException.class, () -> response.getHeaders().remove(HttpHeader.SERVER)); - assertThrows(IllegalStateException.class, () -> response.getHeaders().remove(HttpHeader.DATE)); + assertThrows(UnsupportedOperationException.class, () -> response.getHeaders().remove(HttpHeader.SERVER)); + assertThrows(UnsupportedOperationException.class, () -> response.getHeaders().remove(HttpHeader.DATE)); response.getHeaders().remove("Temp"); response.getHeaders().add("Temp", "field"); Iterator iterator = response.getHeaders().iterator(); assertThat(iterator.next().getHeader(), is(HttpHeader.SERVER)); - assertThrows(IllegalStateException.class, iterator::remove); + assertThrows(UnsupportedOperationException.class, iterator::remove); assertThat(iterator.next().getHeader(), is(HttpHeader.DATE)); - assertThrows(IllegalStateException.class, iterator::remove); + assertThrows(UnsupportedOperationException.class, iterator::remove); assertThat(iterator.next().getName(), is("Test")); assertThat(iterator.next().getName(), is("Temp")); iterator.remove(); assertFalse(response.getHeaders().contains("Temp")); - assertThrows(IllegalStateException.class, () -> response.getHeaders().remove(HttpHeader.SERVER)); + assertThrows(UnsupportedOperationException.class, () -> response.getHeaders().remove(HttpHeader.SERVER)); assertFalse(iterator.hasNext()); ListIterator listIterator = response.getHeaders().listIterator(); assertThat(listIterator.next().getHeader(), is(HttpHeader.SERVER)); - assertThrows(IllegalStateException.class, listIterator::remove); + assertThrows(UnsupportedOperationException.class, listIterator::remove); assertThat(listIterator.next().getHeader(), is(HttpHeader.DATE)); - assertThrows(IllegalStateException.class, () -> listIterator.set(new HttpField("Something", "else"))); + assertThrows(UnsupportedOperationException.class, () -> listIterator.set(new HttpField("Something", "else"))); listIterator.set(new HttpField(HttpHeader.DATE, "1970-01-01")); assertThat(listIterator.previous().getHeader(), is(HttpHeader.DATE)); - assertThrows(IllegalStateException.class, listIterator::remove); + assertThrows(UnsupportedOperationException.class, listIterator::remove); assertThat(listIterator.previous().getHeader(), is(HttpHeader.SERVER)); - assertThrows(IllegalStateException.class, listIterator::remove); + assertThrows(UnsupportedOperationException.class, listIterator::remove); assertThat(listIterator.next().getHeader(), is(HttpHeader.SERVER)); assertThat(listIterator.next().getHeader(), is(HttpHeader.DATE)); - assertThrows(IllegalStateException.class, listIterator::remove); + assertThrows(UnsupportedOperationException.class, listIterator::remove); listIterator.add(new HttpField("Temp", "value")); assertThat(listIterator.previous().getName(), is("Temp")); listIterator.remove(); @@ -130,8 +130,11 @@ public boolean handle(Request request, Response response, Callback callback) response.getHeaders().add("Temp", "field"); response.reset(); response.getHeaders().add("Test", "after reset"); + + response.getHeaders().putDate("Date", 1L); + assertThrows(UnsupportedOperationException.class, () -> response.getHeaders().put(HttpHeader.SERVER, (String)null)); response.getHeaders().put(HttpHeader.SERVER, "jettyrocks"); - assertThrows(IllegalStateException.class, () -> response.getHeaders().put(HttpHeader.SERVER, (String)null)); + assertThrows(UnsupportedOperationException.class, () -> response.getHeaders().put(HttpHeader.SERVER, (String)null)); callback.succeeded(); return true; } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java index 1c94cd813cb7..71d768b473bd 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipHandlerTest.java @@ -919,6 +919,44 @@ public void testGzippedRequestPostChunked() throws Exception assertThat(response.getContent(), is(data)); } + @Test + public void testGzippedRequestOtherEncodingPost() throws Exception + { + _contextHandler.setHandler(new EchoHandler()); + _server.start(); + + String data = "Hello Nice World! "; + for (int i = 0; i < 10; ++i) + { + data += data; + } + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + GZIPOutputStream output = new GZIPOutputStream(baos); + output.write(data.getBytes(StandardCharsets.UTF_8)); + output.close(); + byte[] bytes = baos.toByteArray(); + + // generated and parsed test + HttpTester.Request request = HttpTester.newRequest(); + HttpTester.Response response; + + request.setMethod("POST"); + request.setURI("/ctx/echo"); + request.setVersion("HTTP/1.0"); + request.setHeader("Host", "tester"); + request.setHeader("Content-Type", "text/plain"); + request.setHeader("Content-Encoding", "gzip"); + request.setHeader("Content-Encoding", "other"); + request.setContent(bytes); + + response = HttpTester.parseResponse(_connector.getResponse(request.generate())); + + assertThat(response.getStatus(), is(200)); + GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(response.getContentBytes())); + String received = IO.toString(in); + assertThat(received, is(data)); + } + @Test public void testIncludeExcludeInflationPaths() throws Exception { diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletCoreResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletCoreResponse.java index a11ae77b6bba..69e853a4bac9 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletCoreResponse.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletCoreResponse.java @@ -205,14 +205,14 @@ private HttpServletResponseHttpFields(HttpServletResponse response) } @Override - public ListIterator listIterator() + public ListIterator listIterator(int index) { // The minimum requirement is to implement the listIterator, but it is inefficient. // Other methods are implemented for efficiency. final ListIterator list = _response.getHeaderNames().stream() .map(n -> new HttpField(n, _response.getHeader(n))) .collect(Collectors.toList()) - .listIterator(); + .listIterator(index); return new ListIterator<>() {