diff --git a/zap/src/main/java/org/parosproxy/paros/network/HttpMessage.java b/zap/src/main/java/org/parosproxy/paros/network/HttpMessage.java index 4f2497aecd1..e843ac35a11 100644 --- a/zap/src/main/java/org/parosproxy/paros/network/HttpMessage.java +++ b/zap/src/main/java/org/parosproxy/paros/network/HttpMessage.java @@ -66,6 +66,7 @@ package org.parosproxy.paros.network; import java.net.HttpCookie; +import java.nio.charset.Charset; import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; @@ -504,7 +505,9 @@ private static void setBodyCharset(HttpBody body, HttpHeader header) { String charset = header.getCharset(); body.setCharset(charset); - if (charset != null && !charset.equalsIgnoreCase(body.getCharset())) { + if (charset != null + && !charset.equalsIgnoreCase(body.getCharset()) + && !isCharsetSupported(charset)) { LOGGER.warn( "Failed to set charset {} from content-type value: {}", charset, @@ -512,6 +515,14 @@ private static void setBodyCharset(HttpBody body, HttpHeader header) { } } + private static boolean isCharsetSupported(String charset) { + try { + return Charset.isSupported(charset); + } catch (IllegalArgumentException ignore) { + return false; + } + } + /** * Sets the given string body as the response body. * diff --git a/zap/src/test/java/org/parosproxy/paros/network/HttpMessageUnitTest.java b/zap/src/test/java/org/parosproxy/paros/network/HttpMessageUnitTest.java index bdb45d01c4b..151cf27e150 100644 --- a/zap/src/test/java/org/parosproxy/paros/network/HttpMessageUnitTest.java +++ b/zap/src/test/java/org/parosproxy/paros/network/HttpMessageUnitTest.java @@ -21,8 +21,10 @@ import static java.util.Arrays.asList; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; @@ -39,6 +41,10 @@ import java.util.List; import java.util.stream.Stream; import org.apache.commons.httpclient.URI; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.Logger; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.Configurator; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -55,14 +61,18 @@ import org.zaproxy.zap.network.HttpEncodingGzip; import org.zaproxy.zap.network.HttpRequestBody; import org.zaproxy.zap.network.HttpResponseBody; +import org.zaproxy.zap.testutils.Log4jTestAppender; import org.zaproxy.zap.users.User; /** Unit test for {@link HttpMessage}. */ class HttpMessageUnitTest { + private Log4jTestAppender testAppender; + @AfterEach - void cleanUp() { + void cleanUp() throws Exception { HttpMessage.setContentEncodingsHandler(null); + Configurator.reconfigure(getClass().getResource("/log4j2-test.properties").toURI()); } @Test @@ -577,23 +587,52 @@ void shouldNotBeWebSocketUpgradeIfResponseMissUpgradeHeader() throws Exception { @ParameterizedTest @MethodSource(value = "setBodyData") void shouldUseContentTypeCharsetWhenSettingRequestBody( - String charset, String body, String expectedBody) throws Exception { + String charset, String expectedCharset, String body, String expectedBody) + throws Exception { // Given HttpMessage message = new HttpMessage(); message.setRequestHeader( new HttpRequestHeader( "GET / HTTP/1.1\r\nContent-Type: text/plain; charset=" + charset)); + withLoggerAppender(); // When message.setRequestBody(body); // Then - assertThat(message.getRequestBody().getCharset(), is(equalTo(charset))); + assertThat(message.getRequestBody().getCharset(), is(equalTo(expectedCharset))); assertThat(message.getRequestBody().toString(), is(equalTo(expectedBody))); + assertThat(testAppender.getLogEvents(), hasSize(0)); + } + + @ParameterizedTest + @ValueSource(strings = {"unknown", "!~invalid~name~"}) + void shouldUseDefaultAndWarnOnUnknownCharsetWhenSettingRequestBody(String charset) + throws Exception { + // Given + HttpMessage message = new HttpMessage(); + message.setRequestHeader( + new HttpRequestHeader( + "GET / HTTP/1.1\r\nContent-Type: text/plain; charset=" + charset)); + withLoggerAppender(); + // When + message.setRequestBody("Body"); + // Then + assertThat(message.getRequestBody().getCharset(), is(equalTo("ISO-8859-1"))); + assertThat(message.getRequestBody().toString(), is(equalTo("Body"))); + assertThat(testAppender.getLogEvents(), hasSize(1)); + assertThat( + testAppender.getLogEvents().get(0).getMessage(), + containsString("Failed to set charset")); } static Stream setBodyData() { + String iso8851 = StandardCharsets.ISO_8859_1.name(); + String utf8 = StandardCharsets.UTF_8.name(); return Stream.of( - arguments(StandardCharsets.ISO_8859_1.name(), "J/ψ → VP", "J/????VP"), - arguments(StandardCharsets.UTF_8.name(), "J/ψ → VP", "J/ψ → VP")); + arguments(iso8851, iso8851, "J/ψ → VP", "J/????VP"), + arguments(utf8, utf8, "J/ψ → VP", "J/ψ → VP"), + // Check aliases work as well and don't cause warns + arguments("ISO_8859-1:1987", iso8851, "J/ψ → VP", "J/????VP"), + arguments("utf8", utf8, "J/ψ → VP", "J/ψ → VP")); } static Stream getNonPostMethods() { @@ -605,17 +644,41 @@ static Stream getNonPostMethods() { @ParameterizedTest @MethodSource(value = "setBodyData") void shouldUseContentTypeCharsetWhenSettingResponseBody( - String charset, String body, String expectedBody) throws Exception { + String charset, String expectedCharset, String body, String expectedBody) + throws Exception { // Given HttpMessage message = new HttpMessage(); message.setResponseHeader( new HttpResponseHeader( "HTTP/1.1 200 OK\r\nContent-Type: text/plain; charset=" + charset)); + withLoggerAppender(); // When message.setResponseBody(body); // Then - assertThat(message.getResponseBody().getCharset(), is(equalTo(charset))); + assertThat(message.getResponseBody().getCharset(), is(equalTo(expectedCharset))); assertThat(message.getResponseBody().toString(), is(equalTo(expectedBody))); + assertThat(testAppender.getLogEvents(), hasSize(0)); + } + + @ParameterizedTest + @ValueSource(strings = {"unknown", "!~invalid~name~"}) + void shouldUseDefaultAndWarnOnUnknownCharsetWhenSettingResponseBody(String charset) + throws Exception { + // Given + HttpMessage message = new HttpMessage(); + message.setResponseHeader( + new HttpResponseHeader( + "HTTP/1.1 200 OK\r\nContent-Type: text/plain; charset=" + charset)); + withLoggerAppender(); + // When + message.setResponseBody("Body"); + // Then + assertThat(message.getResponseBody().getCharset(), is(equalTo("UTF-8"))); + assertThat(message.getResponseBody().toString(), is(equalTo("Body"))); + assertThat(testAppender.getLogEvents(), hasSize(1)); + assertThat( + testAppender.getLogEvents().get(0).getMessage(), + containsString("Failed to set charset")); } private static HttpMessage newHttpMessage() throws Exception { @@ -644,4 +707,12 @@ private static List encodings(HttpBody body) { verify(body).setContentEncodings(arg.capture()); return arg.getValue(); } + + private void withLoggerAppender() { + testAppender = new Log4jTestAppender(); + LoggerContext context = LoggerContext.getContext(); + Logger logger = context.getLogger(HttpMessage.class.getCanonicalName()); + context.getConfiguration().addLoggerAppender(logger, testAppender); + logger.setLevel(Level.WARN); + } }