-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
core: add checks to chunked encoding for http/2 #980
Conversation
private long seenContentLength; | ||
|
||
@Override | ||
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be useful to add a comment for each type of violation that you're checking against.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a comment block at the top.
} | ||
if (msg instanceof HttpContent) { | ||
ByteBuf content = ((HttpContent) msg).content(); | ||
seenContentLength = Math.addExact(seenContentLength, content.readableBytes()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest extracting this into a method, for readability. It's stateful and the increment isn't obvious immediately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
return; | ||
} | ||
} | ||
if (expectedContentLength != UNSET_CONTENT_LENGTH && HttpUtil.isTransferEncodingChunked(req)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider extracting usages of expectedContentLength != UNSET_CONTENT_LENGTH
and naming appropriately to convey the intent of that check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
No description provided.