-
Notifications
You must be signed in to change notification settings - Fork 170
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
Add relaxed padding setting to the nclient4 #543
Add relaxed padding setting to the nclient4 #543
Conversation
69d44fc
to
475cd0f
Compare
@pmazzini review required |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #543 +/- ##
==========================================
+ Coverage 73.77% 73.83% +0.05%
==========================================
Files 81 81
Lines 5122 5141 +19
==========================================
+ Hits 3779 3796 +17
- Misses 1200 1202 +2
Partials 143 143 ☔ View full report in Codecov by Sentry. |
I am not sure if we should make this an optional parsing or if it should make it the default one. cc @hugelgupf |
Signed-off-by: Bogdan-Denisenko <bogdandenisenko28@gmail.com>
475cd0f
to
79d51f4
Compare
Yes, I think it makes sense to make this behavior the default. I made it optional just to avoid changing the behavior of the exported functions. |
Which functions? What is the benefit of having 2 behaviors? |
Functions |
Continuation in #545 |
RFC 2132 says:
The end option marks the end of valid information in the vendor field. Subsequent octets should be filled with pad options.
But it seems that this does not mean that the client should consider packages with non-pad options after the End option invalid, because RFC 2119 explains:
3. SHOULD This word, or the adjective "RECOMMENDED", mean that there may exist valid reasons in particular circumstances to ignore a particular item, but the full implications must be understood and carefully weighed before choosing a different course.
So we need a way to accept packets not padded with zeros.