cksum: Fix GNU test cksum-c.sh after bump to 9.9
#9511
Conversation
RenjiSann
force-pushed
the
cksum-fix-cksum-c
branch
from
b03ec78 to
5f883fc
Compare
CodSpeed Performance ReportMerging #9511 will not alter performanceComparing Summary
Footnotes
|
RenjiSann
force-pushed
the
cksum-fix-cksum-c
branch
2 times, most recently
from
cd5feaa to
e522a31
Compare
|
GNU testsuite comparison: |
RenjiSann
force-pushed
the
cksum-fix-cksum-c
branch
from
e522a31 to
19df883
Compare
RenjiSann
changed the title
cksum-c.sh after bump to 9.9cksum-c.sh after bump to 9.9
|
GNU testsuite comparison: |
RenjiSann
force-pushed
the
cksum-fix-cksum-c
branch
from
19df883 to
3279939
Compare
|
GNU testsuite comparison: |
| line_info: &LineInfo, | ||
| len_hint: Option<usize>, | ||
| checksum: &String, | ||
| bytelen_hint: Option<usize>, |
There was a problem hiding this comment.
I would use byte_len_hint because "byte" and "length" are words on their own.
| // checksum with it. | ||
| let digest_char_length_hint = match (algo_kind, algo_byte_len) { | ||
| (AlgoKind::Blake2b, Some(bytelen)) => Some(bytelen * 2), | ||
| (AlgoKind::Blake2b, Some(bytelen)) => Some(bytelen), |
There was a problem hiding this comment.
Same here (plus for consistency reasons, as one line above algo_byte_len is used).
| (AlgoKind::Blake2b, Some(bytelen)) => Some(bytelen), | |
| (AlgoKind::Blake2b, Some(byte_len)) => Some(byte_len), |
This commit differentiates Base64 strings that are known to be invalid before decoding (because their length is not a multiple of 4), from Base64 strings that are invalid at decoding (padding is invalid).
RenjiSann
force-pushed
the
cksum-fix-cksum-c
branch
from
3279939 to
ba5ded0
Compare
|
GNU testsuite comparison: |
| match base64_simd::forgiving_decode_to_vec(checksum.as_bytes()) { | ||
| Ok(buffer) if checks_hint(buffer.len()) => Some(hex::encode(buffer).into()), | ||
| // The resulting length is not as expected | ||
| Ok(_) => None, | ||
| Err(_) => None, | ||
| } |
There was a problem hiding this comment.
A functional approach might be cleaner, though I'm aware it's subjective. Feel free to ignore this comment ;-)
base64_simd::forgiving_decode_to_vec(checksum.as_bytes())
.ok()
.and_then(|buffer| {
if checks_hint(buffer.len()) {
Some(hex::encode(buffer).into())
} else {
None
}
})There was a problem hiding this comment.
Yes, I agree with you. However, I'm changing this in the following MR #9532, so I will keep it like so on this MR and avoid a funny rebase :)
Kudos :) |
2 participants
The latest GNU release added tests to
cksum-cwhich made our implementation fail.Namely, the testcase that was problematic was the following:
This command used to produce an "improperly formatted line" error, but is now required to fail at the string digest comparison.
The error here is that the digest is an invalid BASE64 string: although it is the right length, the last
+breaks the padding, so our code previously failed to decode it.This MR makes the difference between Base64 strings that are known to be invalid before decoding, which should be considered as improperly formatted lines, and those that are only recognized to be invalid after decoding, for padding issues, which are to be regarded as properly formatted compared to the obtained checksum.