Skip to content

docs: clarify when filter regexes must be quoted - #8346

Open
citizen204 wants to merge 2 commits into
mitmproxy:mainfrom
citizen204:fix-7715-doc-unquoted-regex-parens
Open

docs: clarify when filter regexes must be quoted#8346
citizen204 wants to merge 2 commits into
mitmproxy:mainfrom
citizen204:fix-7715-doc-unquoted-regex-parens

Conversation

@citizen204

Copy link
Copy Markdown

#7715 reports that an unquoted regex containing grouping parentheses, e.g. ~u get(Info|Routers), is silently misparsed: ( and ) are reserved characters used by the boolean-grouping grammar in mitmproxy/flowfilter.py (unicode_words = pp.CharsNotIn("()~'\"" + DEFAULT_WHITE_CHARS)), so the filter actually becomes ~u get & ~u "Info|Routers" rather than a single regex match. I confirmed this by parsing several expressions directly with mitmproxy.flowfilter.parse():

  • ~u get(Info|Routers) -> url matches /get/i and url matches /Info|Routers/i (broken)
  • ~u "get(Info|Routers)" -> url matches /get(Info|Routers)/i (correct, quoted)
  • ~u get|other / ~u get&other / ~u get!other all parse fine unquoted, since |, &, ! are only treated as operators when surrounded by whitespace.
  • ~u foo~bar and ~u it's also fail unquoted, since ~ and ' are likewise reserved.

Redesigning the grammar to disambiguate regex-parens from grouping-parens is a real design trade-off a maintainer should own, so rather than touch the parser this PR only fixes the documentation, which currently states "Regexes can be specified as quoted strings" without mentioning that some regexes must be quoted to parse correctly - contradicting user expectations as described in the issue.

Fixes #7715

Changes

  • docs/src/content/concepts/filters.md: document that unquoted regexes must avoid parentheses, whitespace, and the ~, ', " characters, and must be quoted otherwise (with a corrected example using the exact case from the issue)

Comment thread mitmproxy/certs.py
Comment on lines 730 to 745

def load_pem_private_key(data: bytes, password: bytes | None) -> rsa.RSAPrivateKey:
"""
like cryptography's load_pem_private_key, but silently falls back to not using a password
like cryptography's load_pem_private_key, but falls back to not using a password
if the private key is unencrypted.
"""
try:
return serialization.load_pem_private_key(data, password) # type: ignore
except TypeError:
if password is not None:
logger.warning(
"A password was configured for a private key that is not encrypted. "
"Ignoring the password and loading the key without one."
)
return load_pem_private_key(data, None)
raise

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove this unrelated stuff please

assert cert.crl_distribution_points == crls


class TestLoadPemPrivateKey:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same

Comment on lines +17 to +20
- Unquoted regexes must not contain parentheses, whitespace, or the `~`, `'`, `"`
characters, as these are used for grouping and quoting. A regex containing any
of these, e.g. one using `(...)` for grouping, must be wrapped in quotes -
`~u "get(Info|Routers)"` - or it will be parsed as multiple filter expressions.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- Unquoted regexes must not contain parentheses, whitespace, or the `~`, `'`, `"`
characters, as these are used for grouping and quoting. A regex containing any
of these, e.g. one using `(...)` for grouping, must be wrapped in quotes -
`~u "get(Info|Routers)"` - or it will be parsed as multiple filter expressions.
- Regexes containing parentheses, whitespace, or the `~`, `'`, `"` characters must be quoted because these characters are reserved by the filter expression syntax. Otherwise, the expression may be parsed differently or rejected. For example, use ~u "get(Info|Routers)".

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Broken unquoted regular expressions

2 participants