search

ReactiveX / RxNetty

Reactive Extension (Rx) Adaptor for Netty
Apache License 2.0
1.38k stars 254 forks source link

Upgrade Response should not contain "transfer-encoding" #576

Open violetagg opened 7 years ago

violetagg commented 7 years ago

Hi,

I'm experimenting with the example here [1]. When inspecting the request/response headers I see that RxNetty includes "transfer-encoding" to the response's headers when switching the protocols. This header is added here [2].

According https://tools.ietf.org/html/rfc2616#section-4.3 Message Body "All 1xx (informational), 204 (no content), and 304 (not modified) responses MUST NOT include a message-body."

I think that "transfer-encoding" should not be added to the response's headers.

Request Headers:

upgrade: websocket connection: upgrade sec-websocket-version: 13 sec-websocket-key: ... host: localhost:63050

Response Headers:

HTTP/1.1 101 Switching Protocols sec-websocket-accept: ... upgrade: websocket connection: upgrade transfer-encoding: chunked

What do you think?

Best Regards, Violeta Georgieva

[1] https://github.com/ReactiveX/RxNetty/blob/0.5.x/rxnetty-examples/src/main/java/io/reactivex/netty/examples/http/ws/echo/WebSocketEchoServer.java [2] https://github.com/ReactiveX/RxNetty/blob/0.5.x/rxnetty-http/src/main/java/io/reactivex/netty/protocol/http/ws/server/Ws7To13UpgradeHandler.java#L85

NiteshKant commented 7 years ago

I am not sure if presence of transfer-encoding header violates the condition:

"All 1xx (informational), 204 (no content), and 304 (not modified) responses MUST NOT include a message-body."

violetagg commented 7 years ago

Hi,

Yes there is no direct violation of the specification. But even in the Websocket Protocol specification it is described which are the expected headers when opening a connection.

http://www.rfc-base.org/txt/rfc-6455.txt

4.2.2. Sending the Server's Opening Handshake ...

  1. If the server chooses to accept the incoming connection, it MUST reply with a valid HTTP response indicating the following.
    1. A Status-Line with a 101 response code as per RFC 2616 [RFC2616]. Such a response could look like "HTTP/1.1 101 Switching Protocols".
      1. An |Upgrade| header field with value "websocket" as per RFC 2616 [RFC2616].
      2. A |Connection| header field with value "Upgrade".
    2. A |Sec-WebSocket-Accept| header field...
      1. Optionally, a |Sec-WebSocket-Protocol| header field, with a value /subprotocol/ as defined in step 4 in Section 4.2.2.
      2. Optionally, a |Sec-WebSocket-Extensions| header field, with a value /extensions/ as defined in step 4 in Section 4.2.2...

This completes the server's handshake.

Although it is not explicitly forbidden most of the servers like Tomcat, Jetty, Undertow, etc. do not send transfer-encoding and follow the specification quoted above.

Also some of the websocket clients fail the handshake when there is transfer-encoding in the server response.

This header is useless in this scenario and I propose to send a response without it.

Regards, Violeta

Sebmaster commented 4 years ago

Found this in a Google search by chance since I'm just fixing this in actix-web as well.

I am not sure if presence of transfer-encoding header violates the condition:

"All 1xx (informational), 204 (no content), and 304 (not modified) responses MUST NOT include a message-body."

It's actually described in the Content-Length and Transfer-Encoding headers in RFC7230:

A server MUST NOT send a Transfer-Encoding header field in any response with a status code of 1xx (Informational) or 204 (No Content). A server MUST NOT send a Transfer-Encoding header field in any 2xx (Successful) response to a CONNECT request (Section 4.3.6 of [RFC7231]).

Similar language is also in the section after for Content-Length.