Skip to content

Commit 26ca986

Browse files
psiinonpsiinon
authored
Merge pull request zaproxy#9082 from thc202/ct-warn
Warn once on invalid charset
2 parents fb2179f + b168d20 commit 26ca986

2 files changed

Lines changed: 134 additions & 4 deletions

File tree

‎zap/src/main/java/org/parosproxy/paros/network/HttpMessage.java‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,15 @@
6868
import java.net.HttpCookie;
6969
import java.nio.charset.Charset;
7070
import java.util.ArrayList;
71+
import java.util.Collections;
7172
import java.util.HashMap;
73+
import java.util.HashSet;
7274
import java.util.Iterator;
7375
import java.util.LinkedList;
7476
import java.util.List;
7577
import java.util.Locale;
7678
import java.util.Map;
79+
import java.util.Set;
7780
import java.util.SortedSet;
7881
import java.util.TreeSet;
7982
import java.util.Vector;
@@ -156,6 +159,9 @@ public class HttpMessage implements Message {
156159
*/
157160
private boolean responseFromTargetHost = false;
158161

162+
private static final Set<String> WARNED_CONTENT_TYPE_VALUES =
163+
Collections.synchronizedSet(new HashSet<>());
164+
159165
public HistoryReference getHistoryRef() {
160166
return historyRef;
161167
}
@@ -508,13 +514,25 @@ private static void setBodyCharset(HttpBody body, HttpHeader header) {
508514
if (charset != null
509515
&& !charset.equalsIgnoreCase(body.getCharset())
510516
&& !isCharsetSupported(charset)) {
511-
LOGGER.warn(
512-
"Failed to set charset {} from content-type value: {}",
513-
charset,
514-
header.getNormalisedContentTypeValue());
517+
String contentTypeValue = header.getNormalisedContentTypeValue();
518+
if (WARNED_CONTENT_TYPE_VALUES.add(contentTypeValue)) {
519+
LOGGER.warn(
520+
"Failed to set charset {} from content-type value: {}",
521+
charset,
522+
header.getNormalisedContentTypeValue());
523+
}
515524
}
516525
}
517526

527+
/**
528+
* Resets the set of warned content-type values.
529+
*
530+
* <p><strong>Note:</strong> Not part of the public API.
531+
*/
532+
public static void resetWarnedContentTypeValues() {
533+
WARNED_CONTENT_TYPE_VALUES.clear();
534+
}
535+
518536
private static boolean isCharsetSupported(String charset) {
519537
try {
520538
return Charset.isSupported(charset);

‎zap/src/test/java/org/parosproxy/paros/network/HttpMessageUnitTest.java‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import static java.util.Arrays.asList;
2323
import static org.hamcrest.MatcherAssert.assertThat;
24+
import static org.hamcrest.Matchers.allOf;
2425
import static org.hamcrest.Matchers.containsString;
2526
import static org.hamcrest.Matchers.empty;
2627
import static org.hamcrest.Matchers.equalTo;
@@ -72,6 +73,7 @@ class HttpMessageUnitTest {
7273
@AfterEach
7374
void cleanUp() throws Exception {
7475
HttpMessage.setContentEncodingsHandler(null);
76+
HttpMessage.resetWarnedContentTypeValues();
7577
Configurator.reconfigure(getClass().getResource("/log4j2-test.properties").toURI());
7678
}
7779

@@ -624,6 +626,61 @@ void shouldUseDefaultAndWarnOnUnknownCharsetWhenSettingRequestBody(String charse
624626
containsString("Failed to set charset"));
625627
}
626628

629+
@Test
630+
void shouldWarnOnceOnSameUnknownCharsetWhenSettingRequestBody() throws Exception {
631+
// Given
632+
HttpMessage message = new HttpMessage();
633+
message.setRequestHeader(
634+
new HttpRequestHeader(
635+
"GET / HTTP/1.1\r\nContent-Type: text/plain; charset=1st_unknown"));
636+
withLoggerAppender();
637+
// When
638+
message.setRequestBody("Body");
639+
message.setRequestBody("Body");
640+
message.setRequestHeader(
641+
new HttpRequestHeader(
642+
"GET / HTTP/1.1\r\nContent-Type: text/plain; charset=2nd_unknown"));
643+
message.setRequestBody("Body");
644+
message.setRequestBody("Body");
645+
// Then
646+
assertThat(testAppender.getLogEvents(), hasSize(2));
647+
assertThat(
648+
testAppender.getLogEvents().get(0).getMessage(),
649+
allOf(
650+
containsString("Failed to set charset"),
651+
containsString("charset=1st_unknown")));
652+
assertThat(
653+
testAppender.getLogEvents().get(1).getMessage(),
654+
allOf(
655+
containsString("Failed to set charset"),
656+
containsString("charset=2nd_unknown")));
657+
}
658+
659+
@Test
660+
void shouldWarnOnceAgainOnSameUnknownCharsetWhenSettingRequestBodyAfterResettingWarns()
661+
throws Exception {
662+
// Given
663+
HttpMessage message = new HttpMessage();
664+
message.setRequestHeader(
665+
new HttpRequestHeader(
666+
"GET / HTTP/1.1\r\nContent-Type: text/plain; charset=unknown"));
667+
withLoggerAppender();
668+
// When
669+
message.setRequestBody("Body");
670+
message.setRequestBody("Body");
671+
HttpMessage.resetWarnedContentTypeValues();
672+
message.setRequestBody("Body");
673+
message.setRequestBody("Body");
674+
// Then
675+
assertThat(testAppender.getLogEvents(), hasSize(2));
676+
assertThat(
677+
testAppender.getLogEvents().get(0).getMessage(),
678+
allOf(containsString("Failed to set charset"), containsString("charset=unknown")));
679+
assertThat(
680+
testAppender.getLogEvents().get(1).getMessage(),
681+
allOf(containsString("Failed to set charset"), containsString("charset=unknown")));
682+
}
683+
627684
static Stream<Arguments> setBodyData() {
628685
String iso8851 = StandardCharsets.ISO_8859_1.name();
629686
String utf8 = StandardCharsets.UTF_8.name();
@@ -681,6 +738,61 @@ void shouldUseDefaultAndWarnOnUnknownCharsetWhenSettingResponseBody(String chars
681738
containsString("Failed to set charset"));
682739
}
683740

741+
@Test
742+
void shouldWarnOnceOnSameUnknownCharsetWhenSettingResponseBody() throws Exception {
743+
// Given
744+
HttpMessage message = new HttpMessage();
745+
message.setResponseHeader(
746+
new HttpResponseHeader(
747+
"HTTP/1.1 200 OK\r\nContent-Type: text/plain; charset=1st_unknown"));
748+
withLoggerAppender();
749+
// When
750+
message.setResponseBody("Body");
751+
message.setResponseBody("Body");
752+
message.setResponseHeader(
753+
new HttpResponseHeader(
754+
"HTTP/1.1 200 OK\r\nContent-Type: text/plain; charset=2nd_unknown"));
755+
message.setResponseBody("Body");
756+
message.setResponseBody("Body");
757+
// Then
758+
assertThat(testAppender.getLogEvents(), hasSize(2));
759+
assertThat(
760+
testAppender.getLogEvents().get(0).getMessage(),
761+
allOf(
762+
containsString("Failed to set charset"),
763+
containsString("charset=1st_unknown")));
764+
assertThat(
765+
testAppender.getLogEvents().get(1).getMessage(),
766+
allOf(
767+
containsString("Failed to set charset"),
768+
containsString("charset=2nd_unknown")));
769+
}
770+
771+
@Test
772+
void shouldWarnOnceAgainOnSameUnknownCharsetWhenSettingResponseBodyAfterResettingWarns()
773+
throws Exception {
774+
// Given
775+
HttpMessage message = new HttpMessage();
776+
message.setResponseHeader(
777+
new HttpResponseHeader(
778+
"HTTP/1.1 200 OK\r\nContent-Type: text/plain; charset=unknown"));
779+
withLoggerAppender();
780+
// When
781+
message.setResponseBody("Body");
782+
message.setResponseBody("Body");
783+
HttpMessage.resetWarnedContentTypeValues();
784+
message.setResponseBody("Body");
785+
message.setResponseBody("Body");
786+
// Then
787+
assertThat(testAppender.getLogEvents(), hasSize(2));
788+
assertThat(
789+
testAppender.getLogEvents().get(0).getMessage(),
790+
allOf(containsString("Failed to set charset"), containsString("charset=unknown")));
791+
assertThat(
792+
testAppender.getLogEvents().get(1).getMessage(),
793+
allOf(containsString("Failed to set charset"), containsString("charset=unknown")));
794+
}
795+
684796
private static HttpMessage newHttpMessage() throws Exception {
685797
HttpMessage message =
686798
new HttpMessage(

0 commit comments

Comments
 (0)