Skip to content
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

Jump to bottom

LengthDelimitedCodec::length_field_type()/length_field_len() should also update max_frame_length #5080

Closed
crusader-mike opened this issue Oct 6, 2022 · 1 comment · Fixed by #6414
Closed

LengthDelimitedCodec::length_field_type()/length_field_len() should also update max_frame_length #5080

crusader-mike opened this issue Oct 6, 2022 · 1 comment · Fixed by #6414
Labels
A-tokio-util Area: The tokio-util crate C-bug Category: This is a bug. M-codec Module: tokio-util/codec

Comments

@crusader-mike
Copy link
Contributor

crusader-mike commented Oct 6, 2022 •
edited

Version tokio-util v0.7.2

Platform Win7

Description
Currently LengthDelimitedCodecs Encodertruncates frame length, which leads to corrupted data (e.g. 6Mb frame prefixed with 2-bytes length).

I expected to see this happen:

  • calling length_field_type::<u16>() (or length_field_len(2)) should also set max_frame_length to min(65535, max_frame_length)
  • calling max_frame_length(N) should also update length_field_len to smallest M such that N < (1 << 8*M) (probably not a good idea since it will change network-lvl presentation... maybe panic, if length_field_len is too small to accomodate N?)
@crusader-mike crusader-mike added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Oct 6, 2022
@Darksonn Darksonn added A-tokio-util Area: The tokio-util crate M-codec Module: tokio-util/codec and removed A-tokio Area: The main tokio crate labels Oct 6, 2022
@crusader-mike
Copy link
Contributor Author

Hmm... maybe the best approach would be to validate params in Builder::new_codec() and fail if some params are incompatible/invalid?

matheus-consoli added a commit to matheus-consoli/tokio that referenced this issue Jun 22, 2023
@matheus-consoli
Fix tokio-rs#5080: Handle possible dangling reference safely
66b3679
matheus-consoli added a commit to matheus-consoli/tokio that referenced this issue Jun 23, 2023
@matheus-consoli
Fix tokio-rs#5080: Handle possible dangling reference safely
ed19ce8
@maminrayej maminrayej mentioned this issue Mar 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-tokio-util Area: The tokio-util crate C-bug Category: This is a bug. M-codec Module: tokio-util/codec
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

util: assert compatibility between LengthDelimitedCodec options maminrayej/tokio
2 participants
@Darksonn @crusader-mike