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

Fixed incorrect truncated tlv error that prevents reading empty (0 byte) TLVs #57

Merged
merged 2 commits into from
Dec 9, 2020

Conversation

astromechza
Copy link
Contributor

We hit this issue when reading a 0 byte custom TLV value. Testing identified that this was because the length was 0 bytes and the length check in this PR was affecting it.

…te) TLVs

We hit this issue when reading a 0 byte custom TLV value. Testing identified that this was because the length was 0 bytes and the length check in this PR was affecting it.
@coveralls
Copy link

coveralls commented Dec 9, 2020

Coverage Status

Coverage remained the same at 93.514% when pulling 2cb1b5b on astromechza:patch-1 into 152f707 on pires:master.

@emersion
Copy link
Contributor

emersion commented Dec 9, 2020

This change looks good. Can you add a test with a 0-length TLV?

@astromechza
Copy link
Contributor Author

@emersion I've added the test now and checked that it fails on the old code 👍

Any idea how we get a new release cut?

@astromechza
Copy link
Contributor Author

@pires what do you think of this diff?

Copy link
Contributor

@emersion emersion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@emersion
Copy link
Contributor

emersion commented Dec 9, 2020

Any idea how we get a new release cut?

Once this PR is merged, you can just go get github.com/pires/go-proxyproto@<commit> in your project to get the fix. Or wait for @pires to tag a new release.

Copy link
Owner

@pires pires left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great catch, thanks a lot, @astromechza!

@pires pires merged commit adbbabe into pires:master Dec 9, 2020
@pires
Copy link
Owner

pires commented Dec 9, 2020

Tagged v0.3.3 and will handle release notes over the coming days.

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.

4 participants