Skip to content

DHCPv4 options as list - #587

Open
scufre wants to merge 2 commits into
insomniacslk:masterfrom
scufre:options.as.list.with.padding
Open

DHCPv4 options as list#587
scufre wants to merge 2 commits into
insomniacslk:masterfrom
scufre:options.as.list.with.padding

Conversation

@scufre

@scufre scufre commented Aug 12, 2026

Copy link
Copy Markdown

Changing to store DHCPv4 options as a list instead of map and allow inserting padding options.

This is to workaround this issue https://binaryfury.wann.net/2024/12/pxe-two-tftp-filenames-one-dhcp-offer-plus-weird-filenames/ where old hardware treats the boot filename option as a null terminated string instead of relying on the option length.

To do so, the Options type was changed to be a struct with a single field that is an array of a newly created struct (optionCodeValue) having the option code and option data. And then removed a couple of cases where padding options where filtered out.
Some disclaimer on the decision to use a struct with a single field being the array of options: I'm new to go and did a try on directly declaring the Options type as an array, but then couldn't find a way of declaring those methods that need to add or remove elements in the array as that in my understanding requires creating a new array.

With these changes, now a the bootfile option can be provided and right after it, a padding option can be added, which will null terminate the string for those buggy clients.

@pmazzini

pmazzini commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Initially the DHCPv4 library used to have a list of option but after @hugelgupf refactor it is now a map. This fits what the RFC specifies. I am not a great fan of changing the library behaviour to make non RFC compliant clients work.

@pmazzini pmazzini left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment

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