From 8331809c4020d7762b4d0507d64b21851f3dc0c3 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 3 Oct 2023 03:45:40 +0200 Subject: [PATCH] Fix getCharacterEncoding issue with #10563 (#10650) Do not persist a defaulted charset used in the request. Throw `UnsupportedEncodingException` from `getReader` --- .../org/eclipse/jetty/http/MimeTypes.java | 8 ++- .../org/eclipse/jetty/server/Request.java | 11 +++- .../jetty/ee10/servlet/ServletApiRequest.java | 51 +++++++++++++---- .../jetty/ee10/servlet/RequestTest.java | 57 +++++++++++++++++++ .../eclipse/jetty/ee9/nested/RequestTest.java | 49 ++++++++++++++++ 5 files changed, 163 insertions(+), 13 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java index c663130beb1..12eecc60145 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java @@ -18,6 +18,7 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.UnsupportedEncodingException; import java.nio.charset.Charset; +import java.nio.charset.IllegalCharsetNameException; import java.nio.charset.StandardCharsets; import java.nio.charset.UnsupportedCharsetException; import java.util.Arrays; @@ -329,8 +330,13 @@ public class MimeTypes * Get the explicit, assumed, or inferred Charset for a mime type * @param mimeType String form or a mimeType * @return A {@link Charset} or null; + * @throws IllegalCharsetNameException + * If the given charset name is illegal + * @throws UnsupportedCharsetException + * If no support for the named charset is available + * in this instance of the Java virtual machine */ - public Charset getCharset(String mimeType) + public Charset getCharset(String mimeType) throws IllegalCharsetNameException, UnsupportedCharsetException { if (mimeType == null) return null; diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index c8fa53efedd..93c8112c409 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -19,6 +19,8 @@ import java.net.InetSocketAddress; import java.net.SocketAddress; import java.nio.ByteBuffer; import java.nio.charset.Charset; +import java.nio.charset.IllegalCharsetNameException; +import java.nio.charset.UnsupportedCharsetException; import java.security.Principal; import java.util.ArrayList; import java.util.List; @@ -501,7 +503,14 @@ public interface Request extends Attributes, Content.Source return Content.Source.asInputStream(request); } - static Charset getCharset(Request request) + /** + * Get a {@link Charset} from the request {@link HttpHeader#CONTENT_TYPE}, if any. + * @param request The request. + * @return A {@link Charset} or null + * @throws IllegalCharsetNameException If the charset name is illegal + * @throws UnsupportedCharsetException If no support for the charset is available + */ + static Charset getCharset(Request request) throws IllegalCharsetNameException, UnsupportedCharsetException { String contentType = request.getHeaders().get(HttpHeader.CONTENT_TYPE); return Objects.requireNonNullElse(request.getContext().getMimeTypes(), MimeTypes.DEFAULTS).getCharset(contentType); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java index b051b89cc34..986bcb6a663 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java @@ -19,7 +19,9 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.UnsupportedEncodingException; import java.nio.charset.Charset; +import java.nio.charset.IllegalCharsetNameException; import java.nio.charset.StandardCharsets; +import java.nio.charset.UnsupportedCharsetException; import java.security.Principal; import java.util.AbstractList; import java.util.Collection; @@ -717,8 +719,15 @@ public class ServletApiRequest implements HttpServletRequest @Override public String getCharacterEncoding() { - if (_charset == null) - _charset = Request.getCharset(getRequest()); + try + { + if (_charset == null) + _charset = Request.getCharset(getRequest()); + } + catch (IllegalCharsetNameException | UnsupportedCharsetException e) + { + return MimeTypes.getCharsetFromContentType(getRequest().getHeaders().get(HttpHeader.CONTENT_TYPE)); + } if (_charset == null) return getServletRequestInfo().getServletContext().getServletContext().getRequestCharacterEncoding(); @@ -1027,18 +1036,38 @@ public class ServletApiRequest implements HttpServletRequest if (_inputState == ServletContextRequest.INPUT_READER) return _reader; - if (_charset == null) - _charset = Request.getCharset(getRequest()); - if (_charset == null) - _charset = getRequest().getContext().getMimeTypes().getCharset(getServletRequestInfo().getServletContext().getServletContextHandler().getDefaultRequestCharacterEncoding()); - if (_charset == null) - _charset = StandardCharsets.ISO_8859_1; + Charset charset = _charset; + try + { + if (charset == null) + { + charset = _charset = Request.getCharset(getRequest()); + if (charset == null) + charset = StandardCharsets.ISO_8859_1; + } - if (_reader == null || !_charset.equals(_readerCharset)) + } + catch (IllegalCharsetNameException | UnsupportedCharsetException e) + { + throw new UnsupportedEncodingException(e.getMessage()) + { + { + initCause(e); + } + + @Override + public String toString() + { + return "%s@%x:%s".formatted(UnsupportedEncodingException.class.getName(), hashCode(), getMessage()); + } + }; + } + + if (_reader == null || !charset.equals(_readerCharset)) { ServletInputStream in = getInputStream(); - _readerCharset = _charset; - _reader = new BufferedReader(new InputStreamReader(in, _charset)) + _readerCharset = charset; + _reader = new BufferedReader(new InputStreamReader(in, charset)) { @Override public void close() throws IOException diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java index 0f7bc853d9e..af7d74676e3 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/RequestTest.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.ee10.servlet; import java.io.IOException; +import java.io.UnsupportedEncodingException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -33,6 +34,7 @@ import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.component.LifeCycle; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; @@ -41,6 +43,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; public class RequestTest @@ -282,4 +285,58 @@ public class RequestTest assertThat(cookieHistory.get(0), sameInstance(cookieHistory.get(2))); assertThat(cookieHistory.get(2), not(sameInstance(cookieHistory.get(4)))); } + + @Test + public void testGetCharacterEncoding() throws Exception + { + startServer(new HttpServlet() + { + @Override + protected void service(HttpServletRequest request, HttpServletResponse resp) throws IOException + { + // No character encoding specified + request.getReader(); + // Try setting after read has been obtained + request.setCharacterEncoding("ISO-8859-2"); + assertThat(request.getCharacterEncoding(), nullValue()); + } + }); + + String rawResponse = _connector.getResponse( + """ + GET /test HTTP/1.1\r + Host: host\r + Connection: close\r + \r + """); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + } + + @Test + public void testUnknownCharacterEncoding() throws Exception + { + startServer(new HttpServlet() + { + @Override + protected void service(HttpServletRequest request, HttpServletResponse resp) throws IOException + { + assertThat(request.getCharacterEncoding(), is("Unknown")); + Assertions.assertThrows(UnsupportedEncodingException.class, request::getReader); + } + }); + + String rawResponse = _connector.getResponse( + """ + POST /test HTTP/1.1\r + Host: host\r + Content-Type:text/plain; charset=Unknown\r + Content-Length: 10\r + Connection: close\r + \r + 1234567890\r + """); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + } } diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java index 2380c258f1c..64451816d1b 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java +++ b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/RequestTest.java @@ -88,6 +88,7 @@ import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.NanoTime; import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -2493,4 +2494,52 @@ public class RequestTest } } } + + @Test + public void testGetCharacterEncoding() throws Exception + { + _handler._checker = (request, response) -> + { + // No character encoding specified + request.getReader(); + // Try setting after read has been obtained + request.setCharacterEncoding("ISO-8859-2"); + assertThat(request.getCharacterEncoding(), nullValue()); + return true; + }; + + String rawResponse = _connector.getResponse( + """ + GET /test HTTP/1.1\r + Host: host\r + Connection: close\r + \r + """); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + } + + @Test + public void testUnknownCharacterEncoding() throws Exception + { + _handler._checker = (request, response) -> + { + assertThat(request.getCharacterEncoding(), is("Unknown")); + Assertions.assertThrows(UnsupportedEncodingException.class, request::getReader); + return true; + }; + + String rawResponse = _connector.getResponse( + """ + POST /test HTTP/1.1\r + Host: host\r + Content-Type:multipart/form-data; charset=Unknown\r + Content-Length: 10\r + Connection: close\r + \r + 1234567890\r + """); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + } }