Skip to content

netty: add soft Metadata size limit enforcement. #11603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Oct 28, 2024
Merged

netty: add soft Metadata size limit enforcement. #11603

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ac70ff8
netty: add soft Metadata size limit enforcement.
ran-su Oct 7, 2024
32c877c
fix linter issues.
ran-su Oct 7, 2024
7de2450
address review comments
ran-su Oct 23, 2024
396d5a2
format files
ran-su Oct 23, 2024
3ad0492
format files
ran-su Oct 23, 2024
2d3b4b3
add returns for client onHeadersRead early rejects.
ran-su Oct 25, 2024
8ced5de
Use manual setup instead of change values for all tests.
ran-su Oct 25, 2024
07117cb
Add a comment for SuppressWarnings.
ran-su Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Use manual setup instead of change values for all tests.
  • Loading branch information
@ran-su
ran-su committed Oct 25, 2024
commit 8ced5deb5c48755a31947e3ac68ff66fd2c47f93
8 changes: 6 additions & 2 deletions netty/src/test/java/io/grpc/netty/NettyClientHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ public class NettyClientHandlerTest extends NettyHandlerTestBase<NettyClientHand
private NettyClientStream.TransportState streamTransportState;
private Http2Headers grpcHeaders;
private long nanoTime; // backs a ticker, for testing ping round-trip time measurement
private int maxHeaderListSize = 8000;
private int softLimitHeaderListSize = 2000;
private int maxHeaderListSize = Integer.MAX_VALUE;
private int softLimitHeaderListSize = Integer.MAX_VALUE;
private int streamId = STREAM_ID;
private ClientTransportLifecycleManager lifecycleManager;
private KeepAliveManager mockKeepAliveManager = null;
Expand Down Expand Up @@ -222,6 +222,10 @@ public Void answer(InvocationOnMock invocation) throws Throwable {
@Test
@SuppressWarnings("InlineMeInliner")
Copy link
Member

Choose a reason for hiding this comment

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

What was wanting to be inlined?

Copy link
Member Author

Choose a reason for hiding this comment

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

The inliner wants me to use string.repeat() instead of Strings.repeat() but that method is JDK 11+. I am not sure we want to use JDK 11 methods even for tests, so I suppressed the warning.

Copy link
Member

Choose a reason for hiding this comment

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

:-/ . That sort of thing is going to get more annoying. But ignoring it is right. We'll often put a comment on the line with the annotation (e.g., // String.repeat() requires Java 11) to make clear why it is there.

public void sendLargerThanSoftLimitHeaderMayFail() throws Exception {
maxHeaderListSize = 8000;
softLimitHeaderListSize = 2000;
manualSetUp();

createStream();
// total head size of 7999, soft limit = 2000 and max = 8000.
// This header has 5999/6000 chance to be rejected.
Expand Down
Loading