Skip to content

Commit 5554385

Browse files
franz1981vietj
franz1981
authored and
vietj
committed
Improve HTTP validation for inlining and specialized types
1 parent bff88a0 commit 5554385

File tree

3 files changed

+278
-43
lines changed

3 files changed

+278
-43
lines changed

‎src/main/java/io/vertx/core/http/impl/HttpUtils.java‎

Lines changed: 169 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -863,67 +863,181 @@ public static void validateHeader(CharSequence name, Iterable<? extends CharSequ
863863
});
864864
}
865865

866-
public static void validateHeaderValue(CharSequence seq) {
866+
public static void validateHeaderValue(CharSequence value) {
867+
if (value instanceof AsciiString) {
868+
validateAsciiHeaderValue((AsciiString) value);
869+
} else if (value instanceof String) {
870+
validateStringHeaderValue((String) value);
871+
} else {
872+
validateSequenceHeaderValue(value);
873+
}
874+
}
875+
876+
private static void validateAsciiHeaderValue(AsciiString value) {
877+
final int length = value.length();
878+
if (length == 0) {
879+
return;
880+
}
881+
byte[] asciiChars = value.array();
882+
int off = value.arrayOffset();
883+
if (off == 0 && length == asciiChars.length) {
884+
for (int index = 0; index < asciiChars.length; index++) {
885+
int latinChar = asciiChars[index] & 0xFF;
886+
if (latinChar == 0x7F) {
887+
throw new IllegalArgumentException("a header value contains a prohibited character '127': " + value);
888+
}
889+
// non-printable chars are rare so let's make it a fall-back method, whilst still accepting HTAB
890+
if (latinChar < 32 && latinChar != 0x09) {
891+
validateSequenceHeaderValue(value, index - off);
892+
break;
893+
}
894+
}
895+
} else {
896+
validateAsciiRangeHeaderValue(value, off, length, asciiChars);
897+
}
898+
}
899+
900+
/**
901+
* This method is the slow-path generic version of {@link #validateAsciiHeaderValue(AsciiString)} which
902+
* is optimized for {@link AsciiString} instances which are backed by a 0-offset full-blown byte array.
903+
*/
904+
private static void validateAsciiRangeHeaderValue(AsciiString value, int off, int length, byte[] asciiChars) {
905+
int end = off + length;
906+
for (int index = off; index < end; index++) {
907+
int latinChar = asciiChars[index] & 0xFF;
908+
if (latinChar == 0x7F) {
909+
throw new IllegalArgumentException("a header value contains a prohibited character '127': " + value);
910+
}
911+
// non-printable chars are rare so let's make it a fall-back method, whilst still accepting HTAB
912+
if (latinChar < 32 && latinChar != 0x09) {
913+
validateSequenceHeaderValue(value, index - off);
914+
break;
915+
}
916+
}
917+
}
918+
919+
private static void validateStringHeaderValue(String value) {
920+
final int length = value.length();
921+
if (length == 0) {
922+
return;
923+
}
924+
925+
for (int index = 0; index < length; index++) {
926+
char latinChar = value.charAt(index);
927+
if (latinChar == 0x7F) {
928+
throw new IllegalArgumentException("a header value contains a prohibited character '127': " + value);
929+
}
930+
// non-printable chars are rare so let's make it a fall-back method, whilst still accepting HTAB
931+
if (latinChar < 32 && latinChar != 0x09) {
932+
validateSequenceHeaderValue(value, index);
933+
break;
934+
}
935+
}
936+
}
867937

868-
int state = 0;
869-
// Start looping through each of the character
870-
for (int index = 0; index < seq.length(); index++) {
871-
state = validateValueChar(seq, state, seq.charAt(index));
938+
private static void validateSequenceHeaderValue(CharSequence value) {
939+
final int length = value.length();
940+
if (length == 0) {
941+
return;
872942
}
873943

874-
if (state != 0) {
875-
throw new IllegalArgumentException("a header value must not end with '\\r' or '\\n':" + seq);
944+
for (int index = 0; index < length; index++) {
945+
char latinChar = value.charAt(index);
946+
if (latinChar == 0x7F) {
947+
throw new IllegalArgumentException("a header value contains a prohibited character '127': " + value);
948+
}
949+
// non-printable chars are rare so let's make it a fall-back method, whilst still accepting HTAB
950+
if (latinChar < 32 && latinChar != 0x09) {
951+
validateSequenceHeaderValue(value, index);
952+
break;
953+
}
876954
}
877955
}
878956

879957
private static final int HIGHEST_INVALID_VALUE_CHAR_MASK = ~0x1F;
958+
private static final int NO_CR_LF_STATE = 0;
959+
private static final int CR_STATE = 1;
960+
private static final int LF_STATE = 2;
961+
962+
/**
963+
* This method is taken as we need to validate the header value for the non-printable characters.
964+
*/
965+
private static void validateSequenceHeaderValue(CharSequence seq, int index) {
966+
// we already expect the very-first character to be non-printable
967+
int state = validateValueChar(seq, NO_CR_LF_STATE, seq.charAt(index));
968+
for (int i = index + 1; i < seq.length(); i++) {
969+
state = validateValueChar(seq, state, seq.charAt(i));
970+
}
971+
if (state != NO_CR_LF_STATE) {
972+
throw new IllegalArgumentException("a header value must not end with '\\r' or '\\n':" + seq);
973+
}
974+
}
880975

881-
private static int validateValueChar(CharSequence seq, int state, char character) {
976+
private static int validateValueChar(CharSequence seq, int state, char ch) {
882977
/*
883978
* State:
884979
* 0: Previous character was neither CR nor LF
885980
* 1: The previous character was CR
886981
* 2: The previous character was LF
887982
*/
888-
if ((character & HIGHEST_INVALID_VALUE_CHAR_MASK) == 0 || character == 0x7F) { // 0x7F is "DEL".
889-
// The only characters allowed in the range 0x00-0x1F are : HTAB, LF and CR
890-
switch (character) {
891-
case 0x09: // Horizontal tab - HTAB
892-
case 0x0a: // Line feed - LF
893-
case 0x0d: // Carriage return - CR
894-
break;
895-
default:
896-
throw new IllegalArgumentException("a header value contains a prohibited character '" + (int) character + "': " + seq);
983+
if (ch == 0x7F) {
984+
throw new IllegalArgumentException("a header value contains a prohibited character '127': " + seq);
985+
}
986+
if ((ch & HIGHEST_INVALID_VALUE_CHAR_MASK) == 0) {
987+
// this is a rare scenario
988+
validateNonPrintableCtrlChar(seq, ch);
989+
// this can include LF and CR as they are non-printable characters
990+
if (state == NO_CR_LF_STATE) {
991+
// Check the CRLF (HT | SP) pattern
992+
switch (ch) {
993+
case '\r':
994+
return CR_STATE;
995+
case '\n':
996+
return LF_STATE;
997+
}
998+
return NO_CR_LF_STATE;
897999
}
8981000
}
1001+
if (state != NO_CR_LF_STATE) {
1002+
// this is a rare scenario
1003+
return validateCrLfChar(seq, state, ch);
1004+
} else {
1005+
return NO_CR_LF_STATE;
1006+
}
1007+
}
8991008

900-
// Check the CRLF (HT | SP) pattern
1009+
private static int validateCrLfChar(CharSequence seq, int state, char ch) {
9011010
switch (state) {
902-
case 0:
903-
switch (character) {
904-
case '\r':
905-
return 1;
906-
case '\n':
907-
return 2;
908-
}
909-
break;
910-
case 1:
911-
switch (character) {
912-
case '\n':
913-
return 2;
914-
default:
915-
throw new IllegalArgumentException("only '\\n' is allowed after '\\r': " + seq);
1011+
case CR_STATE:
1012+
if (ch == '\n') {
1013+
return LF_STATE;
9161014
}
917-
case 2:
918-
switch (character) {
1015+
throw new IllegalArgumentException("only '\\n' is allowed after '\\r': " + seq);
1016+
case LF_STATE:
1017+
switch (ch) {
9191018
case '\t':
9201019
case ' ':
921-
return 0;
1020+
// return to the normal state
1021+
return NO_CR_LF_STATE;
9221022
default:
9231023
throw new IllegalArgumentException("only ' ' and '\\t' are allowed after '\\n': " + seq);
9241024
}
1025+
default:
1026+
// this should never happen
1027+
throw new AssertionError();
1028+
}
1029+
}
1030+
1031+
private static void validateNonPrintableCtrlChar(CharSequence seq, int ch) {
1032+
// The only characters allowed in the range 0x00-0x1F are : HTAB, LF and CR
1033+
switch (ch) {
1034+
case 0x09: // Horizontal tab - HTAB
1035+
case 0x0a: // Line feed - LF
1036+
case 0x0d: // Carriage return - CR
1037+
break;
1038+
default:
1039+
throw new IllegalArgumentException("a header value contains a prohibited character '" + (int) ch + "': " + seq);
9251040
}
926-
return state;
9271041
}
9281042

9291043
private static final boolean[] VALID_H_NAME_ASCII_CHARS;
@@ -959,13 +1073,15 @@ private static int validateValueChar(CharSequence seq, int state, char character
9591073
public static void validateHeaderName(CharSequence value) {
9601074
if (value instanceof AsciiString) {
9611075
// no need to check for ASCII-ness anymore
962-
validateHeaderName((AsciiString) value);
1076+
validateAsciiHeaderName((AsciiString) value);
1077+
} else if(value instanceof String) {
1078+
validateStringHeaderName((String) value);
9631079
} else {
964-
validateHeaderName0(value);
1080+
validateSequenceHeaderName(value);
9651081
}
9661082
}
9671083

968-
private static void validateHeaderName(AsciiString value) {
1084+
private static void validateAsciiHeaderName(AsciiString value) {
9691085
final int len = value.length();
9701086
final int off = value.arrayOffset();
9711087
final byte[] asciiChars = value.array();
@@ -981,7 +1097,20 @@ private static void validateHeaderName(AsciiString value) {
9811097
}
9821098
}
9831099

984-
private static void validateHeaderName0(CharSequence value) {
1100+
private static void validateStringHeaderName(String value) {
1101+
for (int i = 0; i < value.length(); i++) {
1102+
final char c = value.charAt(i);
1103+
// Check to see if the character is not an ASCII character, or invalid
1104+
if (c > 0x7f) {
1105+
throw new IllegalArgumentException("a header name cannot contain non-ASCII character: " + value);
1106+
}
1107+
if (!VALID_H_NAME_ASCII_CHARS[c & 0x7F]) {
1108+
throw new IllegalArgumentException("a header name cannot contain some prohibited characters, such as : " + value);
1109+
}
1110+
}
1111+
}
1112+
1113+
private static void validateSequenceHeaderName(CharSequence value) {
9851114
for (int i = 0; i < value.length(); i++) {
9861115
final char c = value.charAt(i);
9871116
// Check to see if the character is not an ASCII character, or invalid

‎src/test/benchmarks/io/vertx/benchmarks/BenchmarkBase.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,6 @@
2929
@Threads(1)
3030
@BenchmarkMode(Mode.Throughput)
3131
@Fork(value = 1, jvmArgs = {
32-
"-XX:+UseBiasedLocking",
33-
"-XX:BiasedLockingStartupDelay=0",
34-
"-XX:+AggressiveOpts",
3532
"-Djmh.executor=CUSTOM",
3633
"-Djmh.executor.class=io.vertx.core.impl.VertxExecutorService"
3734
})

‎src/test/benchmarks/io/vertx/benchmarks/HeadersValidationBenchmark.java‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package io.vertx.benchmarks;
2+
3+
import java.util.concurrent.TimeUnit;
4+
import java.util.function.Consumer;
5+
6+
import org.openjdk.jmh.annotations.Benchmark;
7+
import org.openjdk.jmh.annotations.CompilerControl;
8+
import org.openjdk.jmh.annotations.Measurement;
9+
import org.openjdk.jmh.annotations.Param;
10+
import org.openjdk.jmh.annotations.Scope;
11+
import org.openjdk.jmh.annotations.Setup;
12+
import org.openjdk.jmh.annotations.State;
13+
import org.openjdk.jmh.annotations.Warmup;
14+
15+
import io.netty.handler.codec.DefaultHeaders;
16+
import io.netty.handler.codec.http.DefaultHttpHeadersFactory;
17+
import io.vertx.core.http.impl.HttpUtils;
18+
19+
@State(Scope.Thread)
20+
@Warmup(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
21+
@Measurement(iterations = 5, time = 400, timeUnit = TimeUnit.MILLISECONDS)
22+
public class HeadersValidationBenchmark extends BenchmarkBase {
23+
24+
@Param({ "true", "false" })
25+
public boolean ascii;
26+
27+
private CharSequence[] headerNames;
28+
private CharSequence[] headerValues;
29+
private static final DefaultHeaders.NameValidator<CharSequence> nettyNameValidator = DefaultHttpHeadersFactory.headersFactory().getNameValidator();
30+
private static final DefaultHeaders.ValueValidator<CharSequence> nettyValueValidator = DefaultHttpHeadersFactory.headersFactory().getValueValidator();
31+
private static final Consumer<CharSequence> vertxNameValidator = HttpUtils::validateHeaderName;
32+
private static final Consumer<CharSequence> vertxValueValidator = HttpUtils::validateHeaderValue;
33+
34+
@Setup
35+
public void setup() {
36+
headerNames = new CharSequence[4];
37+
headerValues = new CharSequence[4];
38+
headerNames[0] = io.vertx.core.http.HttpHeaders.CONTENT_TYPE;
39+
headerValues[0] = io.vertx.core.http.HttpHeaders.createOptimized("text/plain");
40+
headerNames[1] = io.vertx.core.http.HttpHeaders.CONTENT_LENGTH;
41+
headerValues[1] = io.vertx.core.http.HttpHeaders.createOptimized("20");
42+
headerNames[2] = io.vertx.core.http.HttpHeaders.SERVER;
43+
headerValues[2] = io.vertx.core.http.HttpHeaders.createOptimized("vert.x");
44+
headerNames[3] = io.vertx.core.http.HttpHeaders.DATE;
45+
headerValues[3] = io.vertx.core.http.HttpHeaders.createOptimized(HeadersUtils.DATE_FORMAT.format(new java.util.Date(0)));
46+
if (!ascii) {
47+
for (int i = 0; i < headerNames.length; i++) {
48+
headerNames[i] = headerNames[i].toString();
49+
}
50+
}
51+
if (!ascii) {
52+
for (int i = 0; i < headerValues.length; i++) {
53+
headerValues[i] = headerValues[i].toString();
54+
}
55+
}
56+
}
57+
58+
@Benchmark
59+
public void validateNameNetty() {
60+
for (CharSequence headerName : headerNames) {
61+
nettyNameValidation(headerName);
62+
}
63+
}
64+
65+
@CompilerControl(CompilerControl.Mode.DONT_INLINE)
66+
private void nettyNameValidation(CharSequence headerName) {
67+
nettyNameValidator.validateName(headerName);
68+
}
69+
70+
@Benchmark
71+
public void validateNameVertx() {
72+
for (CharSequence headerName : headerNames) {
73+
vertxNameValidation(headerName);
74+
}
75+
}
76+
77+
@CompilerControl(CompilerControl.Mode.DONT_INLINE)
78+
private void vertxNameValidation(CharSequence headerName) {
79+
vertxNameValidator.accept(headerName);
80+
}
81+
82+
83+
@Benchmark
84+
public void validateValueNetty() {
85+
for (CharSequence headerValue : headerValues) {
86+
nettyValueValidation(headerValue);
87+
}
88+
}
89+
90+
@CompilerControl(CompilerControl.Mode.DONT_INLINE)
91+
private void nettyValueValidation(CharSequence headerValue) {
92+
nettyValueValidator.validate(headerValue);
93+
}
94+
95+
@Benchmark
96+
public void validateValueVertx() {
97+
for (CharSequence headerValue : headerValues) {
98+
vertxValueValidation(headerValue);
99+
}
100+
}
101+
102+
@CompilerControl(CompilerControl.Mode.DONT_INLINE)
103+
private void vertxValueValidation(CharSequence headerValue) {
104+
vertxValueValidator.accept(headerValue);
105+
}
106+
107+
108+
109+
}

0 commit comments

Comments
 (0)