display: fix fmt_hex_max! to not panic when given precision greater than byte length - #261
Conversation
|
BTW @Kixunil do you have access to the hex-conservative Project Loupe repo? There are only 7 issues there, and at a glance they all have this flavor to them. Probably you could fix several of them at once in a principled series of fixes, similar to what I recently did with rust-bech32. |
|
This is a breaking change, right? If we don't want to break the API another solution would be to deprecate |
| encoder.put_bytes(bytes.into_iter().take(n)); | ||
| &encoder.as_str()[..p] | ||
| let bytes = encoder.put_bytes(bytes.into_iter().take(n)); | ||
| &encoder.as_str()[..core::cmp::min(p, bytes * 2)] |
There was a problem hiding this comment.
Changing the return type is a breaking change because someone could've passed the function to a function that expects a function returning (). But it's not required, just write encoder.as_str().get(..p).unwrap_or(encoder.as_str()).
There was a problem hiding this comment.
Oh, derp, I had started to write this, then tried to combine the unwrap_or branch with the p >= N branch, and then went off down a rabbit hole doing this more complicated thing.
There was a problem hiding this comment.
The code confused me too, LOL. I was also feeling like that thing should either work or should need fewer checks and spent a bunch of time thinking it through. Maybe it needs a good comment.
|
And yes, I do have access, I just thought I saw PRs to fix them already but after looking more closely at this one, I now see I mistook it for an unrelated issue. |
|
On 866ee09 successfully ran local tests |
|
On 16504d3 successfully ran local tests |
d2bd25b to
a9dfebd
Compare
…byte len and claimed len
a9dfebd to
86fe196
Compare
|
On 86fe196 successfully ran local tests |
3 participants
The
fmt_hex_max!macro is documented to work given some source of bytes and an upper bound on the length of those bytes. However, if the source of bytes has strictly fewer bytes than claimed, and a user uses a formatter with the precision set between those values, there is a panic.This fixes it by tracking the number of bytes consumed by the iterator. I'm not super familiar with how this is supposed to be used or what its optimization goals are, so hopefully adding this extra counter doesn't optimize that.
Fixes https://github.com/project-loupe/audit-hex-conservative/issues/2