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

ByteBufInputStream.read() may have a problem #9305

Closed
xiaoheng1 opened this issue Jun 29, 2019 · 3 comments
Closed

ByteBufInputStream.read() may have a problem #9305

xiaoheng1 opened this issue Jun 29, 2019 · 3 comments

Comments

@xiaoheng1
Copy link
Contributor

i use netty4.1.34

souce code:

public int read() throws IOException {
        if (!buffer.isReadable()) {
            return -1;
        }
        return buffer.readByte() & 0xff;
    }

In my opinion, the buffer.isReadable() method should not be used here to judge, but the available() method should be used. Because ByteBufInputStream is passed in length when constructing, so if the buffer.isReadable() method is used here If judged, it may exceed the limit of length, which is unreasonable.

@normanmaurer
Copy link
Member

@xiaoheng1 good point... are you willing to open a PR ?

@xiaoheng1
Copy link
Contributor Author

@normanmaurer I am glad to,i will open a PR.

@xiaoheng1
Copy link
Contributor Author

#9306 Fix public int read() throws IOException method exceeds the limit of length

normanmaurer pushed a commit that referenced this issue Jul 1, 2019
…length (#9306)

Motivation:

buffer.isReadable() should not be used to limit the amount of data that can be read as the amount may be less then was is readable.

Modification:

- Use  available() which takes the length into account
- Add unit test

Result:

Fixes #9305
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

No branches or pull requests

2 participants