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

TLV capture and parsing for proxy protocol v2 #9

Closed
wants to merge 8 commits into from

Conversation

stu-elastic
Copy link
Contributor

  • Adds rawTLVs to Header struct
  • API Change
    • non-io.EOF errors encountered when reading from payload reader are returned
      from parseVersion2 and Read
    • Padding is saved in rawTLV
  • TLV splitting is implemented by Header.TLVs(), TLV length validation without
    this explicit call
  • Includes helpers for parsing AWS VPCEs and SSL information
  • Parsing of other types possible by consumers via the PP2Type constants using
    the TLV and PP2SSL exported types

… size for rawTLVs, add rawTLVs to formatted header
 * Adds rawTLVs to Header struct
 * API Change 
   - non-io.EOF errors encountered when reading from payload reader are returned
     from parseVersion2 and Read
   - Padding is saved in rawTLV
 * TLV splitting is implemented by `Header.TLVs()`, TLV length validation without
   this explicit call
 * Includes helpers for parsing AWS VPCEs and SSL information
 * Parsing of other types possible by consumers via the `PP2Type` constants using
   the TLV and PP2SSL exported types
README.md Show resolved Hide resolved
@Freeaqingme
Copy link
Contributor

Given the overlap with #2 I'll see if I can give this code a try later this week and see if it also accomplishes what I was trying to do with #2. Not that that should be the goal in itself, but I'd like to see if - if we start changing the API - can do so with only a single change rather than having to change it multiple times.

@stu-elastic
Copy link
Contributor Author

I'd like to see if - if we start changing the API - can do so with only a single change rather than having to change it multiple times.

This is a backwards compatible change, if there are non-bwc API changes that are required for #2 then I'd suggest rolling a new major version while letting this go into a minor version release.

@stu-elastic
Copy link
Contributor Author

Closed in favor of #10

@Freeaqingme
Copy link
Contributor

This is a backwards compatible change

Good point. Though you are extending the public interface a bit, so I'd like to see if it also suffices for my original requirements. Just to see if we can prevent a future BC break. (or at least know so in advance)

@pires
Copy link
Owner

pires commented Aug 21, 2019

Maybe it's time to do releases and stop getting this from master. dep/go modules have come a long way.

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.

None yet

3 participants