Commit Graph

9 Commits

Author SHA1 Message Date
Jakob Buchgraber
d5d932a739 Fix GOAWAY logic in Http2Encoder and Http2Decoder.
Motivation:

1) The current implementation doesn't allow for HEADERS, DATA, PING, PRIORITY and SETTINGS
   frames to be sent after GOAWAY.

2) When receiving or sending a GOAWAY frame, all streams with ids greater than the lastStreamId
   of the GOAWAY frame should be closed. That's not happening.

Modifications:

1) Allow sending of HEADERS and DATA frames after GOAWAY for streams with ids < lastStreamId.
2) Always allow sending PING, PRIORITY AND SETTINGS frames.
3) Allow sending multiple GOAWAY frames with decreasing lastStreamIds.
4) After receiving or sending a GOAWAY frame, close all streams with ids > lastStreamId.

Result:

The GOAWAY handling is more correct.
2015-04-07 20:32:28 -07:00
Scott Mitchell
86edc88448 HTTP/2 LifecycleManager and Http2ConnectionHandler interface clarifications
Motiviation:
The interface provided by Http2LifecycleManager is not clear as to how the writeXXX methods should behave.  The implementation of this interface from the Http2ConnectionHandler's perspecitve is unclear what writeXXX means in this context.

Modifications:
- Method names in Http2LifecycleManager and Http2ConnectionHandler should be renamed and comments should clarify the interfaces.

Results:
Http2LifecycleManager is more clear and Http2ConnectionHandler's implementation makes sense w.r.t to return values.
2015-04-06 14:34:20 -07:00
nmittler
6fbca14f8a Cleaning up the initialization of Http2ConnectionHandler
Motivation:

It currently takes a builder for the encoder and decoder, which makes it difficult to decorate them.

Modifications:

Removed the builders from the interfaces entirely. Left the builder for the decoder impl but removed it from the encoder since it's constructor only takes 2 parameters. Also added decorator base classes for the encoder and decoder and made the CompressorHttp2ConnectionEncoder extend the decorator.

Result:

Fixes #3530
2015-03-30 11:23:02 -07:00
Scott Mitchell
0d3a6e0511 HTTP/2 Decoder reduce preface conditional checks
Motivation:
The DefaultHttp2ConnectionDecoder class is calling verifyPrefaceReceived() for almost every frame event at all times.
The Http2ConnectionHandler class is calling readClientPrefaceString() on every decode event.

Modifications:
- DefaultHttp2ConnectionDecoder should not have to continuously call verifyPrefaceReceived() because it transitions boolean state 1 time for each connection.
- Http2ConnectionHandler should not have to continuously call readClientPrefaceString() because it transitions boolean state 1 time for each connection.

Result:
- Less conditional checks for the mainstream usage of the connection.
2015-03-28 18:52:35 -07:00
Scott Mitchell
23f881b382 Comment punctuation cleanup
Motivation:
Commit d857b16d76 introduced some comments that had no punctuation.

Modifications:
Add punctuation.

Result:
Comments have punctuation.
2015-03-26 13:01:37 -07:00
Scott Mitchell
d857b16d76 Http/2 RST_STREAM frame robustness
Motivation:
The Http2ConnectionHandler writeRstStream method allows RST_STREAM frames to be sent when we do not know about the stream and after a RST_STREAM frame has already been sent.  This may lead to sending frames when we should not according to the HTTP/2 spec. There is also the potential to notify the closeListener multiple times if the closeStream method is called multiple times.

Modifications:
- Prevent RST_STREAM from being sent if we don't know about the stream, or if we already sent the RST_STREAM.
- Prevent the closeListener from being notified multiple times.

Result:
More robust writeRstStream logic in boundary conditions.
2015-03-26 11:47:00 -07:00
Jakob Buchgraber
9bad408de5 HTTP2: Close encoder and decoder on channelInactive and initialize clientPrefaceString on handlerAdded.
Motivation:

- The encoder and decoder should be closed right after the handler releases its resources.
- The clientPrefaceString is allocated in the constructor but releases in handlerRemoved.
  If the handler is never added to the pipeline, the clientPrefaceString will never be
  released.

Modifications:

- Call encoder.close() and decoder.close() on channelInactive.
- Release the clientPrefaceString on handlerRemoved.

Result:

- The encoder and decoder get closed right after the handler's resources are freed.
- It's easier to verify that the clientPrefaceString will also get released.
2015-03-19 13:09:34 -07:00
nmittler
c91eaace5e Cleaning up HTTP/2 method names for max_concurrent_streams
Motivation:

The current documentation for Endpoint methods referring to concurrent streams and the SETTINGS_MAX_CONCURRENT_STREAMS setting are a bit confusing.

Modifications:

Renamed a few of the methods and added more clear documentation.

Result:

Fixes #3451
2015-03-16 10:17:39 -07:00
Nitesh Kant
2d24e1f27d Back port HTTP/2 codec from master to 4.1
Motivation:

HTTP/2 codec was implemented in master branch.
Since, master is not yet stable and will be some time before it gets released, backporting it to 4.1, enables people to use the codec with a stable netty version.

Modification:

The code has been copied from master branch as is, with minor modifications to suit the `ChannelHandler` API in 4.x.
Apart from that change, there are two backward incompatible API changes included, namely,

- Added an abstract method:

  `public abstract Map.Entry<CharSequence, CharSequence> forEachEntry(EntryVisitor<CharSequence> visitor)
            throws Exception;`

to `HttpHeaders` and implemented the same in `DefaultHttpHeaders` as a delegate to the internal `TextHeader` instance.

- Added a method:

`FullHttpMessage copy(ByteBuf newContent);`

in `FullHttpMessage` with the implementations copied from relevant places in the master branch.

- Added missing abstract method related to setting/adding short values to `HttpHeaders`

Result:

HTTP/2 codec can be used with netty 4.1
2015-01-23 11:06:11 -05:00