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.

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

-W option and PYTHONWARNINGS env variable does not accept module regexes #78805

Closed
coldfix mannequin opened this issue Sep 10, 2018 · 16 comments
Closed

-W option and PYTHONWARNINGS env variable does not accept module regexes #78805

coldfix mannequin opened this issue Sep 10, 2018 · 16 comments
Labels
3.11 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@coldfix
Copy link
Mannequin

coldfix mannequin commented Sep 10, 2018

BPO 34624
Nosy @ncoghlan, @vstinner, @blueyed, @tiran, @nedbat, @mpaolini, @kernc, @tirkarthi, @coldfix, @kevinoid
PRs
  • bpo-34624: Allow regex for module passed via -W or PYTHONWARNINGS #9358
  • bpo-42272: fix misleading warning filter message/module docs #23172
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2021-12-20.13:04:35.484>
    created_at = <Date 2018-09-10.23:36:34.750>
    labels = ['type-feature', 'library', '3.11']
    title = '-W option and PYTHONWARNINGS env variable does not accept module regexes'
    updated_at = <Date 2022-03-12.09:00:18.988>
    user = 'https://github.com/coldfix'

    bugs.python.org fields:

    activity = <Date 2022-03-12.09:00:18.988>
    actor = 'kevinoid'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-12-20.13:04:35.484>
    closer = 'coldfix'
    components = ['Library (Lib)']
    creation = <Date 2018-09-10.23:36:34.750>
    creator = 'coldfix'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 34624
    keywords = ['patch', 'security_issue']
    message_count = 16.0
    messages = ['324959', '324960', '324997', '325538', '328648', '328650', '336646', '348202', '373358', '373536', '373669', '408305', '408496', '408498', '408499', '408959']
    nosy_count = 11.0
    nosy_names = ['ncoghlan', 'vstinner', 'blueyed', 'christian.heimes', 'nedbat', 'mpaolini', 'kernc', 'xtreak', 'coldfix', 'kevinoid', 'Yongjik Kim']
    pr_nums = ['9358', '23172']
    priority = 'normal'
    resolution = None
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue34624'
    versions = ['Python 3.11']

    @coldfix
    Copy link
    Mannequin Author

    coldfix mannequin commented Sep 10, 2018

    Hi,

    This command does not report a warning, while it should:

    python -c 'import warnings; warnings.warn("This should show up")' -Wi -W'default:::.*'

    If the regex .* is replaced by __main__ it works as expected.

    Same applies for regexes in PYTHONWARNINGS and for the message part of the argument.

    The reason can be found in Lib/warnings.py:144 (def _setoption):

      module = re.escape(module)

    This point-blank escape makes me think that it was intended that no regexes can be passed to message/module. On the other, the documentation reads as if it should be supported.

    Specifically, the -W option is documented in 1. While this page lists only basic examples, it refers to 2 and 3 for more details. 2 states that message/module are regexes. 3 seems to be written to specifically address the syntax of the PYTHONWARNINGS and the -W option and explicitly lists an example with a regex.

    I would welcome if we could remove re.escape to make the implementation fit the documentation, or are there any downsides to this?

    Best regards, Thomas

    @coldfix coldfix mannequin added 3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir labels Sep 10, 2018
    @coldfix
    Copy link
    Mannequin Author

    coldfix mannequin commented Sep 10, 2018

    Very sorry, the example command above should read:

    python -Wi -W'default:::.*' -c 'import warnings; warnings.warn("This should show up")'

    @tirkarthi
    Copy link
    Member

    The original escape was done with 2a862c6 which was in 2000. The doc example you are referring to at [1] was added with https://bugs.python.org/issue31975 and there doesn't seem to be a test involving regular expression as I can see from the PR. Adding Nick to the issue since he had committed the doc and might help here.

    [1] https://docs.python.org/3/library/warnings.html#describing-warning-filters

    Thanks

    @coldfix
    Copy link
    Mannequin Author

    coldfix mannequin commented Sep 17, 2018

    Thanks for your response. I have opened a PR at 1 that would remove the re.escape such that the implementation matches the documentation if you decide that this is fine.

    Personally, I would go even further and always add the (\..*)?$ suffix (instead of only $) to make it easier to match all modules in a package which is probably the most important use-case for the case of packages. What do you think?

    @nikratio nikratio mannequin changed the title -W option does not accept module regexes -W option and PYTHONWARNINGS env variable does not accept module regexes Oct 7, 2018
    @nedbat
    Copy link
    Member

    nedbat commented Oct 27, 2018

    Another option is to make clear in the docs that the command line does not accept regular expressions, but only literal module names, and raise an error if a non-importable name (for example with asterisks in it) is specified.

    And while we are here: the docs show "error:::mymodule[.*]" which doesn't even make sense as a regex!

    @coldfix
    Copy link
    Mannequin Author

    coldfix mannequin commented Oct 27, 2018

    Hi, thanks for the consideration!

    Is there any reason to introduce different behaviour than with filterwarnings here? This increases the number of things to remember - and except for the dot regex syntax doesn't interfere with module names, so there is no big drawback here either.

    More importantly, my main use case for filterwarnings would be to select/ignore warnings based on module *or package* name. With the current interpretation as exact module name, you have to list all submodules in advance, which is quite inhibiting.

    I have fixed the [.*] bogus example in the PR.

    Best, Thomas

    @ncoghlan
    Copy link
    Contributor

    I think the only reason I didn't mention this discrepancy in my doc updates is because I wasn't aware there *was* a discrepancy.

    The weird syntax was then just an incorrect amalgamation of "optional argument" notation with an improperly escaped regex suffix.

    @blueyed
    Copy link
    Mannequin

    blueyed mannequin commented Jul 19, 2019

    Note that "module" might also be the filename (with ".py" removed).

    I've just created bpo-37634 - might be worth including in your PR if it makes sense to only document this.

    @mpaolini
    Copy link
    Mannequin

    mpaolini mannequin commented Jul 8, 2020

    hello Thomas,

    do you need any help fixing the conflicts in your PR?

    even if Lib/warnings.py changed a little in the last 2 years, your PR is still good!

    @coldfix
    Copy link
    Mannequin Author

    coldfix mannequin commented Jul 11, 2020

    Hi, I have rebased this on master and fixed the minor conflict. Let me know if there is anything else I can do.

    Best, Thomas

    @coldfix coldfix mannequin added 3.9 only security fixes 3.10 only security fixes labels Jul 11, 2020
    @YongjikKim
    Copy link
    Mannequin

    YongjikKim mannequin commented Jul 15, 2020

    Hi, sorry if I'm interrupting, but while we're at this, could we also not escape regex for "message" part? (Or at least amend the documentation to clarify that the message part is literal string match?)

    Currently, the docs on -W just say "The meaning of each of these fields is as described in The Warnings Filter" and then "The Warnings Filter" section says that the "message" field is a regex, but currently it's only true if you run warnings.filterwarnings() directly, and not if you use the -W option.

    @tiran
    Copy link
    Member

    tiran commented Dec 11, 2021

    Adding regular expression support to -W and PYTHONWARNINGS env var turns the options into potential attack vectors. It can introduce REDOS vulnerability.

    @tiran tiran added 3.11 only security fixes type-feature A feature request or enhancement and removed 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes labels Dec 11, 2021
    @vstinner
    Copy link
    Member

    Oh, I didn't know this issue. I closed my issue bpo-43862 as a duplicate.

    @vstinner
    Copy link
    Member

    Adding regular expression support to -W and PYTHONWARNINGS env var turns the options into potential attack vectors.

    Why would an attacker control these options?

    If an attacker controls how Python is run, they are more efficient way to take control of Python and execute arbitrary code, than just trigger a denial of service, no

    @vstinner
    Copy link
    Member

    One option is to keep the current behavior by default, but support a new "/regex/" format. The /regex/ format is commonly used in Perl and sed.

    Example to ignore warnings containing "deprecated" string in their message:

    python3 -W "ignore:/deprecated/"
    PYTHONWARNINGS="ignore:/deprecated/"

    whereas the following commands continue to match exactly the whole warning message "deprecated":

    python3 -W "ignore:deprecated"
    PYTHONWARNINGS="ignore:deprecated"

    @coldfix
    Copy link
    Mannequin Author

    coldfix mannequin commented Dec 20, 2021

    Ok, it seems at least the incorrect documentation has been fixed in the mean time.

    I'm going to close this as there seems to be no capacity to deal with this.

    @coldfix coldfix mannequin closed this as completed Dec 20, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants