Skip to content

Use .To4() on serverId#14

Open
tfheen wants to merge 1 commit into
krolaw:masterfrom
tfheen:master
Open

Use .To4() on serverId#14
tfheen wants to merge 1 commit into
krolaw:masterfrom
tfheen:master

Conversation

@tfheen

@tfheen tfheen commented Apr 15, 2015

Copy link
Copy Markdown
Contributor

The server id should be four octets, per RFC1533, so call .To4 on it
to ensure we have a v4 address and not a mapped v4 address.

The server id should be four octets, per RFC1533, so call .To4 on it
to ensure we have a v4 address and not a mapped v4 address.
@tfheen

tfheen commented Apr 15, 2015

Copy link
Copy Markdown
Contributor Author

I wasn't entirely sure if you wanted to keep the API, .To4() can return nil if serverId is a v6 address, in which case the ReplyPacket should probably fail. Not using .To4() means you can end up sending a 16 octet long ServerIdentifier, which doesn't work with all DHCP clients (at least the ISC DHCP client is unhappy with it).

@krolaw

krolaw commented Apr 16, 2015

Copy link
Copy Markdown
Owner

I'm going to think about this some more.
If IPv4 nochange.
If mapped v4 corrects a potential bug.
If IPv6 goes from invalid serverId (too long) to invalid serverId (zero length).

However behaviour would be inconsistent between ReplyPacket and .AddOption which could make a bug harder to find.

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