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

Swap 'special' and 'unknown' in enum, so that 'unknown' is default #41

Merged
merged 1 commit into from
Mar 20, 2024

Conversation

mikebeaton
Copy link
Contributor

If 'special' is the default for unknown options then dhcptest refuses to accept them, with 'Fatal error: Can't specify a value for special option NN' when there is no format specifed, or with an assertion at line 616 for basically the same reason, if there is a format specifier.

Swapping so that 'unknown' is the default means that unknown options can be successfully manually specified.

If 'special' is the default for unknown options then dhcptest refuses
to accept them, with 'Fatal error: Can't specify a value for special
option NN' when there is no format specifed, or with an assertion at
line 616 for basically the same reason, if there is a format specifier.

Swapping so that 'unknown' is the default means that unknown options
can be successfully manually specified.
@mikebeaton
Copy link
Contributor Author

mikebeaton commented Mar 19, 2024

Here is an example command which works after the change but won't work before the change:

dhcptest --option "60=HTTPClient:Arch:00016:UNDI:003001" --option "93[hex]=0010" --request 67

and a screenshot from Wireshark showing that we have specified a valid value for a valid option:

Screenshot 2024-03-19 at 21 11 46

While it might also be useful to do a PR to make 93 a known option, I think it is definitely useful to allow unknown options to be specified like this.

@CyberShadow CyberShadow merged commit 4807603 into CyberShadow:master Mar 20, 2024
2 checks passed
@CyberShadow
Copy link
Owner

Thanks!

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.

2 participants