Skip to content

HTTP/2 encoder: allow HEADER_TABLE_SIZE greater than Integer.MAX_VALUE#13368

Merged
normanmaurer merged 1 commit into
netty:4.1from
idelpivnitskiy:DefaultHttp2ConnectionEncoder
May 2, 2023
Merged

HTTP/2 encoder: allow HEADER_TABLE_SIZE greater than Integer.MAX_VALUE#13368
normanmaurer merged 1 commit into
netty:4.1from
idelpivnitskiy:DefaultHttp2ConnectionEncoder

Conversation

@idelpivnitskiy

@idelpivnitskiy idelpivnitskiy commented May 1, 2023

Copy link
Copy Markdown
Member

Motivation:

HpackEncoder.setMaxHeaderTableSize(...) supports any unsigned 32-bit integer value, however DefaultHttp2ConnectionEncoder adjusts the SETTINGS_HEADER_TABLE_SIZE value to 31-bit unsigned integer. DefaultHttp2ConnectionDecoder doesn't adjust. We should keep them consistent. Value range is validated by Http2Settings and HpackEncoder.

Modifications:

  • Remove adjustment for SETTINGS_HEADER_TABLE_SIZE in DefaultHttp2ConnectionEncoder.remoteSettings(...);

Result:

DefaultHttp2ConnectionEncoder.remoteSettings(...) is consistent with DefaultHttp2ConnectionDecoder.applyLocalSettings(...), SETTINGS_HEADER_TABLE_SIZE value is not adjusted.

Motivation:

`HpackEncoder.setMaxHeaderTableSize(...)` supports any unsigned 32-bit
integer value, however `DefaultHttp2ConnectionEncoder` adjusts the
`SETTINGS_HEADER_TABLE_SIZE` value to 31-bit unsigned integer.
`DefaultHttp2ConnectionDecoder` doesn't adjust. We should keep them
consistent. Value range is validated by `Http2Settings` and
`HpackEncoder`.

Modifications:

- Remove adjustment for `SETTINGS_HEADER_TABLE_SIZE` in
`DefaultHttp2ConnectionEncoder.remoteSettings(...)`;

Result:

`DefaultHttp2ConnectionEncoder.remoteSettings(...)` is consistent with
`DefaultHttp2ConnectionDecoder.applyLocalSettings(...)`,
`SETTINGS_HEADER_TABLE_SIZE` value is not adjusted.
@normanmaurer normanmaurer added this to the 4.1.93.Final milestone May 2, 2023
@normanmaurer normanmaurer merged commit d90c59d into netty:4.1 May 2, 2023
normanmaurer pushed a commit that referenced this pull request May 2, 2023
#13368)

Motivation:

`HpackEncoder.setMaxHeaderTableSize(...)` supports any unsigned 32-bit
integer value, however `DefaultHttp2ConnectionEncoder` adjusts the
`SETTINGS_HEADER_TABLE_SIZE` value to 31-bit unsigned integer.
`DefaultHttp2ConnectionDecoder` doesn't adjust. We should keep them
consistent. Value range is validated by `Http2Settings` and
`HpackEncoder`.

Modifications:

- Remove adjustment for `SETTINGS_HEADER_TABLE_SIZE` in
`DefaultHttp2ConnectionEncoder.remoteSettings(...)`;

Result:

`DefaultHttp2ConnectionEncoder.remoteSettings(...)` is consistent with
`DefaultHttp2ConnectionDecoder.applyLocalSettings(...)`,
`SETTINGS_HEADER_TABLE_SIZE` value is not adjusted.
@idelpivnitskiy idelpivnitskiy deleted the DefaultHttp2ConnectionEncoder branch May 2, 2023 20:36
dongjoon-hyun pushed a commit to apache/spark that referenced this pull request Jun 28, 2023
### What changes were proposed in this pull request?
This pr aims to upgrade netty from 4.1.92 to 4.1.93.

### Why are the changes needed?
1.v4.1.92 VS v4.1.93
netty/netty@netty-4.1.92.Final...netty-4.1.93.Final

2.The new version brings some bug fix, eg:
- Reset byte buffer in loop for AbstractDiskHttpData.setContent ([#13320](netty/netty#13320))
- OpenSSL MAX_CERTIFICATE_LIST_BYTES option supported ([#13365](netty/netty#13365))
- Adapt to DirectByteBuffer constructor in Java 21 ([#13366](netty/netty#13366))
- HTTP/2 encoder: allow HEADER_TABLE_SIZE greater than Integer.MAX_VALUE ([#13368](netty/netty#13368))
- Upgrade to latest netty-tcnative to fix memory leak ([#13375](netty/netty#13375))
- H2/H2C server stream channels deactivated while write still in progress ([#13388](netty/netty#13388))
- Channel#bytesBefore(un)writable off by 1 ([#13389](netty/netty#13389))
- HTTP/2 should forward shutdown user events to active streams ([#13394](netty/netty#13394))
- Respect the number of bytes read per datagram when using recvmmsg ([#13399](netty/netty#13399))

3.The release notes as follows:
- https://netty.io/news/2023/05/25/4-1-93-Final.html

4.Why not upgrade to `4-1-94-Final` version?
Because the return value of the 'threadCache()' (from `PoolThreadCache` to `PoolArenasCache`) method of the netty Inner class used in the 'arrow memory netty' version '12.0.1' has changed and belongs to break change, let's wait for the upgrade of the 'arrow memory netty' before upgrading to the '4-1-94-Final' version.

The reference is as follows:
https://github.com/apache/arrow/blob/6af660f48472b8b45a5e01b7136b9b040b185eb1/java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java#L164
https://github.com/netty/netty/blob/da1a448d5bc4f36cc1744db93fcaf64e198db2bd/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java#L732-L736

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Pass GA.

Closes #41681 from panbingkun/upgrade_netty.

Authored-by: panbingkun <pbk1982@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants