Validate http request and response headers when using the webclient #6515
Conversation
klustria
commented
Mar 30, 2023
•
klustria
commented
- The main method for validating header value now exists in HTTP HeaderValue, so it can be reused in other places outside of WebClient, if needed.
- HttpCallChainBase.writeHeaders will validate both the header name and values for the HTTP request using Http.HeaderValue.validate().
- Http1HeadersParser.readHeaders used for parsing response headers now includes validation of header name and values using Http.HeaderValue.validate().
- Extensive unit test for each of the new features added.
oracle-contributor-agreement
bot
added
the
OCA Verified
|
This PR is a potential fix for #6338 |
|
I feel like it is OK, but I would like to know also your opinion @tomas-langer |
nima/webclient/webclient/src/main/java/io/helidon/nima/webclient/http1/ClientRequestImpl.java
Outdated
Show resolved
Hide resolved
klustria
force-pushed
the
6338_validate-request-headers_4.x
branch
from
cb2101a
to
14fb22e
Compare
… the following: 1. The main method for validating header value now exists in HTTP HeaderValue, so it can be reused in other places outside of WebClient, if needed. 2. HttpCallChainBase.writeHeaders will validate both the header name and values for the HTTP request. 3. Http1HeadersParser used for parsing response headers now includes validation of header values stored as LazyString. 4. LazyString now has an option to set a customize validation so its string value can be validated when it is retrieved. For example in Http1HeadersParser, it will be utilized to validate http header value when it is retrieved from the response. 5. Extensive unit test for each of the new features added.
klustria
changed the title
|
Please check the description section of this issue to see the list of items that are included in the change. |
There was a problem hiding this comment.
We should not do the change in LazyString
I think validation should be done once for all headers after they are read from the stream (both on server and on client when enabled).
We could add HeaderValue.validate(Validator) method - and make it work nicely with multi-valued headers in the right way, rather then attempting to target LazyString specifically
…idate header name and value after they are parsed in Http1HeadersParser.readHeaders(). Note: HttpToken.validate() used to validate when headerName is parsed in Http1HeadersParser.readHeaderName() was removed as it is already performed in the above change.
With the new change, removed response header validation from LazyString and placed it in Http1HeaderParser.readHeaders after a header's name and value are parsed and using the same Http.HeaderValue.validate() that HttpCallChainBase.writeHeaders() uses to validate both the header name/values for the request Headers. Also remove name token validation in Http1HeaderParser.readHeaderName() as the name is already validated in Http.HeaderValue.validate(). |
