Related: #3157
Motivation:
It should be convenient to have an easy way to classify an
HttpResponseStatus based on the first digit of the HTTP status code, as
defined in the RFC 2616:
- Information 1xx
- Success 2xx
- Redirection 3xx
- Client Error 4xx
- Server Error 5xx
Modification:
- Add HttpStatusClass
- Add HttpResponseStatus.codeClass() that returns the class of the HTTP
status code
- Remove HttpResponseStatus.isInformational()
Result:
It's easier to determine the class of an HTTP status
Motivation:
The HTTP/2 compressor does not release the input buffer when compression is done. This results in buffer leaks.
Modifications:
- Release the buffer in the HTTP/2 compressor
- Update tests to reflect the correct state
Result:
1 less buffer leak.
Motivation:
The interface for HTTP/2 onDataRead states that buffers will be released by the codec. The decompressor and compressor methods are not releasing buffers created during the decompression/compression process.
Modifications:
After onDataRead calls the decompressor and compressor classes will release the data buffer.
Result:
HTTP/2 compressor/decompressors are consistent with onDataRead interface assumptions.
Motivation:
Some of the comments in HTTP/2 Frame Listener interface are misleading.
Modifications:
Clarify comments in Http2FrameListener.
Result:
Http2FrameListener onDataRead comments are clarified.
Motivation:
When DefaultHttp2FrameReader has read a settings frame, the settings
will be passed along the pipeline. This allows a client to hold off
sending data until it has received a settings frame. But for a server it
will always have received a settings frame and the usefulness of this
forwarding of settings is less useful. This also causes a debug message
to be logged on the server side if there is no channel handler to handle
the settings:
[nioEventLoopGroup-1-1] DEBUG io.netty.channel.DefaultChannelPipeline -
Discarded inbound message {INITIAL_WINDOW_SIZE=131072,
MAX_FRAME_SIZE=16384} that reached at the tail of the pipeline. Please
check your pipeline configuration.
Modifications:
Added a builder for the InboundHttp2ToHttpAdapter and
InboundHttp2PriortyAdapter and a new parameter named 'propagateSettings'
to their constructors.
Result:
It is now possible to control whether settings should be passed along
the pipeline or not.
Motivation:
Found performance issues via FindBugs and PMD.
Modifications:
- Removed unnecessary boxing/unboxing operations in DefaultTextHeaders.convertToInt(CharSequence) and DefaultTextHeaders.convertToLong(CharSequence). A boxed primitive is created from a string, just to extract the unboxed primitive value.
- Added a static modifier for DefaultHttp2Connection.ParentChangedEvent class. This class is an inner class, but does not use its embedded reference to the object which created it. This reference makes the instances of the class larger, and may keep the reference to the creator object alive longer than necessary.
- Added a static compiled Pattern to avoid compile it each time it is used when we need to replace some part of authority.
- Improved using of StringBuilders.
Result:
Performance improvements.
Motivation:
The HTTP/2 codec currently does not expose the boundaries of the initial settings. This could be useful for applications to define their own initial settings.
Modifications:
Add new static final variables to Http2CodecUtil and make Http2Settings use these in the bounds checks.
Result:
Applications can use the max (or min) variables to initialize their settings.
Motivation:
When the inbound flow controller recognizes that the flow control window
has been violated on a stream (not connection-wide), it throws a
connection error.
Modifications:
Changed the DefaultHttp2InboundFlowController to properly throw
connection error if the connection window is violated and stream error
if a stream window is violated.
Result:
inbound flow control throws the correct error for window violations.
Motivation:
The current name of the class which converts from HTTP objects to HTTP/2 frames contains the text Http2ToHttp. This is misleading and opposite of what is being done.
Modifications:
Rename this class name to be HttpToHttp2.
Result:
Class names that more clearly identify what they do.
Motivation:
The current decompression frame listener currently opts-out of application level flow control. The application should still be able to control flow control even if decompression is in use.
Modifications:
- DecompressorFrameListener will maintain how many compressed bytes, decompressed bytes, and processed by the listener bytes. A ratio will be used to translate these values into application level flow control amount.
Result:
HTTP/2 decompressor delegates the application level flow control to the listener processing the decompressed data.
Motivation:
Currently when an exception occurs during a listener.onDataRead
callback, we return all bytes as processed. However, the listener may
choose to return bytes via the InboundFlowState object rather than
returning the integer. If the listener returns a few bytes and then
throws, we will attempt to return too many bytes.
Modifications:
Added InboundFlowState.unProcessedBytes() to indicate how many
unprocessed bytes are outstanding.
Updated DefaultHttp2ConnectionDecoder to compare the unprocessed bytes
before and after the listener.onDataRead callback when an exception was
encountered. If there is a difference, it is subtracted off the total
processed bytes to be returned to the flow controller.
Result:
HTTP/2 data frame delivery properly accounts for processed bytes through
an exception.
Motivation:
The current priority algorithm uses 2 different mechanisms to iterate the priority tree and send the results of the allocation. The current algorithm also uses a two step phase where the priority tree is traversed and allocation amounts are calculated and then all active streams are traversed to send for any streams that may or may not have been allocated bytes.
Modifications:
- DefaultHttp2OutboundFlowController will allocate and send (when possible) in the same looping structure.
- The recursive method will send only for the children instead of itself and its children which should simplify the recursion.
Result:
Hopefully simplified recursive algorithm where the tree iteration determines who needs to send and less iteration after the recursive calls complete.
Currently the DefaultHttp2InboundFlowController only supports the
ability to turn on and off "window maintenance" for a stream. This is
insufficient for true application-level flow control that may only want
to return a few bytes to flow control at a time.
Modifications:
Removing "window maintenance" interface from
DefaultHttp2InboundFlowController in favor of the new interface.
Created the Http2InboundFlowState interface which extends Http2FlowState
to add the ability to return bytes for a specific stream.
Changed the onDataRead method to return an integer number of bytes that
will be immediately returned to flow control, to support use cases that
want to opt-out of application-level inbound flow control.
Updated DefaultHttp2InboundFlowController to use 2 windows per stream.
The first, "window", is the actual flow control window that is
decremented as soon as data is received. The second "processedWindow"
is a delayed view of "window" that is only decremented after the
application returns the processed bytes. It is processedWindow that is
used when determining when to send a WINDOW_UPDATE to restore part of
the inbound flow control window for the stream/connection.
Result:
The HTTP/2 inbound flow control interfaces support application-level
flow control.
Motivation:
Too many warnings from IntelliJ IDEA code inspector, PMD and FindBugs.
Modifications:
- Removed unnecessary casts, braces, modifiers, imports, throws on methods, etc.
- Added static modifiers where it is possible.
- Fixed incorrect links in javadoc.
Result:
Better code.
Motivation:
The outbound flow controller logic does not properly reset the allocated
bytes between successive invocations of the priority algorithm.
Modifications:
Updated the priority algorithm to reset the allocated bytes for each
stream.
Result:
Each call to the priority algorithm now starts with zero allocated bytes
for each stream.
Motivation:
I came across an issue when I was adding/setting headers and mistakenly
used an upper case header name. When using the http2 example that ships
with Netty this was not an issue. But when working with a browser that
supports http2, in my case I was using Firefox Nightly, I'm guessing
that it interprets the response as invalid in accordance with the
specifiction
https://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-8.1.2
"However, header field names MUST be converted to lowercase prior
to their encoding in HTTP/2. A request or response containing
uppercase header field names MUST be treated as malformed"
This PR suggests converting to lowercase to be the default.
Modifications:
Added a no-args constructor that defaults to forcing the key/name to
lowercase, and providing a second constructor to override this behaviour
if desired.
Result:
It is now possible to specify a header like this:
Http2Headers headers = new DefaultHttp2Headers(true)
.status(new AsciiString("200"))
.set(new AsciiString("Testing-Uppercase"), new AsciiString("some value"));
And the header written to the client will then become:
testing-uppercase:"some value"
Motivation:
The HTTP/2 codec currently does not provide an interface to compress data. There is an analogous case to this in the HTTP codec and it is expected to be used commonly enough that it will be beneficial to have the feature in the http2-codec.
Modifications:
- Add a class which extends DefaultHttp2ConnectionEncoder and provides hooks to an EmbeddedChannel
- Add a compressor element to the Http2Stream interface
- Update unit tests to utilize the new feature
Result:
HTTP/2 codec supports data compression.
Motivation:
The current logic in DefaultHttp2OutboundFlowController for handling the
case of a stream shutdown results in a Http2Exception (not a
Http2StreamException). This results in a GO_AWAY being sent for what
really could just be a stream-specific error.
Modifications:
Modified DefaultHttp2OutboundFlowController to set a stream exception
rather than a connection-wide exception. Also using the error code of
INTERNAL_ERROR rather than STREAM_CLOSED, since it's more appropriate
for this case.
Result:
Should not be triggering GO_AWAY when a stream closes prematurely.
Motivation:
Currently due to flow control, HEADERS frames can be written
out-of-order WRT DATA frames.
Modifications:
When data is written, we preserve the future as the lastWriteFuture in
the outbound flow controller. The encoder then uses the lastWriteFuture
such that headers are only written after the lastWriteFuture completes.
Result:
HEADERS/DATA write order is correctly preserved.
Motivation:
The HTTP/2 specification indicates that when converting from HTTP/2 to HTTP/1.x and non-ascii characters are detected that an error should be thrown.
Modifications:
- The ASCII validation is already done but the exception that is raised is not properly converted to a RST_STREAM error.
Result:
- If HTTP/2 to HTTP/1.x translation layer is in use and a non-ascii header is received then a RST_STREAM frame should be sent in response.
Motivation:
The DefaultOutboundFlowController was attempting to write frames with a negative length. This resulted in attempting to allocate a buffer of negative size and thus an exception.
Modifications:
- Don't allow DefaultOutboundFlowController to write negative length buffers.
Result:
No more negative length writes which resulted in IllegalArgumentExceptions.
Motivation:
x-gzip and x-deflate are not standard header values, and thus should be
removed from HttpHeaderValues, which is meant to provide the standard
values only.
Modifications:
- Remove X_DEFLATE and X_GZIP from HttpHeaderValues
- Move X_DEFLATE and X_GZIP to HttpContentDecompressor and
DelegatingDecompressorFrameListener
- We have slight code duplication here, but it does less harm than
having non-standard constant.
Result:
HttpHeaderValues contains only standard header values.
Related: 4ce994dd4f
Motivation:
In 4.1, we were not able to change the type of the HTTP header name and
value constants from String to AsciiString due to backward compatibility
reasons.
Instead of breaking backward compatibility in 4.1, we introduced new
types called HttpHeaderNames and HttpHeaderValues which provides the
AsciiString version of the constants, and then deprecated
HttpHeaders.Names/Values.
We should make the same changes while deleting the deprecated classes
activaly.
Modifications:
- Remove HttpHeaders.Names/Values and RtspHeaders
- Add HttpHeaderNames/Values and RtspHeaderNames/Values
- Make HttpHeaderValues.WEBSOCKET lowercased because it's actually
lowercased in all WebSocket versions but the oldest one
- Do not use AsciiString.equalsIgnoreCase(CharSeq, CharSeq) if one of
the parameters are AsciiString
- Avoid using AsciiString.toString() repetitively
- Change the parameter type of some methods from String to
CharSequence
Result:
A user who upgraded from 4.0 to 4.1 first and removed the references to
the deprecated classes and methods can easily upgrade from 4.1 to 5.0.
Motivation:
If the http2 encoder has exhausted all available stream IDs a GOAWAY frame is not sent. Once the encoder detects the a new stream ID has rolled over past the last stream ID a GOAWAY should be sent as recommended in section [5.1.1](https://tools.ietf.org/html/draft-ietf-httpbis-http2-14#section-5.1.1).
Modifications:
-This condition is already detected but it just needs to result in a GOAWAY being sent.
-Add a subclass of Http2Exception so the encoder can detect this special case.
-Add a unit test which checks that the GOAWAY is sent/received.
Result:
Encoder attempting to use the first 'rolled over' stream id results in a GOAWAY being sent.
Motivation:
- There are still various inspector warnings to fix.
- ValueConverter.convert() methods need to end with the type name like
other methods in Headers, such as setInt() and addInt(), for more
consistency
Modifications:
- Fix all inspector warnings
- Rename ValueConverter.convert() to convert<type>()
Result:
- Cleaner code
- Consistency
Motivation:
Headers within netty do not cleanly share a common class hierarchy. As a result some header types support some operations
and don't support others. The consolidation of the class hierarchy will allow for maintenance and scalability for new codec.
The existing hierarchy also has a few short comings such as it is not clear when data conversions are happening. This
could result unintentionally getting back a collection or iterator where a conversion on each entry must happen.
The current headers algorithm also prepends all elements which means to find the first element or return a collection
in insertion order often requires a complete traversal followed by a collections.reverse call.
Modifications:
-Provide a generic base class which provides all the implementation for headers in netty
-Provide an extension to this class which allows for name type conversions to happen (to accommodate legacy CharSequence to String conversions)
-Update the headers interface to clarify when conversions will happen.
-Update the headers data structure so that appends are done to avoid unnecessary iteration or collection reversal.
Result:
-More unified class hierarchy for headers in netty
-Improved headers data structure and algorithms
-headers API more clearly identify when conversions are required.
Motivation:
There should be a unit test for when the stream ID wraps around and is 'too large' or negative.
The lack of unit test masked an issue where this was not being throw.
Modifications:
Add a unit test to cover the case where creating a remote and local stream where stream id is 'too large'
Result:
Unit test scope increases.
Motivation:
Currently when receiving DATA/HEADERS frames, we throw Http2Exception (a
connection error) instead of Http2StreamException (stream error). This
is incorrect according to the HTTP/2 spec.
Modifications:
Updated various places in the encoder and decoder that were out of spec
WRT connection/state checking.
Result:
Stream state verification is properly handled.
Motivation:
Twitter hpack has upgraded to 0.9.1, we should upgrade to the latest.
Modifications:
Updated the parent pom to specify the dependency version. Updated the
http2 pom to use the version specified by the parent.
Result:
HTTP/2 updated to the latest hpack release.
Motiviation:
The HTTP/2 server example is not using the outbound flow control. It is instead using a FrameWriter directly.
This can lead to flow control errors and other comm. related errors
Modifications:
-Force server example to use outbound flow controller
Result:
-Server example should use follow flow control rules.
Motivation:
InboundHttp2ToHttpAdapterTest swaps non-volatile CountDownLatches in
handlers, which seems to cause a race condition that can lead to missing
messages.
Modifications:
Make CountDownLatch variables in InboundHttp2ToHttpAdapterTest volatile.
Result:
InboundHttp2ToHttpAdapterTest should be more stable.
Motivation:
The current GOAWAY methods are in each endpoint and are a little
confusing since their called connection.<endpoint>.goAwayReceived().
Modifications:
Moving GOAWAY methods to connection with more clear names
goAwaySent/goAwayReceived.
Result:
The GOAWAY method names are more clear.
Motivation:
There is a NPE due to the order of builder initialization in the class.
Modifications:
-Correct the ordering of initialization and building to avoid NPE.
Result:
No more NPE in construction.
Motivation:
This was lost in recent changes, just adding it back in.
Modifications:
Added listener() accessor to Http2ConnectionDecoder and the default
impl.
Result:
The Http2FrameListener can be obtained from the decoder.
Motivation:
Currently, Http2LifecycleManager implements the exception handling logic
which makes it difficult to extend or modify the exception handling
behavior. Simply overriding exceptionCaught() will only affect one of
the many possible exception paths. We need to reorganize the exception
handling code to centralize the exception handling logic into a single
place that can easily be extended by subclasses of
Http2ConnectionHandler.
Modifications:
Made Http2LifecycleManager an interface, implemented directly by
Http2ConnectionHandler. This adds a circular dependency between the
handler and the encoder/decoder, so I added builders for them that allow
the constructor of Http2ConnectionHandler to set itself as the lifecycle
manager and build them.
Changed Http2LifecycleManager.onHttpException to just
onException(Throwable) to simplify the interface. This method is now the
central control point for all exceptions. Subclasses now only need to
override onException() to intercept any exception encountered by the
handler.
Result:
HTTP/2 has more extensible exception handling, that is less likely to
see exceptions vanish into the ether.
Motivation:
Some tests occasionally appear unstable, throwing a
org.mockito.exceptions.misusing.UnfinishedStubbingException. Mockito
stubbing does not work properly in multi-threaded environments, so any
stubbing has to be done before the threads are started.
Modifications:
Modified tests to perform any custom stubbing before the client/server
bootstrap logic executes.
Result:
HTTP/2 tests should be more stable.
Motivation:
Some tests do not properly assert that all requests have been
sent/received, so the failures messages may be misleading.
Modifications:
Adding missing asserts to HTTP/2 tests for awaiting requests and
responses.
Result:
HTTP/2 tests properly assert message counts.
Motiviation:
PR https://github.com/netty/netty/pull/2948 missed a collection to synchronize in the HTTP/2 unit tests.
Modifications:
synchronize the collection that was missed
Result:
Missed collection is syncronized and initial size is corrected
The HTTP/2 tests have been unstable, in particular the
Http2ConnectionRoundtripTest.
Modifications:
Modified fields in Http2TestUtil to be volatile.
Result:
Tests should (hopefully) be more stable.
Motivation:
HTTP/2 codec does not properly test exception passed to
exceptionCaught() for instanceof Http2Exception (since the exception
will always be wrapped in a PipelineException), so it will never
properly handle Http2Exceptions in the pipeline.
Also if any streams are present, the connection close logic will execute
twice when a pipeline exception. This is because the exception logic
calls ctx.close() which then triggers the handleInActive() logic to
execute. This clears all of the remaining streams and then attempts to
run the closeListener logic (which has already been run).
Modifications:
Changed exceptionCaught logic to properly extract Http2Exception from
the PipelineException. Also added logic to the closeListener so that is
only run once.
Changed Http2CodecUtil.toHttp2Exception() to avoid NPE when creating
an exception with cause.getMessage().
Refactored Http2ConnectionHandler to more cleanly separate inbound and
outbound flows (Http2ConnectionDecoder/Http2ConnectionEncoder).
Added a test for verifying that a pipeline exception closes the
connection.
Result:
Exception handling logic is tidied up.
Motivation:
The HTTP/2 unit tests are collecting responses read events which are happening in a multithreaded environment.
These collections are currently not synchronized or thread safe and are resulting in verification failures.
Modifications:
-Modify unit tests that use collections to store results for verifiction to be thread safe
Result:
Tests should not fail because of syncrhonization issues while verifying expected results.
Motivation:
The HTTP/2 codec has some duplication and the read/write interfaces are not cleanly exposed to users of the codec.
Modifications:
-Restructure the AbstractHttp2ConnectionHandler class to be able to extend write behavior before the outbound flow control gets the data
-Add Http2InboundConnectionHandler and Http2OutboundConnectionHandler interfaces and restructure external codec interface around these concepts
Result:
HTTP/2 codec provides a cleaner external interface which is easy to extend for read/write events.
Motivation:
The HTTP tranlsation layer uses a FullHttpMessage object after it has been fired up the pipeline.
Although the content ByteBuf is not used by default it is still not ideal to use a releasable object
after it has potentially been released.
Modifications:
-InboundHttp2ToHttpAdapter ordering issues will be corrected
Result:
Safer access to releasable objects in the HTTP/2 to HTTP translation layer.
Motivation:
To eliminate the tests as being a cause of leaks, removing the automatic
retaining of ByteBufs in Http2TestUtil.
Modifications:
Each test that relied on retaining buffers for validation has been
modified to copy the buffer into a list of Strings that are manually
validated after the message is received.
Result:
The HTTP/2 tests should (hopefully) no longer be reporting erroneous
leaks due to the testing code, itself.
Motivation:
The current implementation of the HTTP/2 decompression does not integrate with flow control properly.
The decompression code is giving the post-decompression size to the flow control algorithm which
results in flow control errors at incorrect times.
Modifications:
-DecompressorHttp2FrameReader.java will need to change where it hooks into the HTTP/2 codec
-Enhance unit tests to test this condition
Result:
No more flow control errors because of decompression design flaw
Motivation:
The current build is showing potential leaks in the HTTP/2 tests that
use Http2TestUtil.FrameCountDown, which copies the buffers when it
receives them from the decoder. The leak detecor sees this copy as the
source of a leak. It would be better all around to just retain, rather
than copying the buffer. This should help to lower the overall memory
footprint of the tests as well as potentially getting rid of the
reported "leaks".
Modifications:
Modified Http2TestUtil to use ByteBuf.retain() everywhere that was
previously calling ByteBuf.copy().
Result:
Smaller memory footprint for tests and hopefully getting rid of reported
leaks.
Motivation:
The HTTP/2 spec does not restrict headers to being String. The current
implementation of the HTTP/2 codec uses Strings as header keys and
values. We should change this so that header keys and values allow
binary values.
Modifications:
Making Http2Headers based on AsciiString, which is a wrapper around a
byte[].
Various changes throughout the HTTP/2 codec to use the new interface.
Result:
HTTP/2 codec no longer requires string headers.
Motivation:
The HTTP/2 unit tests are suffering from OOME on the master branch.
These unit tests allocating a large number of threads (~706 peak live) which may
be related to this memory pressure.
Modifications:
Each EventLoopGroup shutdown operation will have a `sync()` call.
Result:
Lower peek live thread count and less associated memory pressure.
Motivation:
The HTTP/2 tests do not always clean up ByteBuf resources reliably. There are issues with the refCnt, over allocating buffers, and potentially not waiting long enough to reclaim resources for stress tests.
Modifications:
Scrub the HTTP/2 unit tests for ByteBuf leaks.
Result:
Less leaks (hopefully none) in the HTTP/2 unit tests. No OOME from HTTP/2 unit tests.
Motivation:
The HTTP/2 codec does not provide a way to decompress data. This functionality is supported by the HTTP codec and is expected to be a commonly used feature.
Modifications:
-The Http2FrameReader will be modified to allow hooks for decompression
-New classes which detect the decompression from HTTP/2 header frames and uses that decompression when HTTP/2 data frames come in
-New unit tests
Result:
The HTTP/2 codec will provide a means to support data decompression
Motivation:
The ServerBootrap's child group would not be shutdown.
Modification:
Add missing shutdownGracefully() call.
Result:
The child group is shutdown correctly.
Motivation:
The HTTP/2 specification places restrictions on the cipher suites that can be used. There is no central place to pull the ciphers that are allowed by the specification, supported by different java versions, and recommended by the community.
Modifications:
-HTTP/2 will have a security utility class to define supported ciphers
-netty-handler will be modified to support filtering the supplied list of ciphers to the supported ciphers for the current SSLEngine
Result:
-Netty provides unified support for HTTP/2 cipher lists and ciphers can be pruned by currently supported ciphers
Motivation:
Outbound flow control does not properly remove the head of queue after
it's written. This will cause streams with multiple frames to get stuck
and not send all of the data.
Modifications:
Modified the DefaultHttp2OutboundFlowController to properly remove the
head of the pending write queue once a queued frame has been written.
Added an integration test that sends a large message to verify that all
DATA frames are properly collected at the other end.
Result:
Outbound flow control properly handles several queued messages.
Motivation:
We failed to release buffers on protocolErrors which caused buffer leaks when using HTTP/2
Modifications:
Release buffer on protocol errors
Result:
No more buffer leaks
Motivation:
Netty only supports a java NPN implementation provided by npn-api and npn-boot.
There is no java implementation for ALPN.
ALPN is needed to be compliant with the HTTP/2 spec.
Modifications:
-SslContext and JdkSslContext to support ALPN
-JettyNpn* class restructure for NPN and ALPN common aspects
-Pull in alpn-api and alpn-boot optional dependencies for ALPN java implementation
Result:
-Netty provides access to a java implementation of APLN
Motivation:
A recent refactoring of the outbound flow controller interface
introduced a bug when writing data. We're no longer properly handling
the completion of the write (i.e. updating stream state/handling error).
Modifications:
Updated AbstractHttp2ConnectionHandler.writeData to properly handle the
completion of the write future.
Result:
DATA writes now perform post-write cleanup.
Motivation:
We currently have a mix of "Observer" and "Listener" for interface
names. We should make them all "Listener" to be consistent.
Modifications:
Renamed Http2DataObserver->Http2DataListener and
Http2FrameObserver->Http2FrameListener.
Result:
Listener interface names are consistent.
Motivation:
The priority information reported by the HTTP/2 to HTTP tranlsation layer is not correct in all situations.
The HTTP translation layer is not using the Http2Connection.Listener interface to track tree restructures.
This incorrect information is being sent up to clients and is misleading.
Modifications:
-Restructure InboundHttp2ToHttpAdapter to allow a default data/header mode
-Extend this interface to provide an optional priority translation layer
Result:
-Priority information being correctly reported in HTTP/2 to HTTP translation layer
-Cleaner code with seperation of concerns (optional priority conversion).
Motivation:
Currently, window maintenance is automatically performed when a flow
control window drops below half its initial size. We should provide a
way for advanced applications to determine whether or not this should be
done on a per-stream basis.
Modifications:
Modifying DefaultHttp2InboundFlowController to allow enabling/disabling
of window maintenance per stream.
Result:
Inbound flow control window maintenance will be dynamically
configurable.
Motivation:
HTTP/2 draft 14 came out a couple of weeks ago and we need to keep up
with the spec.
Modifications:
-Revert back to dispatching FullHttpMessage objects instead of individual HttpObjects
-Corrections to HttpObject comparitors to support test cases
-New test cases to support sending headers immediatley
-Bug fixes cleaned up to ensure the message flow is terminated properly
Result:
Netty HTTP/2 to HTTP/1.x translation layer will support the HTTP/2 draft message flow.
Motivation:
This is just some general cleanup to get rid of the FrameWriter inner
interface withing Http2InboundFlowController. It's not necessary since
the flow controller can just use the Http2FrameWriter to send
WINDOW_UPDATE frames.
Modifications:
Updated DefaultHttp2InboundFlowController to use Http2FrameWriter.
Result:
The inbound flow control code is somewhat less smelly :).
Motivation:
We're currently out-of-spec with HTTP/2 in that we don't include padding
in the flow control logic.
Modifications:
Modified both DefaultHttp2InboundFlowController and
DefaultHttp2OutboundFlowController to properly take padding into
account. Outbound is more complicated since padding has to be properly
accounted for when splitting frames.
Result:
HTTP/2 codec properly flow controls padding.
Motivation:
The current interface and implementation for HTTP/2 priority tree events does not notify listeners of all parent change events. As a result operations which depend upon knowing about parent change events may be missing events and result in stale data. This interface also allows new listeners to easily consume priority tree change events.
Modifications:
-Http2Connection.Listener interface will change to support notifications on every node after the priority has changed and tree events have settled
-This will affect the outbound flow controller, DefaultHttp2Connection, and other listeners using the old interface
Result:
A modified (hopefully simplified and correct) Listener interface to get priority tree change event notification
Motivation:
This is addressing a TODO in the outbound flow controller. We currently
have a separate writer interface passed into the outbound flow
controller. This is confusing and limiting as to how the flow controller
can perform its writes (e.g. no control over flushing). Instead it would
be better to just let the flow controller use the Http2FrameWriter
directly.
Modifications:
- Added a new Http2DataWriter interface, which is extended by
Http2FrameWriter and Http2OutboundFlowController.
- Removed automatic flushing from Http2DataWriter in order to facilitate
optimizing the case where there are multiple writes.
- Updated DefaultHttp2OutboundFlowController to properly optimize
flushing of the ChannelHandlerContext when multiple writes occur.
Result:
Code is greatly simplified WRT outbound flow control and flushes are
optimized for flow-controlled DATA frames.
Motivation:
HTTP/2 draft 14 came out a couple of weeks ago and we need to keep up
with the spec.
Modifications:
- Removed use of segment throughout.
- Added new setting for MAX_FRAME_SIZE. Used by the frame reader/writer
rather than a constant.
- Added new setting for MAX_HEADER_LIST_SIZE. This is currently unused.
- Expanded the header size to 9 bytes. The frame length field is now 3
bytes and added logic for checking that it falls within the valid range.
Result:
Netty will support HTTP/2 draft 14 framing. There will still be some
work to do to be compliant with the HTTP adaptation layer.
Motivation:
The HTTP/2 codec currently provides direct callbacks to access stream events/data. The HTTP/2 codec provides the protocol support for HTTP/2 but it does not pass messages up the context pipeline. It would be nice to have a decoder which could collect the data framed by HTTP/2 and translate this into traditional HTTP type objects. This would allow the traditional Netty context pipeline to be used to separate processing concerns (i.e. HttpContentDecompressor). It would also be good to have a layer which can translate FullHttp[Request|Response] objects into HTTP/2 frame outbound events.
Modifications:
Introduce a new InboundHttp2ToHttpAdapter and supporting classes which will translate HTTP/2 stream events/data into HttpObject objects. Introduce a new DelegatingHttp2HttpConnectionHandler which will translate FullHttp[Request|Response] objects to HTTP/2 frame events.
Result:
Introduced HTTP/2 frame events to HttpObject layer.
Introduced FullHttp[Request|Response] to HTTP/2 frame events.
Introduced new unit tests to support new code.
Updated HTTP/2 client example to use new code.
Miscelaneous updates and bug fixes made to support new code.
Motivation:
There are still a few places in the HTTP/2 code that have the compressed
flag (from pre-draft 13). Need to remove this flag since it's no longer
used.
Modifications:
Various changes to remove the flag from the writing path.
Result:
No references to the compressed flag.
Motivation:
The current HTTP/2 ping handling replies with an ack using the same
buffer as the received ping. It doesn't call retain(), however, which
causes a ReferenceCountException since the buffer ends up getting
released twice (once by the write and once by the decoder).
Modifications:
Modified AbstractHttp2ConnectionHandler to retain() the buffer. Added a
ping to Http2ConnectionRoundtripTest.stressTest() to verify the problem
and that this fixes it.
Result:
Ping should no longer cause an exception.
Motivation:
Need to upgrade HTTP/2 implementation to latest draft.
Modifications:
Various changes to support draft 13.
Result:
Support for HTTP/2 draft 13.
Motivation:
All of the functionality has been merged into
AbstractHttp2ConnectionHandler.
Modifications:
Removed the Http2PrefaceHandler/Test and all uses.
Result:
Code cleaned up a bit.
Related pull request: #2481 written by @nmittler
Motivation:
Some tests were still sending requests from the test thread rather than
from the event loop.
Modifications:
- Modified the roundtrip tests to use a new utility Http2TestUtil to
perform the write operations in the event loop thread.
- Modified the Http2Client under examples to write all requests in the
event loop thread.
- Added HttpPrefaceHandler and its test which were missing.
- Fixed various inspector warnings
Result:
Tests and examples now send requests in the correct thread context.
Motivation:
An IDE's auto-completion often confuses between java.util.Collections
and io.netty.util.collection.Collections, and it's annoying to me. :-)
Modifications:
Use a different class name.
Result:
When I type Collections, it's always 'the' Collections.
Motivation:
Various small fixes/improvements to the interface to the HTTP/2 classes,
as well as some minor performance improvements.
Modifications:
- Added fix for IntObjectHashMap to ensure that capacity is always odd.
Even capacity can cause probing to fail.
- Cleaned the access to GOAWAY information in Http2Connection interface.
Endpoints now manage their own state for GOAWAY. Also added a goingAway
event handler.
- Added Endpoint methods for checking MAX_CONCURRENT_STREAMS or if the
number of streams for the endpoint have been exhausted. See
Endpoint.nextStreamId()/acceptingNewStreams().
- Changed DefaultHttp2Connection to use IntObjectHashMap. This should be
a slight memory improvement.
- Fixed check for MAX_CONCURRENT_STREAMS to correctly use the number of
active streams for the endpoint (not total active). See
DefaultHttp2Connection.checkNewStreamAllowed.
- Exposing a few methods to subclasses of AbstractHttp2ConnectionHandler
(e.g. exception handling).
- Cleaning up GOAWAY and RST_STREAM handling in
AbstractHttp2ConnectionHandler.
Result:
HTTP/2 code should provide more information to subclasses and will have
a reduced memory footprint.
Motivation:
Subclasses of AbstractHttp2ConnectionHandler have to implement all frame
handler methods, many of which can be ignored in many cases. Also there
is no easy way to access the connection object.
Modifications:
Added default implementations for frame handler methods to
AbstractHttp2ConnectionHandler, and added an accessor for the
connection.
Also fixed example test for HTTP/2 with cleartext upgrade. It must have
been broken by recent commits.
Result:
AbstractHttp2ConnectionHandler is more subclass-friendly.
Motivation:
The connection, priority tree, and inbound/outbound flow controllers
each maintain a separate map for stream information. This is wasteful
and complicates the design since as streams are added/removed, multiple
structures have to be updated.
Modifications:
- Merging the priority tree into Http2Connection. Then we can use
Http2Connection as the central stream repository.
- Adding observer pattern to Http2Connection so flow controllers can be
told when a new stream is created, closed, etc.
- Adding properties for inboundFlow/outboundFlow state to Http2Stream.
This allows the controller to access flow control state directly from
the stream without requiring additional structures.
- Separate out the StreamRemovalPolicy and created a "default"
implementation that runs periodic garbage collection. This used to be
internal to the outbound flow controller, but I think it is more general
than that.
Result:
HTTP/2 classes will require less storage for new streams.
Motivation:
At the moment we have some methods that return a ChannelFuture but still throw a Http2Exception too. This is confusing in terms of semantic. A method which returns a ChannelFuture should not throw an Http2Exception but just fail the ChannelFuture.
Modifications:
* Make sure we fail the returned ChannelFuture in cases of Http2Exception and remove the throws Http2Exception from the method signature.
* Also some cleanup
Result:
Make the API usage more clear.
Motivation:
The HTTP/2 connection preface logic is currently handled in two places.
Reading/writing the client preface string is handled by
Http2PrefaceHandler while the reading/writing of the initial settings
frame is handled by AbstractHttp2ConnectionHandler. Given that their
isn't much code in Http2PrefaceHandler, it makes sense to just merge it
into the preface handling logic of AbstractHttp2ConnectionHandler. This
will also make configuring the pipeline simpler for HTTP/2.
Modifications:
Removed Http2PrefaceHandler and added it's logic to
AbstractHttp2ConnectionHandler. Updated other classes depending on
Http2PrefaceHandler.
Result:
All of the HTTP/2 connection preface processing logic is now in one
place.
Motivation:
Previously local settings is applied on its transmission. But this
makes settings value out-of-sync with remote endpoint. The
application of local settings must be done after the remote endpoint
sends back its SETTINGS ACK. This synchronization is vital to the
protocol. Suppose that server sends SETTINGS_HEADER_TABLE_SIZE 0 on
connection establishment and resizes its header table size down to 0.
Meanwhile, client sends HEADERS which has header block encoded in
default 4096 header table size, because client has not seen the
SETTINGS from server. Server tries to decode received HEADERS but due
to the difference of header table size, decoding process may fail.
Modifications:
We don't apply settings on transmission. Instead, we keep track of
outstanding settings in FIFO queue. When we get ACK, we pop oldest
outstanding settings and apply its values.
Result:
The outstanding settings values are applied when its ACK is received,
which is compliant behavior to the HTTP/2 specification.
Motivation:
A few items were identified where the http2 codec is out of compliance
with the spec.
Modifications:
- Fixed handling of priority weight on the wire. Now adding 1 after
reading from the wire and subtracing 1 before writing.
- Fixed handling of next stream ID. Client streamIds were starting at 3,
but they need to start at 1 to allow the upgrade from HTTP/1.1. Also
making next stream ID logic more flexible. Allowing the next created
stream to be any number in the sequence following the previously created
stream.
- Disallowing SETTINGS frames with ENABLE_PUSH specified for server
endpoints. This means that attempts to write this frame from a server,
or read it from a client will fail.
Result:
The http2 implementation will be more inline with the spec.
Motivation:
The Http2FrameObserver isn't provided the ChannelHandlerContext when
it's called back. This will force observers to find an alternative means
of obtaining the context if they need to do things like copying buffers.
Modifications:
Changed the Http2FrameReader and Http2FrameObserver to include the
context. Updated all other uses of these interfaces.
Result:
Frame observers will now have the channel context.
Motivation:
The outbound flow controller currently has to walk the entire tree each
time to calculate the total available data for each subtree. For better
performance we should maintain a running total for each subtree as we
queue/write frames.
Modifications:
I've modified the DefaultHttp2OutboundFlowController to manage the state
of "priorityData" at each node in the priority tree, which is
essentially the total writable data for all streams in that subtree.
These totals do not take into account the connection window, as that is
applied when splitting the data across the streams at each level in the
tree.
The flow controller now sorts the children of a node by the product of
their data (for the subtree) and its weight. This is used since in
certain cases the algorithm might prefer nodes that appear later in the
list. Sorting helps keep nodes with similar characteristics (e.g. a lot
of data and a high priority) with similar output.
To help clean things up, I'm storing a FlowState for the root node as
well, which maintains the runnning total of the currently writable data
for all stream.
Another item of cleanup is that I created a GarbageCollector innerclass
within the outbound flow controller. This keeps all of the garbage
collection code in one place, away from the flow control code.
Also added some more unit tests for the flow controller.
Result:
The outbound flow controller is a bit cleaner and perhaps a bit more
fair when distributing outbound data across streams.
Motivation:
Our ci showed some buffer leaks in the http2 codec. This commit fixes all of these.
Modifications:
* Correctly release buffers in DefaultHttp2FrameWriter
* Fix buffer leaks in tests
Result:
Tests pass without leaks
Motivation:
Draft 12 has just arrived and has quite a few changes. Need to update in
order to keep current with the spec.
Modifications:
This is a rewrite of the original (draft 10) code. There are only 2
handlers now: preface and connection. The connection handler is now
callback based rather than frame based (there are no frame classes
anymore). AbstractHttp2ConnectionHandler is the base class for any
HTTP/2 handlers. All of the stream priority logic now resides in the
outbound flow controller, and its interface exposes methods for
adding/updating priority for streams.
Upgraded to hpack 0.7.0, which is used by draft12. Also removed
draft10 code and moved draft12 code to the ../http2 package
(no draft subpackage).
Result:
Addition of a HTTP/2 draft 12 support.
Motivation:
See: https://github.com/netty/netty/issues/2402
See: https://tools.ietf.org/html/draft-ietf-httpbis-http2-10#section-3.5
Only the client should send the preface string, the server should not.
"The server connection header consists of just a SETTINGS frame (Section 6.5)
that MUST be the first frame the server sends in the HTTP/2 connection."
Modifications:
Split out Http2ClientPrefaceWriter and Http2ServerPrefaceReader from
Http2FrameEncoder and Http2FrameDecoder. The new channel handlers are added
to the pipeline and remove themselves after the preface is written/read.
Result:
HTTP2 client sends preface, server does not, in compliance with spec.
Motivation:
HTTP2 is generally negotiated over SSL, makes more sense to provide an SSL example.
Modifications:
Copy the SDPY example to add SSL and NPN negotiation to the HTTP2 example.
Result:
Http2Server and Http2Client examples use SSL.
and initial settings frame were sent.
Motivation:
The current logic for marking the connection header and initial settings
frame is a bit complicated.
Modifications:
- Http2FrameEncoder, Http2ConnectionHandler: removed sent listeners and
set the sent flag immediately.
Result:
Just a code cleanup ... behavior is the same.
Motivation:
The current HTTP/2 support does not properly comply with the HTTP/2 spec
wrt startup.
Modifications:
Changed the frame codec as well as the connection handler to support
exchange of the connection preface, followed immediately by an initial
settings frame.
Result:
The HTTP/2 initialization handshake will be in compliance with the spec.
Will need more work to support the upgrade protocols, however :)
Motivation:
Provide some example code to show how to bootstrap client and server for
use with HTTP/2 framing.
Modifications:
- Fixed Http2ConnectionHandler to allow headers after stream creation.
Needed for response headers.
- Added toString() to all frame classes to help with debugging/logging
- Added example classes for HTTP/2
Result:
HTTP/2 connections now properly support response headers. Examples for
HTTP/2 provided with the distribution of examples module.
After your change, what will change.
Motivation:
Bring the http2 codec more inline with the rest of Netty's code-base.
Modifications:
- Add null checks in constructors
- Move private static methods into inner-classes where they are used
- Use getBytes(CharsetUtil.UTf8)
Result:
More consistent code-base
Motivation:
The use of the guava library library is fairly superficial and can
easily be removed to reduce the overall dependency list of the module.
Cleaning up the HTTP2 code to use common Netty utilities, etc.
Modifications:
- Various changes to remove use of guava preconditions and collections.
- Changed default charset to use CharsetUtil.UTF_8.
- Changed StreamState to use ArrayDeque instead of LinkedList.
- Changed precondition checks to throw NPE
- Changed implementation of Http2Headers to more closely mirror the SPDY
implementation.
Result:
The behavior of the HTTP2 module will remain unchanged.
Motivation:
Needed a rough performance comparison between SPDY and HTTP 2.0 framing.
Expected performance gains were seen in HTTP 2.0 due to header
compression.
Modifications:
Added a new codec-http2 module containing all of the new source and unit
tests. Updated the top-level pom.xml to add this as a child module.
Result:
Netty will have basic support for HTTP2.
