Skip to content

normalize single hex digit fields to two hex digits#20

Open
hagman wants to merge 1 commit intojpoliv:masterfrom
hagman:fix-single-hex-digit-bug
Open

normalize single hex digit fields to two hex digits#20
hagman wants to merge 1 commit intojpoliv:masterfrom
hagman:fix-single-hex-digit-bug

Conversation

@hagman
Copy link
Copy Markdown

@hagman hagman commented Aug 17, 2025

The "canonical" and "windows" format for MAC addresses with six hex fields separated by colon or dash allow single digit fields (the corresponding regular expressions explicitly use {1,2} to allow single or double digits), but these are converted incorrectly to packet octets.
For example, inputs 01-2-03-04-05-06 and 01-2-3-04-05-06 should both act like 01-02-03-04-05-06, but the first acts like 01-20-30-40-50-06 (shifted nibbles), the second like 01-23-04-05-06 (too short). Neither of these is even remotely likely to be what the user intended. On the other hand, 01-02-03-04-05-6 does act like 01-02-03-04-05-06.

To fix this, one could either (a) be less lenient with user input and reject input with single digit fields as invalid, or (b) ensure correct interpretation of such input.

Considering it the safer and more user-friendly choice, I followed option (b) by adding an intermediate step to insert leading zeros where appropriate. Admittedly, this breaks installations where a user purposefully uses 01-2-03-04-05-06 to stand for 01-20-30-40-50-06, or uses 01-2-3-04-05-06 to purposefully send a completely invalid WOL packet. I consider this highly unlikely.

If there is a normative specification of these two formats that explicitly forbids such single digit edge cases, it would be cleaner to follow option (a) instead (i.e., replace {1,2} with {1} in the respective regexes). However, I am unaware of any such specification. As an example, RFC7042 uses leading zeros, but describes that convention merely as "notation used in this document".

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