cksum: even more fixes by RenjiSann · Pull Request #6929 · uutils/coreutils

RenjiSann · 2024-12-06T19:10:40Z

This PR changes the way the regex to parse a checksum line is chosen. Now, every line gets to choose the regex it matches with, instead of trying to match with the regex that matched the first line (except for non-algo based line which should follow the flavor of the first one).

While doing this, I've decided to get rid of the regex::Capture which was being carried around everywhere, and create a LineInfo struct that holds all the parsed data.

It adds lazy_static as a direct dependency to uu_core (it was an indirect dependency until now). I allows to not recompile the regexes again and again.

EDIT: I have added some code to rework the way the expected checksum is decoded, this PR now fixes #6576 and #6614 as well.

github-actions · 2024-12-06T19:45:20Z

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

sylvestre · 2024-12-06T20:17:05Z

a few jobs are failing

github-actions · 2024-12-06T22:22:43Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

github-actions · 2024-12-06T23:26:10Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

github-actions · 2024-12-07T00:46:11Z

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

RenjiSann · 2024-12-07T01:44:27Z

a few jobs are failing

I've fixed what I could.
There's still the anstream issue which is not fixed yet, and some "license_not_encountered" stuff which I don't think are my fault

github-actions · 2024-12-07T02:10:26Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

src/uucore/Cargo.toml

src/uucore/src/lib/features/checksum.rs

github-actions · 2024-12-10T09:24:28Z

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

…exes

…ate functions

github-actions · 2024-12-10T12:35:06Z

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

src/uucore/src/lib/features/checksum.rs

cakebaker · 2024-12-11T13:46:15Z

src/uucore/src/lib/features/checksum.rs

+    let (filename_to_check_unescaped, prefix) = unescape_filename(filename);
+    let real_filename_to_check = os_str_from_bytes(&filename_to_check_unescaped)?;


I suggest to rename filename_to_check_unescaped to unescaped_filename_to_check to match the naming of real_filename_to_check. And I think the _to_check suffices can be dropped in both cases.

src/uucore/src/lib/features/checksum.rs

cakebaker

Overall good job with this refactoring!

src/uucore/src/lib/features/checksum.rs

…of the expected digest

…ore and introduce LineFormat struct

cakebaker · 2024-12-12T09:16:57Z

Thanks :)

sylvestre · 2024-12-31T15:54:55Z

@RenjiSann btw, you will see that the fuzzer finds some issues:
https://github.com/uutils/coreutils/actions/runs/12560388614/job/35017702985

to run it locally:
cargo +nightly fuzz run fuzz_cksum -- -max_total_time=60 -detect_leaks=0

RenjiSann · 2025-01-02T00:21:03Z

@RenjiSann btw, you will see that the fuzzer finds some issues: https://github.com/uutils/coreutils/actions/runs/12560388614/job/35017702985

to run it locally: cargo +nightly fuzz run fuzz_cksum -- -max_total_time=60 -detect_leaks=0

