From d448bce5592f8b52ccda14b775c719563a8ee044 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 21 Nov 2024 10:18:19 +1100 Subject: [PATCH 1/4] Improve documentation for ambiguous URIs --- .../pages/server/compliance.adoc | 9 +++ .../org/eclipse/jetty/start/usage.txt | 68 ++++++++++++------- .../jetty/ee10/servlet/ServletHandler.java | 8 ++- 3 files changed, 58 insertions(+), 27 deletions(-) diff --git a/documentation/jetty/modules/programming-guide/pages/server/compliance.adoc b/documentation/jetty/modules/programming-guide/pages/server/compliance.adoc index 6707b836b86b..fac52dbf85a8 100644 --- a/documentation/jetty/modules/programming-guide/pages/server/compliance.adoc +++ b/documentation/jetty/modules/programming-guide/pages/server/compliance.adoc @@ -97,6 +97,15 @@ If you want to customize the violations that you want to allow, you can create y include::code:example$src/main/java/org/eclipse/jetty/docs/programming/server/ServerDocs.java[tags=uriComplianceCustom] ---- +[[servleturi]] +=== Servlet URI Compliance Modes + +Even if the server has been configured (as above) to allow ambiguous URIs to be received, individual Servlet Contexts may not allow such ambiguous URIs to be returned via some specific methods. +Specifically the `HttpServletRequest` methods: link:https://jakarta.ee/specifications/servlet/6.0/apidocs/jakarta.servlet/jakarta/servlet/http/httpservletrequest#getServletPath()[getServletPath()] and link:https://jakarta.ee/specifications/servlet/6.0/apidocs/jakarta.servlet/jakarta/servlet/http/httpservletrequest#getPathInfo()[getPathInfo()], may throw `IllegalArgumentException` for such URIs. +The intention is for safer methods, such as link:https://jakarta.ee/specifications/servlet/6.0/apidocs/jakarta.servlet/jakarta/servlet/http/httpservletrequest#getRequestURI()[getRequestURI] to be used instead. + +If necessary, the `ServletHandler` can be configured to allow ambiguous URIs from all methods with link:{javadoc-url}/org/eclipse/jetty/ee10/servlet/ServletHandler.html#setDecodeAmbiguousURIs(boolean)[setDecodeAmbiguousURIs(boolean)]. + [[cookie]] == Cookie Compliance Modes diff --git a/jetty-core/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt b/jetty-core/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt index 66304c35c142..c333b6f79f0a 100644 --- a/jetty-core/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt +++ b/jetty-core/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt @@ -135,6 +135,10 @@ Configure Commands: Options: -------- + --env= + Sets the environment which will apply to all subsequent libraries, + properties and XML files. + --modules=(,)* Enables a module for this execution. To enable a module for all future executions, use the @@ -145,7 +149,8 @@ Options: --libs= Adds the specified class-path entries to the the server - class-path (or module-path). + class-path; or module-path; or environment classpath if + there has been a prior --env argument. --files=| --download=| @@ -168,7 +173,6 @@ Options: and any `--include-jetty-dir` and finally checking the `${jetty.home}`) - --exec Executes the generated command line in a forked JVM (see the --dry-run command). @@ -234,6 +238,42 @@ Options: configuration to all specific ${jetty.base} directories without having to modify ${jetty.home}. + -D= + Specifies a system property, as well as a start property. + Note: this is a program argument that is interpreted and + added to the existing JVM system properties. + + .xml + Specifies a Jetty XML file relative to ${jetty.base}. + This file is in addition to the Jetty XML files resolved + from the [xml] sections of the enabled modules. + If there has been a prior --env argument, the XML will be + executed within that environment + + = + Specifies a property value that overrides the same + property defined in a ${jetty.base}/start.d/*.ini file, + or in the [ini] section of a *.mod file. + + = + Sets the property value unconditionally. + += + Appends the given value to the existing value. + ?= + Sets the property value only if it is not already set. + + If there has been a prior --env argument, then the property is + set only for the current environment, otherwise it is set for + the whole server. + + .properties + Specifies a file of property assignments that overrides the same + property defined in a ${jetty.base}/start.d/*.ini file, + or in the [ini] section of a *.mod file. + If there has been a prior --env argument, then the property is + set only for the current environment, otherwise it is set for + the whole server. + jetty.home= Sets the ${jetty.home} directory. By default it is resolved from the start.jar file path. @@ -272,26 +312,4 @@ Options: maven.repo.uri= The base URL to use to download Maven dependencies. - Defaults to: https://repo1.maven.org/maven2/. - - = - Specifies a property value that overrides the same - property defined in a ${jetty.base}/start.d/*.ini file, - or in the [ini] section of a *.mod file. - - = - Sets the property value unconditionally. - += - Appends the given value to the existing value. - ?= - Sets the property value only if it is not already set. - - -D= - Specifies a system property, as well as a start property. - Note: this is a program argument that is interpreted and - added to the existing JVM system properties. - - - Specifies a Jetty XML file relative to ${jetty.base}. - This file is in addition to the Jetty XML files resolved - from the [xml] sections of the enabled modules. + Defaults to: https://repo1.maven.org/maven2/. \ No newline at end of file diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java index f92103e7225a..670fb6cd44c3 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java @@ -43,6 +43,7 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.UriCompliance; import org.eclipse.jetty.http.pathmap.MappedResource; import org.eclipse.jetty.http.pathmap.MatchedPath; import org.eclipse.jetty.http.pathmap.MatchedResource; @@ -132,8 +133,11 @@ public boolean isDecodeAmbiguousURIs() } /** - * @param decodeAmbiguousURIs {@code True} if ambiguous URIs are decoded by {@link ServletApiRequest#getServletPath()} - * and {@link ServletApiRequest#getPathInfo()}. + * Allow or disallow ambiguous URIs to be returned by {@link ServletApiRequest#getServletPath()} + * and {@link ServletApiRequest#getPathInfo()}. Note that the + * {@link org.eclipse.jetty.server.HttpConfiguration#setUriCompliance(UriCompliance)} must also be set to + * allow ambiguous URIs to be accepted by the {@link org.eclipse.jetty.server.Connector}. + * @param decodeAmbiguousURIs {@code True} if ambiguous URIs are decoded by all servlet API methods. */ public void setDecodeAmbiguousURIs(boolean decodeAmbiguousURIs) { From a7b429cdc576c52498e9337ccef05ad0312ea7ae Mon Sep 17 00:00:00 2001 From: gregw Date: Tue, 26 Nov 2024 11:58:00 +1100 Subject: [PATCH 2/4] Improve documentation for ambiguous URIs updates from review --- .../pages/server/compliance.adoc | 4 +- .../jetty/ee10/servlet/ServletHandler.java | 9 ++-- .../servlet/SizeLimitHandlerServletTest.java | 53 +++++++++++++++++++ .../test/resources/jetty-logging.properties | 2 +- .../eclipse/jetty/ee9/nested/HttpInput.java | 5 +- 5 files changed, 64 insertions(+), 9 deletions(-) diff --git a/documentation/jetty/modules/programming-guide/pages/server/compliance.adoc b/documentation/jetty/modules/programming-guide/pages/server/compliance.adoc index fac52dbf85a8..69436b3c7737 100644 --- a/documentation/jetty/modules/programming-guide/pages/server/compliance.adoc +++ b/documentation/jetty/modules/programming-guide/pages/server/compliance.adoc @@ -100,8 +100,10 @@ include::code:example$src/main/java/org/eclipse/jetty/docs/programming/server/Se [[servleturi]] === Servlet URI Compliance Modes -Even if the server has been configured (as above) to allow ambiguous URIs to be received, individual Servlet Contexts may not allow such ambiguous URIs to be returned via some specific methods. +Even if the server has been configured (as above) to allow ambiguous URIs to be received, individual Servlet contexts may not allow such ambiguous URIs to be returned via some specific methods. + Specifically the `HttpServletRequest` methods: link:https://jakarta.ee/specifications/servlet/6.0/apidocs/jakarta.servlet/jakarta/servlet/http/httpservletrequest#getServletPath()[getServletPath()] and link:https://jakarta.ee/specifications/servlet/6.0/apidocs/jakarta.servlet/jakarta/servlet/http/httpservletrequest#getPathInfo()[getPathInfo()], may throw `IllegalArgumentException` for such URIs. + The intention is for safer methods, such as link:https://jakarta.ee/specifications/servlet/6.0/apidocs/jakarta.servlet/jakarta/servlet/http/httpservletrequest#getRequestURI()[getRequestURI] to be used instead. If necessary, the `ServletHandler` can be configured to allow ambiguous URIs from all methods with link:{javadoc-url}/org/eclipse/jetty/ee10/servlet/ServletHandler.html#setDecodeAmbiguousURIs(boolean)[setDecodeAmbiguousURIs(boolean)]. diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java index 670fb6cd44c3..f42d772b4392 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletHandler.java @@ -133,10 +133,11 @@ public boolean isDecodeAmbiguousURIs() } /** - * Allow or disallow ambiguous URIs to be returned by {@link ServletApiRequest#getServletPath()} - * and {@link ServletApiRequest#getPathInfo()}. Note that the - * {@link org.eclipse.jetty.server.HttpConfiguration#setUriCompliance(UriCompliance)} must also be set to - * allow ambiguous URIs to be accepted by the {@link org.eclipse.jetty.server.Connector}. + *

