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

v2: read length and skip the bytes for UNSPEC #62

Merged
merged 1 commit into from
Jan 20, 2021
Merged

v2: read length and skip the bytes for UNSPEC #62

merged 1 commit into from
Jan 20, 2021

Conversation

bohanyang
Copy link
Contributor

@bohanyang bohanyang marked this pull request as draft January 19, 2021 07:04
@coveralls
Copy link

coveralls commented Jan 19, 2021 •
edited

Coverage Status

Coverage increased (+0.1%) to 94.216% when pulling 70665b5 on bohanyang:unspec-zero-length into 22bc614 on pires:main.

@bohanyang bohanyang changed the title Read length zero for v2 UNSPEC v2: read length and skip the bytes for UNSPEC Jan 19, 2021
@bohanyang bohanyang marked this pull request as ready for review January 19, 2021 09:39
pires
pires requested changes Jan 19, 2021
View reviewed changes
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.

Nice catch!

v2.go Outdated Show resolved Hide resolved
v2.go Show resolved Hide resolved
v2.go Show resolved Hide resolved
v2.go Outdated Show resolved Hide resolved
v2.go Outdated Show resolved Hide resolved
v2_test.go Show resolved Hide resolved
v2_test.go Show resolved Hide resolved
@pires pires added the bug label Jan 19, 2021
@pires pires added this to the 0.4 milestone Jan 19, 2021
pires
pires reviewed Jan 19, 2021
View reviewed changes
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.

One last thing and I think we're good to go. Thank you very much for your patience and contribution!

v2_test.go Show resolved Hide resolved
pires
pires reviewed Jan 20, 2021
View reviewed changes
v2_test.go Show resolved Hide resolved
@bohanyang
Copy link
Contributor Author

Just found that there're two tests with desc TCPv4 length zero but with address and ports, should I fix the desc or the fixture header?

@pires
Copy link
Owner

pires commented Jan 20, 2021

Nice catch! Please, fix the desc of the last one which is clearly a test for TCPv6.

@bohanyang
Copy link
Contributor Author

Done!

pires
pires approved these changes Jan 20, 2021
View reviewed changes
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.

What a great contribution! Thank you very much, @bohanyang

@pires pires merged commit b6f440c into pires:main Jan 20, 2021
@pires
Copy link
Owner

pires commented Jan 20, 2021

https://github.com/pires/go-proxyproto/releases/tag/v0.4.1

@bohanyang bohanyang mentioned this pull request Jan 20, 2021
Merged
@bohanyang bohanyang deleted the unspec-zero-length branch January 20, 2021 11:11
This was referenced Mar 8, 2021
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pires pires pires approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
0.4
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bohanyang @coveralls @pires