timothyjward commented on a change in pull request #812:
URL: https://github.com/apache/cxf/pull/812#discussion_r658685091
##########
File path:
rt/rs/sse/src/main/java/org/apache/cxf/jaxrs/sse/interceptor/SseInterceptor.java
##########
@@ -73,16 +73,6 @@ public void handleMessage(Message message) {
servletResponse = (HttpServletResponse)response;
builder = Response.status(servletResponse.getStatus());
- @SuppressWarnings("unchecked")
Review comment:
> It would be good to know what exactly is the issue with adding headers
to response?
At the point the headers are copied the content of `PROTOCOL_HEADERS` is the
set of HTTP headers from the incoming request. There are numerous issues with
copying these to the response, with several capable of causing potentially
serious bugs. A non-exhaustive exclusion list would contain:
* `Content-Length`- This contains the length in bytes of the incoming
request entity. If set in the response then it is saying that the length of the
response is that number of bytes, which is almost certainly wrong. This can
cause responses to be cut short, or clients to hang waiting for data that never
comes.
* `Content-Type` - This contains the content type of the incoming request
entity. If set in the response then it may override the correct content type of
`text/event-stream` with something like `text/plain` or `application/json`.
This will cause clients to behave incorrectly.
* `Content-Encoding`- This contains the encoding of the incoming request
entity. If set in the response then it may cause clients to incorrectly assume
the response is compressed with `gzip`, or some other encoding. It could also
mask cases where the response *is* compressed but the request wasn't.
* `Transfer-Encoding` - This will contain any encoding performed on the
request by the client, or by intermediate proxies. If used then it may override
the response which should be using a value of `chunked` for an HTTP 1.1 event
stream
* `Upgrade` - The client may request an HTTP upgrade (e.g. to HTTP 2.0, or
to WebSocket), with a fallback to HTTP 1.1 SSE if the upgrade isn't possible on
the server side. Copying the Upgrade request header back to the client makes no
sense according to the upgrade protocol and may cause strange errors.
There are then plenty of headers which only make sense on a request, and
should not be used in a response. The
[`Host`](https://datatracker.ietf.org/doc/html/rfc2616#section-14.23) and
[`User-Agent`](https://datatracker.ietf.org/doc/html/rfc2616#section-14.43)
headers that you reference are examples of this sort of header, others would
include [`Cookie`](https://datatracker.ietf.org/doc/html/rfc6265#section-5.4),
[`Authorization`](https://datatracker.ietf.org/doc/html/rfc7235#section-4.2)
and [`Accept`](https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2).
Adding the request headers to an HTTP response is confusing as they aren't
supposed to be there and don't fit the expected semantics. Depending on the
strictness of the HTTP client the presence of any of these headers may also
cause issues.
> The headers are included into the response for a reason
I suppose my question would be, what is the reason? It doesn't seem to be
following the normal HTTP header rules, but I may be missing something.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
