Add support for non-ASCII header values #26
Conversation
Switch master branch to main
…a String Update value character validator Add test
| public static boolean isValueCharacter(byte ch) { | ||
| return isURICharacter(ch) || ch == ' ' || ch == '\t' || ch == '\n'; | ||
| int intVal = ch & 0xFF; // Convert the value into an integer without extending the sign bit. | ||
| return isURICharacter(ch) || intVal == ' ' || intVal == '\t' || intVal == '\n' || intVal >= 0x80; |
There was a problem hiding this comment.
In addition to header values, this method is used when parsing the response status message. Does the RFC allow for this type of character there as well? Or is there any risk to using this same logic for both of these use cases?
There was a problem hiding this comment.
That part hasn't been superseded by RFC9110, so according to RFC7230, the status reason can still (unfortunately) contain obs-text.
The reason-phrase element exists for the sole purpose of providing a
textual description associated with the numeric status code, mostly
out of deference to earlier Internet application protocols that were
more frequently used with interactive text clients. A client SHOULD
ignore the reason-phrase content.reason-phrase = *( HTAB / SP / VCHAR / obs-text )
| // TODO : Should this be sized with a configuration parameter? | ||
| private final StringBuilder builder = new StringBuilder(); | ||
| // Allocate a 4k buffer for starters, it will grow as needed. | ||
| private final ByteArrayOutputStream byteBuffer = new ByteArrayOutputStream(4096); |
There was a problem hiding this comment.
I suppose this should be ok? We will be allocating at least 4k on each request - this is quite a bit more than the previous StringBuilder - but I suppose that was likely having to re-allocate the underlying array right away.
Perhaps it is very common for the HTTP request preamble to exceed 4k? Any performance concern on this change?
There was a problem hiding this comment.
I presume the previous approach did a lot of re-allocations and copying while it was parsing the prelude. This feels large enough to not need re-allocation in most cases, while not going overboard.
Add support for non-ASCII header values.
Update deps.
Switch from 'master' branch to 'main'
Fixes: