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

Fix underflow in recv #53

Merged
merged 2 commits into from
Nov 10, 2024
Merged

Fix underflow in recv #53

merged 2 commits into from
Nov 10, 2024

Conversation

MahieDev
Copy link
Contributor

@MahieDev MahieDev commented Nov 9, 2024

Fairly new to rust, so might miss something obvious.

Nevertheless, I am experiencing an underflow when performing adb pull, as effective_read may be greater than length.

Fixed by using the length as a reference for the number of bytes to read. Again, not sure if this is the most optimal solution, but works for me.

@cocool97
Copy link
Owner

cocool97 commented Nov 9, 2024

Thanks for the PR, do you have something that might help me to reproduce this issue ?

I haven't had this error before, so this would be nice to reproduce it !

@MahieDev
Copy link
Contributor Author

MahieDev commented Nov 9, 2024

Sure, no problem. Just pushed a minimal .txt with contents "abc" to /data/local/tmp

fn main() {
    let mut server = ADBServer::default();
    let mut device = server.get_device().expect("cannot get device");
    let stdout = io::stdout();
    let mut handle = stdout.lock();
    device.pull("data/local/tmp/abc.txt", &mut handle).unwrap();
}

Amazing repo btw! :)

@cocool97
Copy link
Owner

cocool97 commented Nov 9, 2024

I can indeed reproduce it. It's maybe related to the size of the file pulled, but I have to check.

Thanks for pinpointing it !

@cocool97
Copy link
Owner

cocool97 commented Nov 9, 2024

Looks like the error is related to the fact we pass buf to read, that may have a size bigger then what we have to read.

Can you try with something like this:

let effective_read = self.inner.read(&mut buf[0..length])?;

@MahieDev
Copy link
Contributor Author

MahieDev commented Nov 9, 2024

Yeah, works perfectly, cool!

@cocool97 cocool97 merged commit ec0ae68 into cocool97:main Nov 10, 2024
4 checks passed
@cocool97
Copy link
Owner

Merged, thanks for the PR ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants