Repository navigation
http-api: advance readerIndex in FormUrlEncodedSerializer.deserialize - #3733
Merged
idelpivnitskiy merged 2 commits intoNov 14, 2026
Merged
Conversation
#### Motivation `Deserializer` says the `readerIndex()` will be advanced to show the content that was consumed. `FormUrlEncodedSerializer.deserialize` read the buffer with `toString(charset)`, which does not move the index, so callers saw the whole form as still readable (#3732). #### Modifications - Skip the readable bytes after decoding them to a `String`, the same way `AbstractStringSerializer` does. - Add `FormUrlEncodedSerializerTest` for heap and direct buffers: the index ends at the writer index, decoding starts at the current reader index, and a serialize/deserialize round trip leaves nothing readable. #### Result `FormUrlEncodedSerializer.deserialize` consumes the bytes it decodes. Fixes #3732
idelpivnitskiy
requested changes
Nov 14, 2026
idelpivnitskiy
left a comment
idelpivnitskiy
left a comment
Member
There was a problem hiding this comment.
Thank you for the contribution!
#### Motivation Review on #3733: `decodeQueryParams` can throw (bad escape, too many params), but the bytes were skipped before decoding. Also the deprecated `FormUrlEncodedHttpDeserializer` kept its own copy of the decode logic and still did not advance `readerIndex()`. #### Modifications - `FormUrlEncodedSerializer.deserialize` skips the readable bytes only after `decodeQueryParams` returns, like `ProtobufSerializer`. - `FormUrlEncodedHttpDeserializer` delegates to `FormUrlEncodedSerializer.deserialize(buffer, charset)`. - Tests: malformed input leaves `readerIndex()` unchanged; the deprecated deserializer advances `readerIndex()`. #### Result A failed decode leaves the buffer as it was, and both deserializers consume the bytes they decode.
Contributor
Author
|
The red JDK 11 job is |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Deserializersays thereaderIndex()will be advanced to show the content that was consumed.FormUrlEncodedSerializer.deserializeread the buffer withtoString(charset), which does not move the index, so callers saw the whole form as still readable.Modifications
ProtobufSerializer, so a failure leaves the buffer untouched.FormUrlEncodedHttpDeserializernow delegates toFormUrlEncodedSerializer.deserialize, so it also advancesreaderIndex.FormUrlEncodedSerializerTestfor heap and direct buffers: the index ends at the writer index, decoding starts at the current reader index, a serialize/deserialize round trip leaves nothing readable, and malformed input (a=%zz) leavesreaderIndexunchanged.FormUrlEncodedHttpDeserializerTestcase that checks the deprecated deserializer advancesreaderIndex.Result
FormUrlEncodedSerializer.deserializeand the deprecatedFormUrlEncodedHttpDeserializerconsume the bytes they decode.Fixes #3732