Allow or disallow ambiguous URIs to be returned by {@link ServletApiRequest#getServletPath()} + * and {@link ServletApiRequest#getPathInfo()}.

+ *

Note that the {@link org.eclipse.jetty.server.HttpConfiguration#setUriCompliance(UriCompliance)} + * must also be set to allow ambiguous URIs to be accepted by the {@link org.eclipse.jetty.server.Connector}.

+ * * @param decodeAmbiguousURIs {@code True} if ambiguous URIs are decoded by all servlet API methods. */ public void setDecodeAmbiguousURIs(boolean decodeAmbiguousURIs) diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SizeLimitHandlerServletTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SizeLimitHandlerServletTest.java index a9c191d1778b..a9efd45c37a0 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SizeLimitHandlerServletTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SizeLimitHandlerServletTest.java @@ -19,6 +19,7 @@ import java.net.URI; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Exchanger; import java.util.concurrent.TimeUnit; import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; @@ -26,10 +27,12 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.client.AsyncRequestContent; import org.eclipse.jetty.client.BytesRequestContent; import org.eclipse.jetty.client.ContentResponse; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.Request; +import org.eclipse.jetty.client.Response; import org.eclipse.jetty.client.Result; import org.eclipse.jetty.client.StringRequestContent; import org.eclipse.jetty.http.HttpHeader; @@ -38,6 +41,7 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.SizeLimitHandler; import org.eclipse.jetty.server.handler.gzip.GzipHandler; +import org.eclipse.jetty.util.Blocker; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; import org.junit.jupiter.api.AfterEach; @@ -144,6 +148,55 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I assertThat(response.getStatus(), equalTo(HttpStatus.PAYLOAD_TOO_LARGE_413)); } + @Test + public void testChunkedEcho() throws Exception + { + start(new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException + { + String requestContent = IO.toString(req.getInputStream()); + resp.getWriter().print(requestContent); + } + }); + + String content = "x".repeat(SIZE_LIMIT); + + URI uri = URI.create("http://localhost:" + _connector.getLocalPort()); + Exchanger exchanger = new Exchanger<>(); + AsyncRequestContent asyncRequestContent = new AsyncRequestContent(); + _client.POST(uri).body(asyncRequestContent).send(result -> + { + try + { + exchanger.exchange(result.getResponse()); + } + catch (InterruptedException e) + { + throw new RuntimeException(e); + } + }); + + try (Blocker.Callback callback = Blocker.callback()) + { + asyncRequestContent.write(false, BufferUtil.toBuffer(content), callback); + asyncRequestContent.flush(); + callback.block(); + } + try (Blocker.Callback callback = Blocker.callback()) + { + asyncRequestContent.write(true, BufferUtil.toBuffer(content), callback); + asyncRequestContent.flush(); + callback.block(); + } + asyncRequestContent.close(); + + Response response = exchanger.exchange(null); + assertThat(response.getStatus(), equalTo(HttpStatus.PAYLOAD_TOO_LARGE_413)); + } + + @Test public void testGzipEchoNoAcceptEncoding() throws Exception { diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/resources/jetty-logging.properties b/jetty-ee10/jetty-ee10-servlet/src/test/resources/jetty-logging.properties index 9c750192716e..b14d34fa0821 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/resources/jetty-logging.properties +++ b/jetty-ee10/jetty-ee10-servlet/src/test/resources/jetty-logging.properties @@ -1,5 +1,5 @@ #org.eclipse.jetty.LEVEL=DEBUG -#org.eclipse.jetty.http.LEVEL=DEBUG +org.eclipse.jetty.http.LEVEL=DEBUG #org.eclipse.jetty.server.LEVEL=DEBUG #org.eclipse.jetty.ee10.servlet.LEVEL=DEBUG #org.eclipse.jetty.server.ResourceService.LEVEL=DEBUG diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpInput.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpInput.java index e3c26fab2a6a..a4d81cd5cf67 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpInput.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpInput.java @@ -24,6 +24,7 @@ import org.eclipse.jetty.server.Context; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.ExceptionUtil; import org.eclipse.jetty.util.component.Destroyable; import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.Invocable; @@ -311,9 +312,7 @@ private int read(ByteBuffer buffer, byte[] b, int off, int len) throws IOExcepti LOG.debug("read error={} {}", error, this); if (error != null) { - if (error instanceof IOException) - throw (IOException)error; - throw new IOException(error); + ExceptionUtil.ifExceptionThrowAs(IOException.class, error); } if (content.isEof()) From 9c96d69186718d24167a03a909e2c847fe9825ff Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 28 Nov 2024 11:18:00 +1100 Subject: [PATCH 3/4] Improve documentation for ambiguous URIs updates from review --- .../main/java/org/eclipse/jetty/ee10/servlet/HttpInput.java | 5 ++--- .../src/test/resources/jetty-logging.properties | 2 +- .../main/java/org/eclipse/jetty/ee9/nested/HttpInput.java | 2 -- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpInput.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpInput.java index 4cff1c4dc9d2..23eeafd2519f 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpInput.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpInput.java @@ -22,6 +22,7 @@ import jakarta.servlet.ServletInputStream; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.server.Context; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.Invocable; import org.slf4j.Logger; @@ -276,9 +277,7 @@ private int read(ByteBuffer buffer, byte[] b, int off, int len) throws IOExcepti Throwable failure = chunk.getFailure(); if (LOG.isDebugEnabled()) LOG.debug("read failure={} {}", failure, this); - if (failure instanceof IOException) - throw (IOException)failure; - throw new IOException(failure); + throw IO.rethrow(failure); } // Empty and not a failure; can only be EOF as per ContentProducer.nextChunk() contract. diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/resources/jetty-logging.properties b/jetty-ee10/jetty-ee10-servlet/src/test/resources/jetty-logging.properties index b14d34fa0821..9c750192716e 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/resources/jetty-logging.properties +++ b/jetty-ee10/jetty-ee10-servlet/src/test/resources/jetty-logging.properties @@ -1,5 +1,5 @@ #org.eclipse.jetty.LEVEL=DEBUG -org.eclipse.jetty.http.LEVEL=DEBUG +#org.eclipse.jetty.http.LEVEL=DEBUG #org.eclipse.jetty.server.LEVEL=DEBUG #org.eclipse.jetty.ee10.servlet.LEVEL=DEBUG #org.eclipse.jetty.server.ResourceService.LEVEL=DEBUG diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpInput.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpInput.java index a4d81cd5cf67..82c7d24d9731 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpInput.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpInput.java @@ -311,9 +311,7 @@ private int read(ByteBuffer buffer, byte[] b, int off, int len) throws IOExcepti if (LOG.isDebugEnabled()) LOG.debug("read error={} {}", error, this); if (error != null) - { ExceptionUtil.ifExceptionThrowAs(IOException.class, error); - } if (content.isEof()) { From e7907bce0920f14d5544ae42908361cbf1d618e7 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 28 Nov 2024 16:08:11 +1100 Subject: [PATCH 4/4] updates from review --- .../eclipse/jetty/ee10/servlet/SizeLimitHandlerServletTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SizeLimitHandlerServletTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SizeLimitHandlerServletTest.java index a9efd45c37a0..64ebe1100aff 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SizeLimitHandlerServletTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SizeLimitHandlerServletTest.java @@ -196,7 +196,6 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I assertThat(response.getStatus(), equalTo(HttpStatus.PAYLOAD_TOO_LARGE_413)); } - @Test public void testGzipEchoNoAcceptEncoding() throws Exception {