Skip to content

Carefully manage Keep-Alive connections in HttpStaticFileServer#8914

Merged
normanmaurer merged 1 commit intonetty:4.1from
kachayev:fix-file-server-keep-alive
Mar 6, 2019
Merged

Carefully manage Keep-Alive connections in HttpStaticFileServer#8914
normanmaurer merged 1 commit intonetty:4.1from
kachayev:fix-file-server-keep-alive

Conversation

@kachayev
Copy link
Copy Markdown
Contributor

@kachayev kachayev commented Mar 4, 2019

Motivation:

Simple rules:

  • close the connection when sending any error
  • specify "Connection: close" header when closing the connection
  • successful responses should keep the connection intact when otherwise is not requested by the client

Modifications:

  • "send a response and clean up the connection" logic moved to a helper
  • for all successful responses set correct "Content-Lenght" header
  • do not specify "Connection: Keep-Alive" header as far it's a default for HTTP/1.1
  • set "Connection: close" header when necessary

Result:

Keep-Alive connections management conforms with RFCs.

@netty-bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kachayev this is not correct as you already write a FullHttpResonse and so MUST NOT write an extra LastHttpContent

Motivation:

Simple rules:

* close the connection when sending any error
* specify "Connection: close" header when closing the connection
* successful responses should keep the connection intact when otherwise is not requested by the client

Modifications:

* "send response and cleanup the connection" logic moved to a helper
* for all successful responses set "Content-Lenght" header
* do not specify "Connection: Keep-Alive" header as far it's a default for HTTP/1.1
* set "Connection: close" header when necessary

Result:

Keep-Alive connections management is inlined with RFCs.
@kachayev kachayev force-pushed the fix-file-server-keep-alive branch from b199ef6 to ec9cb5e Compare March 5, 2019 09:24
@normanmaurer normanmaurer added this to the 4.1.34.Final milestone Mar 6, 2019
@normanmaurer normanmaurer self-assigned this Mar 6, 2019
@normanmaurer normanmaurer merged commit a651804 into netty:4.1 Mar 6, 2019
normanmaurer pushed a commit that referenced this pull request Mar 6, 2019
Motivation:

Simple rules:

* close the connection when sending any error
* specify "Connection: close" header when closing the connection
* successful responses should keep the connection intact when otherwise is not requested by the client

Modifications:

* "send response and cleanup the connection" logic moved to a helper
* for all successful responses set "Content-Lenght" header
* do not specify "Connection: Keep-Alive" header as far it's a default for HTTP/1.1
* set "Connection: close" header when necessary

Result:

Keep-Alive connections management is inlined with RFCs.
kachayev added a commit to kachayev/netty that referenced this pull request Apr 2, 2019
Motivation:

"Connection: close" header should be specified each time we're going
to close an underlying TCP connection when sending HTTP/1.1 reply.

Modifications:

Introduces changes made in netty#8914 for the following examples:

* WebSocket index page and WebSocket server handler
* HelloWorld server
* SPDY server handler
* HTTP/1.1 server handler from HTTP/2 HelloWorld example
* HTTP/1.1 server handler from tiles example

Result:

Keep-Alive connections management conforms with RFCs.
normanmaurer pushed a commit that referenced this pull request Apr 2, 2019
…8966)

Motivation:

"Connection: close" header should be specified each time we're going
to close an underlying TCP connection when sending HTTP/1.1 reply.

Modifications:

Introduces changes made in #8914 for the following examples:

* WebSocket index page and WebSocket server handler
* HelloWorld server
* SPDY server handler
* HTTP/1.1 server handler from HTTP/2 HelloWorld example
* HTTP/1.1 server handler from tiles example

Result:

Keep-Alive connections management conforms with RFCs.
normanmaurer pushed a commit that referenced this pull request Apr 2, 2019
…8966)

Motivation:

"Connection: close" header should be specified each time we're going
to close an underlying TCP connection when sending HTTP/1.1 reply.

Modifications:

Introduces changes made in #8914 for the following examples:

* WebSocket index page and WebSocket server handler
* HelloWorld server
* SPDY server handler
* HTTP/1.1 server handler from HTTP/2 HelloWorld example
* HTTP/1.1 server handler from tiles example

Result:

Keep-Alive connections management conforms with RFCs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants