Skip to content

gh-70312: Add note for shlex.quote() #13333

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
Closed
wants to merge 3 commits into from
Closed

gh-70312: Add note for shlex.quote() #13333

wants to merge 3 commits into from

Conversation

Windsooon
Copy link
Contributor

@Windsooon Windsooon commented May 15, 2019 •
edited by AlexWaygood
Loading

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels May 15, 2019
@CharlesDaffern CharlesDaffern mannequin mentioned this pull request Jun 7, 2022
Open
slateny
slateny reviewed Oct 22, 2022
View reviewed changes
Doc/library/shlex.rst
Comment on lines +107 to +108
The :func:`quote` function will not quote shell keywords.
(like echo, done, while, etc.)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that saying it doesn't quote shell keywords makes one wonder why shell keywords specifically. It might be better to say something like this instead (source), but I'll defer to others if this is too much detail:

Suggested change
The :func:`quote` function will not quote shell keywords.
(like echo, done, while, etc.)
The :func:`quote` function will not quote any ASCII word characters, which
include shell keywords like ``echo``, ``done``, ``while``, etc.

@serhiy-storchaka
Copy link
Member

If add such note (I am not sure that it should be added at all), I think that the warning few paragraphs above is a better place.

@AlexWaygood AlexWaygood changed the title bpo-26124: Add note for shlex.quote() gh-70312: Add note for shlex.quote() Oct 30, 2022
@erlend-aasland
Copy link
Contributor

If add such note (I am not sure that it should be added at all), I think that the warning few paragraphs above is a better place.

I think Murray's suggestion in #70312 (comment) is a better approach. Suggesting to close this PR and create a new one based on that suggestion.

@erlend-aasland erlend-aasland added the pending The issue will be closed if no feedback is provided label Feb 9, 2024
@erlend-aasland erlend-aasland marked this pull request as draft February 9, 2024 11:38
@willingc
Copy link
Contributor

Thanks @Windsooon and reviewers. I'm going to go ahead and close this stale PR that has been marked pending since February.

The existing examples illustrate the advantage of using quote to avoid injection.

@willingc willingc closed this Oct 31, 2024
@AA-Turner AA-Turner removed the pending The issue will be closed if no feedback is provided label Apr 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slateny slateny slateny left review comments

@auvipy auvipy auvipy approved these changes

@bitdancer bitdancer Awaiting requested review from bitdancer

@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka

Assignees
No one assigned
Labels
docs Documentation in the Doc dir skip news
Projects
Docs PRs
Status: Todo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.