Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cksum/hashsum: Support for non-UTF-8 input in checksum files #6793

Merged
merged 6 commits into from
Oct 24, 2024
Merged

cksum/hashsum: Support for non-UTF-8 input in checksum files #6793

merged 6 commits into from
Oct 24, 2024

Conversation

RenjiSann
Copy link
Contributor

@RenjiSann RenjiSann commented Oct 18, 2024 •
edited
Loading

Second part of splitting #6603.

This PR adds support for non-UTF-8 checksum file content for cksum and hashsum validation tools.

In practice, given a CHECKSUM file with the following content, non-UTF-8 characters can appear in 2 places:

  • in the filename
  • in a comment
    All the other places are either algorithm name, which should be ASCII, or hexa/base64 digest, which should be ASCII as well.
$ cat CHECKSUM
SHA1 (filename-might-have-non-utf-8) = 058ab38dd3603703b3a7063cf95dc51a4286b6fe
# comment-might-have-non-utf-8

The most important step is to get rid of the call to BufRead::lines() to iterate on the lines of the CHECKSUM file, otherwise, it panics on non-UTF-8 characters.

This change makes heavy use of String/OsString/Vec<u8> conversion, which happen to not be equally treated on all platforms.

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/timeout/timeout is no longer failing!

@RenjiSann RenjiSann changed the title cksum/hashsum: Support for non-UTF-8 inputs cksum/hashsum: Support for non-UTF-8 input in checksum files Oct 18, 2024
@RenjiSann RenjiSann mentioned this pull request Oct 18, 2024
Closed
@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

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

@RenjiSann RenjiSann force-pushed the checksum-utf8 branch 3 times, most recently from 050ad32 to 91c1cd8 Compare October 19, 2024 15:45
@RenjiSann RenjiSann closed this Oct 19, 2024
@RenjiSann RenjiSann reopened this Oct 19, 2024
@RenjiSann
Copy link
Contributor Author

I closed the PR by mistake, wanted to mark it as ready :/

@RenjiSann RenjiSann marked this pull request as ready for review October 19, 2024 15:46
@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/timeout/timeout is no longer failing!

@RenjiSann RenjiSann force-pushed the checksum-utf8 branch 3 times, most recently from 3285e74 to 953fa37 Compare October 19, 2024 23:23
@RenjiSann
Copy link
Contributor Author

@cakebaker @BenWiederhake
PR is ready for review if one of you can take a look :)

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/tail/assert is no longer failing!
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

@cakebaker
Copy link
Contributor

@RenjiSann sorry, there are some conflicts :|

@RenjiSann
Copy link
Contributor Author

@RenjiSann sorry, there are some conflicts :|

Fixed ! 😁

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/misc/usage_vs_getopt is no longer failing!

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/misc/usage_vs_getopt is no longer failing!

@RenjiSann RenjiSann force-pushed the checksum-utf8 branch 3 times, most recently from 23d0c71 to ebe53e0 Compare October 23, 2024 15:05
@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/misc/usage_vs_getopt is no longer failing!
Congrats! The gnu test tests/timeout/timeout is no longer failing!

@RenjiSann RenjiSann force-pushed the checksum-utf8 branch 3 times, most recently from 178cc03 to 50bfa3d Compare October 23, 2024 21:35
@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/misc/usage_vs_getopt is no longer failing!
Congrats! The gnu test tests/timeout/timeout is no longer failing!

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/timeout/timeout is no longer failing!

@github-actions GitHub Actions
Copy link

GNU testsuite comparison:

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

@RenjiSann
Copy link
Contributor Author

Ready for what should be a final review :)

@cakebaker cakebaker merged commit cb8711f into uutils:main Oct 24, 2024
67 of 68 checks passed
@cakebaker
Copy link
Contributor

Thanks :)

@RenjiSann RenjiSann deleted the checksum-utf8 branch October 24, 2024 13:48
@RenjiSann RenjiSann mentioned this pull request Oct 25, 2024
Merged
@BrewTestBot BrewTestBot mentioned this pull request Nov 16, 2024
Merged
@RenjiSann RenjiSann mentioned this pull request Nov 17, 2024
Closed
@moonfruit moonfruit mentioned this pull request Nov 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sylvestre sylvestre sylvestre left review comments

@cakebaker cakebaker cakebaker left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RenjiSann @cakebaker @sylvestre