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.

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

[BUG] The collect function in FragmentQueue fails to sort frames correctly. #55

Closed
YouZiSoftware opened this issue Jan 26, 2024 · 2 comments
Labels
Bug Something isn't working Complete The code review for this issue has been completed; and is now being finalized. Core A core related issue or PR

Comments

@YouZiSoftware
Copy link

Describe the bug
The sort_by function in the collect method of FragmentQueue is using id for comparison during sorting (it should actually be index), causing the Frame to be sorted incorrectly. As a result, in some instances, the output Vec<u8> may be incorrect.

To Reproduce
Steps to reproduce the behavior:
This issue may probabilistically occur when receiving a Fragment because the sorting is not done correctly, primarily due to the improper use of id instead of index in the sorting process.

Expected behavior
Correcting the usage of the sort_by function, here is the suggested modified code:

frames.sort_by(|a, b| {
                    a.fragment_meta
                        .as_ref()
                        .unwrap()
                        .index
                        .cmp(&b.fragment_meta.as_ref().unwrap().index)
                });

Server versions (please complete the following information):

Additional context
Here is the debug output of my code, and the buffer concatenation is not correct:
code:

for frame in frames.iter() {
                    let fm = frame.fragment_meta.clone().unwrap();
                    println!("index: {}, len: {}", fm.index, frame.body.len());
                    buffer.extend_from_slice(&frame.body);
                }

output:

index: 0, len: 1463
index: 1, len: 1463
index: 5, len: 637
index: 2, len: 1463
index: 3, len: 1463
index: 4, len: 1463
@YouZiSoftware YouZiSoftware added the Bug Something isn't working label Jan 26, 2024
@john-bv john-bv added Core A core related issue or PR In Progress We are working on this Unconfirmed This bug/issue has not been confirmed yet labels Jan 26, 2024
@john-bv
Copy link
Member

john-bv commented Jan 26, 2024

Do you have a input buffer source or no? If not, judging the frame length and indices im assuming this is a MC buffer source.

@john-bv john-bv added Complete The code review for this issue has been completed; and is now being finalized. and removed Unconfirmed This bug/issue has not been confirmed yet In Progress We are working on this labels Jan 26, 2024
@john-bv
Copy link
Member

john-bv commented Jan 27, 2024

This issue should be fixed! Please let me know if you're still having this issue after updating to 0.3.2!

@NetrexMC NetrexMC locked as resolved and limited conversation to collaborators Feb 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Something isn't working Complete The code review for this issue has been completed; and is now being finalized. Core A core related issue or PR
Projects
None yet
Development

No branches or pull requests

2 participants