Indeed. Many tests fail because the cksum installed in the CI is older than ours (probably GNU coreutils 9.1 which is Ubuntu's default), which does not support --base64 and --raw.

Another issue is that --length validity should be checked before the --check argument.
I will try to identify other problems and open issues for them.

RenjiSann force-pushed the cksum-fixes branch from 6bbbb87 to 31de97a Compare December 6, 2024 19:18

checksum: move regex detection to the line level

5cbe876

RenjiSann force-pushed the cksum-fixes branch 2 times, most recently from 42997a2 to f36fe46 Compare December 6, 2024 21:57

RenjiSann force-pushed the cksum-fixes branch from f36fe46 to 219a91d Compare December 6, 2024 23:00

RenjiSann force-pushed the cksum-fixes branch from 219a91d to 00d1730 Compare December 7, 2024 00:20

RenjiSann force-pushed the cksum-fixes branch 2 times, most recently from eda6659 to f836b08 Compare December 7, 2024 01:43

cakebaker reviewed Dec 7, 2024

View reviewed changes

src/uucore/Cargo.toml Show resolved Hide resolved

cakebaker reviewed Dec 9, 2024

View reviewed changes

src/uucore/src/lib/features/checksum.rs Outdated Show resolved Hide resolved

cakebaker reviewed Dec 9, 2024

View reviewed changes

src/uucore/src/lib/features/checksum.rs Outdated Show resolved Hide resolved

cakebaker reviewed Dec 9, 2024

View reviewed changes

src/uucore/src/lib/features/checksum.rs Outdated Show resolved Hide resolved

cakebaker reviewed Dec 9, 2024

View reviewed changes

src/uucore/src/lib/features/checksum.rs Outdated Show resolved Hide resolved

RenjiSann force-pushed the cksum-fixes branch 2 times, most recently from 5f22dad to 5e92491 Compare December 10, 2024 08:58

RenjiSann added 4 commits December 10, 2024 13:09

checksum: keep a cache of the first used regex for non-algo-based reg…

ed15ca1

…exes

checksum: avoid to recompute Regexps

65ddccb

test(cksum): Add tests

df16c1c

checksum: split treatment of algo-based and non-algo based into separ…

10a9b0b

…ate functions

RenjiSann force-pushed the cksum-fixes branch from 5e92491 to 6873fe2 Compare December 10, 2024 12:09

cakebaker reviewed Dec 10, 2024

View reviewed changes

src/uucore/src/lib/features/checksum.rs Show resolved Hide resolved

cakebaker reviewed Dec 11, 2024

View reviewed changes

src/uucore/src/lib/features/checksum.rs Outdated Show resolved Hide resolved

cakebaker reviewed Dec 11, 2024

View reviewed changes

src/uucore/src/lib/features/checksum.rs Outdated Show resolved Hide resolved

cakebaker reviewed Dec 11, 2024

View reviewed changes

src/uucore/src/lib/features/checksum.rs Outdated Show resolved Hide resolved

cakebaker reviewed Dec 11, 2024

View reviewed changes

src/uucore/src/lib/features/checksum.rs Outdated Show resolved Hide resolved

cakebaker reviewed Dec 11, 2024

View reviewed changes

src/uucore/src/lib/features/checksum.rs Outdated Show resolved Hide resolved

cakebaker reviewed Dec 11, 2024

View reviewed changes

RenjiSann force-pushed the cksum-fixes branch 4 times, most recently from c410c28 to e7fa8dd Compare December 11, 2024 23:28

cakebaker reviewed Dec 12, 2024

View reviewed changes

src/uucore/src/lib/features/checksum.rs Outdated Show resolved Hide resolved

cakebaker reviewed Dec 12, 2024

View reviewed changes

src/uucore/src/lib/features/checksum.rs Outdated Show resolved Hide resolved

RenjiSann added 3 commits December 12, 2024 09:49

checksum: use the blake2b length as an hint to check the correctness …

f4e5dc2

…of the expected digest

checksum: remove ALGO_BASED_REGEX (non base64) as its not useful anym…

567bbc5

…ore and introduce LineFormat struct

test(cksum): un-ignore tests that are now implemented

958222a

RenjiSann force-pushed the cksum-fixes branch from e7fa8dd to 958222a Compare December 12, 2024 08:49

cakebaker merged commit 209ec0b into uutils:main Dec 12, 2024

cakebaker mentioned this pull request Dec 12, 2024

cksum: Detect algorithm/digest size mismatch #6614

Closed

moonfruit mentioned this pull request Jan 20, 2025

uutils-selected 0.0.29 moonfruit/homebrew-tap#200

Closed

RenjiSann deleted the cksum-fixes branch January 29, 2025 15:22

RenjiSann mentioned this pull request Feb 3, 2025

cksum: gets confused by base64 that happens to consist entirely of hexadecimal digits #6572

Closed

		let (filename_to_check_unescaped, prefix) = unescape_filename(filename);
		let real_filename_to_check = os_str_from_bytes(&filename_to_check_unescaped)?;

Uh oh!

Conversation

RenjiSann commented Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 6, 2024

Uh oh!

sylvestre commented Dec 6, 2024

Uh oh!

github-actions bot commented Dec 6, 2024

Uh oh!

github-actions bot commented Dec 6, 2024

Uh oh!

github-actions bot commented Dec 7, 2024

Uh oh!

RenjiSann commented Dec 7, 2024

Uh oh!

github-actions bot commented Dec 7, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 10, 2024

Uh oh!

github-actions bot commented Dec 10, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cakebaker Dec 11, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cakebaker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cakebaker commented Dec 12, 2024

Uh oh!

sylvestre commented Dec 31, 2024

Uh oh!

RenjiSann commented Jan 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

RenjiSann commented Dec 6, 2024 •

edited

Loading