From 5fb45f7273b493c2ad109e1719c047dd2b9dcc8c Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 5 May 2022 08:59:29 +1000 Subject: [PATCH] fix WebSocket HttpFieldsWrapper Signed-off-by: Lachlan Roberts --- .../server/internal/HttpFieldsWrapper.java | 179 +++++++++++------- .../internal/WebSocketHttpFieldsWrapper.java | 1 + .../tests/JettyWebSocketNegotiationTest.java | 2 - 3 files changed, 116 insertions(+), 66 deletions(-) diff --git a/jetty-core/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/HttpFieldsWrapper.java b/jetty-core/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/HttpFieldsWrapper.java index 0e70e878b4f..cf1cf221c66 100644 --- a/jetty-core/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/HttpFieldsWrapper.java +++ b/jetty-core/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/HttpFieldsWrapper.java @@ -13,10 +13,14 @@ package org.eclipse.jetty.websocket.core.server.internal; +import java.util.EnumSet; +import java.util.Iterator; import java.util.ListIterator; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpHeaderValue; public class HttpFieldsWrapper implements HttpFields.Mutable { @@ -27,8 +31,6 @@ public class HttpFieldsWrapper implements HttpFields.Mutable _fields = fields; } - // TODO a signature that took HttpField would be better. - // TODO Do we need Put? Could it just be done as a onRemoveField then an onAddField? public boolean onPutField(String name, String value) { return true; @@ -43,73 +45,122 @@ public class HttpFieldsWrapper implements HttpFields.Mutable { return true; } + + @Override + public Mutable add(String name, String value) + { + if (onAddField(name, value)) + return _fields.add(name, value); + return this; + } + + @Override + public Mutable add(HttpHeader header, HttpHeaderValue value) + { + if (onAddField(header.name(), value.asString())) + return _fields.add(header, value); + return this; + } + + @Override + public Mutable add(HttpHeader header, String value) + { + if (onAddField(header.name(), value)) + return _fields.add(header, value); + return this; + } + + @Override + public Mutable add(HttpField field) + { + if (onAddField(field.getName(), field.getValue())) + return _fields.add(field); + return this; + } + + @Override + public Mutable add(HttpFields fields) + { + for (HttpField field : fields) + { + add(field); + } + return this; + } + + @Override + public Mutable clear() + { + return _fields.clear(); + } + + @Override + public Iterator iterator() + { + return _fields.iterator(); + } @Override public ListIterator listIterator() { - return new ListIterator<>() + return _fields.listIterator(); + } + + @Override + public Mutable put(HttpField field) + { + if (onPutField(field.getName(), field.getValue())) + return _fields.put(field); + return this; + } + + @Override + public Mutable put(String name, String value) + { + if (onPutField(name, value)) + return _fields.put(name, value); + return this; + } + + @Override + public Mutable put(HttpHeader header, HttpHeaderValue value) + { + if (onPutField(header.name(), value.asString())) + return _fields.put(header, value); + return this; + } + + @Override + public Mutable put(HttpHeader header, String value) + { + if (onPutField(header.name(), value)) + return _fields.put(header, value); + return this; + } + + @Override + public Mutable remove(HttpHeader header) + { + if (onRemoveField(header.name())) + return _fields.remove(header); + return this; + } + + @Override + public Mutable remove(EnumSet fields) + { + for (HttpHeader header : fields) { - final ListIterator _list = _fields.listIterator(); - HttpField _last; + remove(header); + } + return this; + } - @Override - public boolean hasNext() - { - return _list.hasNext(); - } - - @Override - public HttpField next() - { - return _last = _list.next(); - } - - @Override - public boolean hasPrevious() - { - return _list.hasPrevious(); - } - - @Override - public HttpField previous() - { - return _last = _list.previous(); - } - - @Override - public int nextIndex() - { - return _list.nextIndex(); - } - - @Override - public int previousIndex() - { - return _list.previousIndex(); - } - - @Override - public void remove() - { - if (_last != null && HttpFieldsWrapper.this.onRemoveField(_last.getName())) - _list.remove(); - } - - @Override - public void set(HttpField httpField) - { - if (_last != null && HttpFieldsWrapper.this.onPutField(_last.getName(), _last.getValue())) - _list.set(httpField); - } - - @Override - public void add(HttpField httpField) - { - // TODO: we don't know if this resulted from a put or an add. - // if (httpField != null && HttpFieldsWrapper.this.onAddField(httpField.getName(), httpField.getValue())) - if (_last != null && HttpFieldsWrapper.this.onAddField(_last.getName(), _last.getValue())) - _list.add(httpField); - } - }; + @Override + public Mutable remove(String name) + { + if (onRemoveField(name)) + return _fields.remove(name); + return this; } } diff --git a/jetty-core/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/WebSocketHttpFieldsWrapper.java b/jetty-core/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/WebSocketHttpFieldsWrapper.java index 9024725c009..c169e9891e7 100644 --- a/jetty-core/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/WebSocketHttpFieldsWrapper.java +++ b/jetty-core/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/WebSocketHttpFieldsWrapper.java @@ -79,6 +79,7 @@ public class WebSocketHttpFieldsWrapper extends HttpFieldsWrapper if (HttpHeader.SEC_WEBSOCKET_EXTENSIONS.is(name)) { + // TODO: why add extensions?? _response.addExtensions(Collections.emptyList()); return false; } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/JettyWebSocketNegotiationTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/JettyWebSocketNegotiationTest.java index c28f11751d2..59583b7e17c 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/JettyWebSocketNegotiationTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/JettyWebSocketNegotiationTest.java @@ -34,7 +34,6 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.internal.HttpChannelState; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; @@ -112,7 +111,6 @@ public class JettyWebSocketNegotiationTest } } - @Disabled @Test public void testManualNegotiationInCreator() throws Exception {