Skip to content

Commit dfa0134

Browse files
committed
Modify HTTPRequestParser to use a a byte buffer before converting to a String
Update value character validator Add test
1 parent 3a50dce commit dfa0134

File tree

4 files changed

+66
-16
lines changed

4 files changed

+66
-16
lines changed

java-http.iml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,11 @@
7272
<orderEntry type="module-library" scope="TEST">
7373
<library>
7474
<CLASSES>
75-
<root url="jar://$MODULE_DIR$/.savant/cache/org/testng/testng/7.8.0/testng-7.8.0.jar!/" />
75+
<root url="jar://$MODULE_DIR$/.savant/cache/org/testng/testng/7.10.2/testng-7.10.2.jar!/" />
7676
</CLASSES>
7777
<JAVADOC />
7878
<SOURCES>
79-
<root url="jar://$MODULE_DIR$/.savant/cache/org/testng/testng/7.8.0/testng-7.8.0-src.jar!/" />
79+
<root url="jar://$MODULE_DIR$/.savant/cache/org/testng/testng/7.10.2/testng-7.10.2-src.jar!/" />
8080
</SOURCES>
8181
</library>
8282
</orderEntry>
@@ -105,11 +105,11 @@
105105
<orderEntry type="module-library" scope="TEST">
106106
<library>
107107
<CLASSES>
108-
<root url="jar://$MODULE_DIR$/.savant/cache/org/webjars/jquery/3.6.1/jquery-3.6.1.jar!/" />
108+
<root url="jar://$MODULE_DIR$/.savant/cache/org/webjars/jquery/3.7.1/jquery-3.7.1.jar!/" />
109109
</CLASSES>
110110
<JAVADOC />
111111
<SOURCES>
112-
<root url="jar://$MODULE_DIR$/.savant/cache/org/webjars/jquery/3.6.1/jquery-3.6.1-src.jar!/" />
112+
<root url="jar://$MODULE_DIR$/.savant/cache/org/webjars/jquery/3.7.1/jquery-3.7.1-src.jar!/" />
113113
</SOURCES>
114114
</library>
115115
</orderEntry>

src/main/java/io/fusionauth/http/server/HTTPRequestProcessor.java

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
*/
1616
package io.fusionauth.http.server;
1717

18+
import java.io.ByteArrayOutputStream;
1819
import java.nio.ByteBuffer;
20+
import java.nio.charset.StandardCharsets;
1921

