Skip to content

Restrict option parsers to their compatible options - #487

Draft
hugelgupf wants to merge 7 commits into
insomniacslk:masterfrom
hugelgupf:restrict
Draft

Restrict option parsers to their compatible options#487
hugelgupf wants to merge 7 commits into
insomniacslk:masterfrom
hugelgupf:restrict

Conversation

@hugelgupf

@hugelgupf hugelgupf commented Feb 19, 2023

Copy link
Copy Markdown
Collaborator
  • Top-level messages can only parse options that belong to top-level messages.
  • Relay messages can only parse options that belong to relay messages.
  • IAPD can parse IAPrefix + StatusCode
  • IATA/IANA can parse IAAddress + StatusCode
  • etc etc

All other options are still parsed; the option parser does not return an error. Most RFCs that specify options say that one should ignore options specified in places they shouldn't be, rather than take other actions.

This also moves us toward satisfying requests like #395 -- being able to implement lazy parsing.

@codecov

codecov Bot commented Feb 19, 2023

Copy link
Copy Markdown

Codecov Report

Base: 69.46% // Head: 69.59% // Increases project coverage by +0.13% 🎉

Coverage data is based on head (e8932a8) compared to base (223c67f).
Patch coverage: 80.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #487      +/-   ##
==========================================
+ Coverage   69.46%   69.59%   +0.13%     
==========================================
  Files          90       90              
  Lines        5737     5710      -27     
==========================================
- Hits         3985     3974      -11     
+ Misses       1571     1560      -11     
+ Partials      181      176       -5     
Impacted Files Coverage Δ
dhcpv6/dhcpv6message.go 55.35% <41.37%> (-1.25%) ⬇️
dhcpv6/dhcpv6relay.go 69.51% <50.00%> (-2.72%) ⬇️
dhcpv6/option_ntp_server.go 66.66% <69.23%> (+3.25%) ⬆️
dhcpv6/option_fqdn.go 83.33% <80.00%> (-0.88%) ⬇️
dhcpv6/option_iaprefix.go 81.81% <87.50%> (-1.24%) ⬇️
dhcpv6/option_iapd.go 66.66% <88.23%> (+1.96%) ⬆️
dhcpv6/option_nontemporaryaddress.go 77.94% <88.23%> (-0.03%) ⬇️
dhcpv6/option_4rd.go 85.56% <100.00%> (+0.95%) ⬆️
dhcpv6/option_archtype.go 100.00% <100.00%> (ø)
dhcpv6/option_bootfileparam.go 86.36% <100.00%> (+9.44%) ⬆️
... and 24 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@hugelgupf
hugelgupf force-pushed the restrict branch 2 times, most recently from e8a6e5b to ee037bd Compare February 19, 2023 08:02
@hugelgupf hugelgupf closed this Feb 19, 2023
@hugelgupf hugelgupf reopened this Feb 19, 2023
@hugelgupf
hugelgupf marked this pull request as draft February 19, 2023 19:41
Signed-off-by: Chris Koch <chrisko@google.com>
Signed-off-by: Chris Koch <chrisko@google.com>
Signed-off-by: Chris Koch <chrisko@google.com>
Signed-off-by: Chris Koch <chrisko@google.com>
…tions

RFC 8415 Appendix B and C describe which option codes are allowed in
which options, for most of them anyway.

Signed-off-by: Chris Koch <chrisko@google.com>
Signed-off-by: Chris Koch <chrisko@google.com>
Tests that for the correct option code, the correct deserialization is
applied.

Signed-off-by: Chris Koch <chrisko@google.com>
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.

1 participant