search

insomniacslk / dhcp

DHCPv6 and DHCPv4 packet library, client and server written in Go
BSD 3-Clause "New" or "Revised" License
710 stars 169 forks source link

Add Support for Labels Containing Partial Domain Names (RFC 4704 Section 4.2) #467

Closed rtpt-erikgeiser closed 2 years ago

rtpt-erikgeiser commented 2 years ago

This PR only contains a failing test case for #466. I don't know enough about RFC 1035 (especially compressed ones) to be comfortable implementing a solution that does not break anything.

rtpt-erikgeiser commented 2 years ago

With this PR, partial domain name labels are now correctly decoded. However, the information whether the label is partial or not is not exposed to the user and marshaling discards the information.

For a complete solution it would probably be necessary to save the information whether it is partial or not for each label and present this to the user somehow. Therefore, I think it is best just to be able to decode partial labels first.

pmazzini commented 2 years ago

@rtpt-erikgeiser Sorry for the late review. I left a comment.

codecov[bot] commented 2 years ago

Codecov Report

Base: 67.39% // Head: 67.41% // Increases project coverage by +0.01% :tada:

Coverage data is based on head (034ec2b) compared to base (3194d6d). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #467 +/- ## ========================================== + Coverage 67.39% 67.41% +0.01% ========================================== Files 90 90 Lines 3797 3799 +2 ========================================== + Hits 2559 2561 +2 Misses 1061 1061 Partials 177 177 ``` | Flag | Coverage Δ | | |---|---|---| | integtests | `∅ <ø> (∅)` | | | unittests | `67.41% <100.00%> (+0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=insomniac#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/insomniacslk/dhcp/pull/467?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=insomniac) | Coverage Δ | | |---|---|---| | [rfc1035label/label.go](https://codecov.io/gh/insomniacslk/dhcp/pull/467/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=insomniac#diff-cmZjMTAzNWxhYmVsL2xhYmVsLmdv) | `89.85% <100.00%> (+0.30%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=insomniac). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=insomniac)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

rtpt-erikgeiser commented 2 years ago

@pmazzini I removed the superfluous return statement.