2022
import io.fusionauth.http.HTTPMethod;
2123
import io.fusionauth.http.HTTPValues.Headers;
@@ -34,8 +36,8 @@
3436
public class HTTPRequestProcessor {
3537
private final int bufferSize;
3638

37-
// TODO : Should this be sized with a configuration parameter?
38-
private final StringBuilder builder = new StringBuilder();
39+
// Allocate a 4k buffer for starters, it will grow as needed.
40+
private final ByteArrayOutputStream byteBuffer = new ByteArrayOutputStream(4096);
3941

4042
private final HTTPServerConfiguration configuration;
4143

@@ -78,28 +80,27 @@ public RequestState processBodyBytes() {
7880

7981
public RequestState processPreambleBytes(ByteBuffer buffer) {
8082
while (buffer.hasRemaining()) {
81-
// TODO : Can we get some performance using ByteBuffer rather than StringBuilder here?
8283

8384
// If there is a state transition, store the value properly and reset the builder (if needed)
8485
byte ch = buffer.get();
8586
RequestPreambleState nextState = preambleState.next(ch);
8687
if (nextState != preambleState) {
8788
switch (preambleState) {
88-
case RequestMethod -> request.setMethod(HTTPMethod.of(builder.toString()));
89-
case RequestPath -> request.setPath(builder.toString());
90-
case RequestProtocol -> request.setProtocol(builder.toString());
91-
case HeaderName -> headerName = builder.toString();
92-
case HeaderValue -> request.addHeader(headerName, builder.toString());
89+
case RequestMethod -> request.setMethod(HTTPMethod.of(byteBuffer.toString(StandardCharsets.UTF_8)));
90+
case RequestPath -> request.setPath(byteBuffer.toString(StandardCharsets.UTF_8));
91+
case RequestProtocol -> request.setProtocol(byteBuffer.toString(StandardCharsets.UTF_8));
92+
case HeaderName -> headerName = byteBuffer.toString(StandardCharsets.UTF_8);
93+
case HeaderValue -> request.addHeader(headerName, byteBuffer.toString(StandardCharsets.UTF_8));
9394
}
9495

9596
// If the next state is storing, reset the builder
9697
if (nextState.store()) {
97-
builder.delete(0, builder.length());
98-
builder.appendCodePoint(ch);
98+
byteBuffer.reset();
99+
byteBuffer.write(ch);
99100
}
100101
} else if (preambleState.store()) {
101102
// If the current state is storing, store the character
102-
builder.appendCodePoint(ch);
103+
byteBuffer.write(ch);
103104
}
104105

105106
preambleState = nextState;

src/main/java/io/fusionauth/http/util/HTTPTools.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,10 @@ public static boolean isURICharacter(byte ch) {
119119
return ch >= '!' && ch <= '~';
120120
}
121121

122+
// RFC9110 section-5.5 allows for "obs-text", which includes 0x80-0xFF, but really shouldn't be used.
122123
public static boolean isValueCharacter(byte ch) {
123-
return isURICharacter(ch) || ch == ' ' || ch == '\t' || ch == '\n';
124+
int intVal = ch & 0xFF; // Convert the value into an integer without extending the sign bit.
125+
return isURICharacter(ch) || intVal == ' ' || intVal == '\t' || intVal == '\n' || intVal >= 0x80;
124126
}
125127

126128
/**

src/test/java/io/fusionauth/http/CoreTest.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,53 @@ public void hugeHeaders(String scheme) throws Exception {
288288
}
289289
}
290290

291+
292+
@Test()
293+
public void utf8HeaderValues() throws Exception {
294+
String scheme = "http";
295+
String city = "São Paulo";
296+
297+
HTTPHandler handler = (req, res) -> {
298+
res.setHeader(Headers.ContentType, "text/plain");
299+
res.setHeader(Headers.ContentLength, "" + ExpectedResponse.length());
300+
res.setHeader("X-Response-Header", city);
301+
res.setStatus(200);
302+
303+
try {
304+
OutputStream outputStream = res.getOutputStream();
305+
outputStream.write(ExpectedResponse.getBytes());
306+
outputStream.close();
307+
} catch (IOException e) {
308+
throw new RuntimeException(e);
309+
}
310+
};
311+
312+
// Java HTTPClient only supports ASCII header values, so send it directly
313+
try (HTTPServer ignore = makeServer(scheme, handler).start(); Socket sock = new Socket("127.0.0.1", 4242)) {
314+
var os = sock.getOutputStream();
315+
var is = sock.getInputStream();
316+
os.write("""
317+
GET /api/status HTTP/1.1\r
318+
Host: localhost:42\r
319+
X-Foo: São Paulo\r
320+
\r
321+
""".getBytes(StandardCharsets.UTF_8));
322+
os.flush();
323+
324+
var resp = new String(is.readAllBytes(), StandardCharsets.UTF_8);
325+
326+
assertEquals(resp, """
327+
HTTP/1.1 200 \r
328+
content-length: 16\r
329+
content-type: text/plain\r
330+
connection: keep-alive\r
331+
x-response-header: São Paulo\r
332+
\r
333+
{"version":"42"}""");
334+
}
335+
}
336+
337+
291338
@Test
292339
public void keepAliveTimeout() {
293340
AtomicBoolean called = new AtomicBoolean(false);

0 commit comments

Comments
 (0)