Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

printf: accept non-UTF-8 input in FORMAT and ARGUMENT arguments#8329

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.

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

Already on GitHub?Sign in to your account

Merged
RenjiSann merged 7 commits intouutils:mainfromdrinkcat:printf-7209-update
Jul 15, 2025

Conversation

drinkcat
Copy link
Contributor

@drinkcatdrinkcat commentedJul 11, 2025•
edited
Loading

Rebased#7209 yet again (I "had" to squash 2 commits together to make the rebase slightly easier..), I'll also attempt to apply the changes I suggested.

--

Rebases#6812 on#7208.

EDIT: Now also includes a commit from me to pass the printf-mb GNU test, as well as a few other odds and ends.Fixes#6804.

@github-actionsGitHub Actions
Copy link

GNU testsuite comparison:

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

@github-actionsGitHub Actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/printf/printf-mb is no longer failing!

@drinkcatdrinkcat marked this pull request as ready for reviewJuly 11, 2025 11:00
@drinkcat
Copy link
ContributorAuthor

@jtracey if you have time to review this, both the rebase and my added changes, it'd be absolutely awesome...

I ended up modifyingExtendedParserError::PartialMatch to actually own a String (instead of a&str), as we need to store that inparse_quote_start. Makes the diff blow up a bit sadly.

@sylvestre
Copy link
Contributor

It looks great to me!

@github-actionsGitHub Actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)Congrats! The gnu test tests/printf/printf-mb is no longer failing!

@github-actionsGitHub Actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)Congrats! The gnu test tests/printf/printf-mb is no longer failing!

@jtracey
Copy link
Contributor

Lgtm! Thanks for picking this up. FWIW, the first commit should probably have multiple authors, but if this gets squashed GitHub might handle it on its own. If not, it's not the end of the world if the commit history is a little messy.

@drinkcat
Copy link
ContributorAuthor

Lgtm! Thanks for picking this up. FWIW, the first commit should probably have multiple authors, but if this gets squashed GitHub might handle it on its own. If not, it's not the end of the world if the commit history is a little messy.

Oh, didn't know the logic (I think native git only allows a single author?), I did keep a "Author:" line in the first squashed commit, but github didn't pick it up. Looks like adding aCo-authored-by: tag in the trailer of that squashed commit did the trick (you now appear as an author).

Also I'll... rebase to fix the conflict.

@drinkcat
Copy link
ContributorAuthor

Rebased... Hopefully all tests still pass.

@github-actionsGitHub Actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)Congrats! The gnu test tests/printf/printf-mb is no longer failing!

@sylvestresylvestre self-requested a reviewJuly 15, 2025 06:09
@sylvestre
Copy link
Contributor

Interesting, github does not allow me to merge it

@drinkcat
Copy link
ContributorAuthor

Weird... Not sure if I can do anything to help... Looks like it's waiting for a review by you? But... "It is not required to merge." it says...

andrewliebenowand others added7 commitsJuly 16, 2025 00:28
Other implementations of `printf` permit arbitrary data to be passedto `printf`. The only restriction is that a null byte terminatesFORMAT and ARGUMENT argument strings (since they are C strings).The current implementation only accepts FORMAT and ARGUMENTarguments that are valid UTF-8 (this is being enforced by clap).This commit removes the UTF-8 validation by switching to OsStrand OsString.This allows users to use `printf` to transmit or reformat null-safebut not UTF-8-safe data, such as text encoded in an 8-bit textencoding. See the `non_utf_8_input` test for an example (ISO-8859-1text).[drinkcat: also squashed in this commit to ease rebase]Author: Justin Tracey <jdt.dev@unsuspicious.click>uucore, printf: improve non-UTF-8 format argumentsThis fixes handling of format arguments, in part by eliminating duplicateimplementations. Utilities with format arguments other than printf will nolonger accept things like "'a" as numbers, etc.Co-authored-by: Justin Tracey <jdt.dev@unsuspicious.click>
And fix all the callers, e.g. all the next_ functions.
In one case, we'll need an actual owned String in PartialMatch,so it's easier to just use that. Removes a bunch of lifetime thingsin the code too.
After this, we can use a common extract_value function.
Suggested by Gemini and I think that's not a bad idea.
@RenjiSannRenjiSann merged commit1bb7930 intouutils:mainJul 15, 2025
25 checks passed
@github-actionsGitHub Actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)Congrats! The gnu test tests/printf/printf-mb is no longer failing!

@sylvestre
Copy link
Contributor

well done folks

jtracey reacted with hooray emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@sylvestresylvestreAwaiting requested review from sylvestre

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

Successfully merging this pull request may close these issues.

echo, printf: UTF-8 sensitivity in arguments
5 participants
@drinkcat@sylvestre@jtracey@RenjiSann@andrewliebenow
[8]ページ先頭
©2009-2025 Movatter.jp