Issue #11659 - Properly ignore OWS before field values.

[joakime]

Update HttpParser.CACHE to handle UNMATCHED_VALUE scenarios (like Content-Length: 10) by restoring Jetty 9/10/11 behavior of header only matches from CACHE, allowing HttpParser.parseFields() to handle OWS properly.

Fixes #11659

[gregw]

I think this is the wrong fix. The parser itself is meant to skip OWS, so the convertContentLength method should never see any such characters.

We need to investigate why the state machine in parseFields is not working correctly rather than cover up the problem afterwards

[gregw]

I think this is the wrong fix. The parser itself is meant to skip OWS, so the convertContentLength method should never see any such characters.

We need to investigate why the state machine in parseFields is not working correctly rather than cover up the problem afterwards

The problem looks to be an issue with the handling of a cached header look ahead. investigating....

[gregw]

It was broken by #10308,which changed HttpField so that the value could never be null. This breaks the HttpParser header cache, as it uses null values to indicate a header only cache item.

@sbordet Do you remember why you did that? There is a difference between a null value (i.e. not set) and an empty value.

The fix is either to revert this change OR to update HttpParser to test for blank values rather than null values in it's cache handling... but then that will need careful handling of line termination handling to really handle empty values.

Also it is a little bit of a worry that our tests didn't detect this. I think we need a better tests for OWS that cover: cached header; cached header with no value; cached header with unknown value; cached header and value.

[sbordet]

@gregw HttpField null values were treated inconsistently, meaning that in 95% of the places they were assumed to be non-null and used without a null guard, possibly leading to NPEs.

Since it was the majority of the cases, and HTTP really does not have any meaning for a null value (it can only be the empty string), the change was made.

I think it would be better to change HttpParser as it is more of a private class (HttpField is exposed to applications, and we don't really want users to create an HttpField with a null value).

[gregw]

@joakime perhaps it would be best to create the cache with a special sentinel value for the header only cases so that value not set can be distinguished from a empty string value.

[joakime]

@gregw @sbordet this is updated per your reviews.

• reverted the changes to convertContentLength
• implemented changes to CACHE and parseFields to handle OWS properly for all UNMATCHED_VALUE headers.
• added 3 new OWS test cases (that fail in 12.0.x HEAD btw) to handle this OWS case.