lib: deprecate fromHexString on dodgy inputs#434218
Conversation
nixpkgs-ci
bot
added
10.rebuild-linux: 1-10
emilazy
force-pushed
the
push-wwwsplnotwlx
branch
from
ae1bd4f to
3c7d67d
Compare
|
(Whoops, made the regex slightly too strict… fixed and added a test to catch my mistake. We could also choose to allow longer strings of leading zeroes, but I’d prefer to permit only the minimal format that people are observably using in the wild for now, and worry about all the rest after we have closed off the rest of the input space.) |
| str: | ||
| let | ||
| noPrefix = lib.strings.removePrefix "0x" (lib.strings.toLower value); | ||
| match = builtins.match "(0x)?([0-7]?[0-9A-Fa-f]{1,15})" str; |
There was a problem hiding this comment.
maybe should be case insensitive in the 0x part? idk. we did have a test for it working.
There was a problem hiding this comment.
I couldn’t find any examples of people using that in my GitHub search, and it’s inconsistent with the syntax of TOML and C (although Python does allow it). I’d prefer to err on the side of conservatism here so that we can buy ourselves back maximum freedom to determine the input space without (hopefully) breaking any existing users, though I’m not super fussed one way or the other here. We’re already in the territory of breaking the tests/examples out of necessity. Honestly I don’t even like the optional 0x here, but there are users of it in the wild, so we shouldn’t break them without good reason; I think this is the most rigid non‐arbitrary format that covers everything I saw. (Well, possibly we could disallow mixed case in the actual digits too, but I don’t see the point.)
nixpkgs-ci
bot
added
the
12.approvals: 1
nixpkgs-ci
bot
added
12.approvals: 2
3 participants
See #433710.
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.