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

Rule W605 cause panic #10434

Closed
qarmin opened this issue Mar 17, 2024 · 4 comments · Fixed by #10480
Closed

Rule W605 cause panic #10434

qarmin opened this issue Mar 17, 2024 · 4 comments · Fixed by #10480
Assignees
Labels
bug Something isn't working fuzzer Surfaced via fuzzing. help wanted Contributions especially welcome

Comments

@qarmin
Copy link

qarmin commented Mar 17, 2024

ruff 0.3.3
(latest changes from main branch)

ruff  *.py --select W605 --no-cache --fix --unsafe-fixes --preview --output-format concise --isolated

file content:

cmd = f"""
GRANT ALL PRIVILEGES ON SCHEMA api TO {kwollect_user};
    req += "\\n" \
  params      JSONB             DEFAULT '{{}}'
    CASE WHEN metrics.device_id ~ '^[a-z�+-\d+$' THEN '1' ELSE '2' END AS idx1,
"""

error

All checks passed!

error: Panicked while linting /opt/tmp_folder/11322743460824035852.py: This indicates a bug in Ruff. If you could open an issue at:

    https://github.com/astral-sh/ruff/issues/new?title=%5BLinter%20panic%5D

...with the relevant file contents, the `pyproject.toml` settings, and the following stack trace, we'd be very appreciative!

panicked at /home/runner/work/Automated-Fuzzer/Automated-Fuzzer/ruff/crates/ruff_source_file/src/locator.rs:463:23:
byte index 173 is not a char boundary; it is inside '\u{9f}' (bytes 172..174) of `cmd = f"""
GRANT ALL PRIVILEGES ON SCHEMA api TO {kwollect_user};
    req += "\\n" \
  params      JSONB             DEFAULT '{{}}'
    CASE WHEN metrics.device_id ~ '^[a-z�\+\\-\d+$' THEN '1' ELSE '2' END AS idx1,
"""`
Backtrace:    0: ruff::panic::catch_unwind::{{closure}}
             at ./ruff/crates/ruff/src/panic.rs:31:25
   1: <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/alloc/src/boxed.rs:2029:9
   2: std::panicking::rust_panic_with_hook
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:783:13
   3: std::panicking::begin_panic_handler::{{closure}}
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:657:13
   4: std::sys_common::backtrace::__rust_end_short_backtrace
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:171:18
   5: rust_begin_unwind
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
   6: core::panicking::panic_fmt
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
   7: core::str::slice_error_fail_rt
   8: core::str::slice_error_fail
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/str/mod.rs:88:9
   9: core::str::traits::<impl core::slice::index::SliceIndex<str> for core::ops::range::Range<usize>>::index
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/str/traits.rs:231:21
  10: core::str::traits::<impl core::ops::index::Index<I> for str>::index
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/str/traits.rs:61:15
  11: ruff_text_size::range::<impl core::ops::index::Index<ruff_text_size::range::TextRange> for str>::index
             at ./ruff/crates/ruff_text_size/src/range.rs:430:14
  12: ruff_source_file::locator::Locator::slice
             at ./ruff/crates/ruff_source_file/src/locator.rs:463:23
  13: ruff_linter::fix::apply_fixes
             at ./ruff/crates/ruff_linter/src/fix/mod.rs:98:25
  14: ruff_linter::fix::fix_file
             at ./ruff/crates/ruff_linter/src/fix/mod.rs:48:14
  15: ruff_linter::linter::lint_fix
             at ./ruff/crates/ruff_linter/src/linter.rs:580:14
  16: ruff::diagnostics::lint_path
             at ./ruff/crates/ruff/src/diagnostics.rs:280:14
  17: ruff::commands::check::lint_path::{{closure}}
             at ./ruff/crates/ruff/src/commands/check.rs:194:9
  18: std::panicking::try::do_call
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:552:40
  19: std::panicking::try
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:516:19
  20: std::panic::catch_unwind
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panic.rs:142:14
  21: ruff::panic::catch_unwind
             at ./ruff/crates/ruff/src/panic.rs:40:18
  22: ruff::commands::check::lint_path
             at ./ruff/crates/ruff/src/commands/check.rs:193:18
  23: ruff::commands::check::check::{{closure}}
             at ./ruff/crates/ruff/src/commands/check.rs:94:17
  24: <rayon::iter::filter_map::FilterMapFolder<C,P> as rayon::iter::plumbing::Folder<T>>::consume
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/iter/filter_map.rs:123:36
  25: rayon::iter::plumbing::Folder::consume_iter
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/iter/plumbing/mod.rs:179:20
  26: rayon::iter::plumbing::Producer::fold_with
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/iter/plumbing/mod.rs:110:9
  27: rayon::iter::plumbing::bridge_producer_consumer::helper
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/iter/plumbing/mod.rs:438:13
  28: rayon::iter::plumbing::bridge_producer_consumer
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/iter/plumbing/mod.rs:397:12
  29: <rayon::iter::plumbing::bridge::Callback<C> as rayon::iter::plumbing::ProducerCallback<I>>::callback
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/iter/plumbing/mod.rs:373:13
  30: <rayon::slice::Iter<T> as rayon::iter::IndexedParallelIterator>::with_producer
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/slice/mod.rs:777:9
  31: rayon::iter::plumbing::bridge
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/iter/plumbing/mod.rs:357:12
  32: <rayon::slice::Iter<T> as rayon::iter::ParallelIterator>::drive_unindexed
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/slice/mod.rs:753:9
  33: <rayon::iter::filter_map::FilterMap<I,P> as rayon::iter::ParallelIterator>::drive_unindexed
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/iter/filter_map.rs:46:9
  34: <rayon::iter::fold::Fold<I,ID,F> as rayon::iter::ParallelIterator>::drive_unindexed
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/iter/fold.rs:59:9
  35: rayon::iter::reduce::reduce
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/iter/reduce.rs:15:5
  36: rayon::iter::ParallelIterator::reduce
             at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rayon-1.9.0/src/iter/mod.rs:998:9
  37: ruff::commands::check::check
             at ./ruff/crates/ruff/src/commands/check.rs:165:10
  38: ruff::check
             at ./ruff/crates/ruff/src/lib.rs:409:13
  39: ruff::run
             at ./ruff/crates/ruff/src/lib.rs:191:33
  40: ruff::main
             at ./ruff/crates/ruff/src/main.rs:65:11
  41: core::ops::function::FnOnce::call_once
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/ops/function.rs:250:5
  42: std::sys_common::backtrace::__rust_begin_short_backtrace
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/sys_common/backtrace.rs:155:18
  43: std::rt::lang_start::{{closure}}
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/rt.rs:166:18
  44: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/ops/function.rs:284:13
  45: std::panicking::try::do_call
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:552:40
  46: std::panicking::try
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:516:19
  47: std::panic::catch_unwind
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panic.rs:142:14
  48: std::rt::lang_start_internal::{{closure}}
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/rt.rs:148:48
  49: std::panicking::try::do_call
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:552:40
  50: std::panicking::try
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:516:19
  51: std::panic::catch_unwind
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panic.rs:142:14
  52: std::rt::lang_start_internal
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/rt.rs:148:20
  53: main
  54: <unknown>
  55: __libc_start_main
  56: _start

python_compressed.zip

@charliermarsh charliermarsh added bug Something isn't working fuzzer Surfaced via fuzzing. help wanted Contributions especially welcome labels Mar 17, 2024
@boolean-light
Copy link
Contributor

Probably this is due to {{}} being normalized to {} in lex_fstring_middle_or_end which is inside crates/ruff_python_parser/src/lexer.rs, which causes the range defined in crates/ruff_linter/src/rules/pycodestyle/rules/invalid_escape_sequence.rs:175 to deviate from the actual range in the content.
This seems to put a wrong diagnostic.

I'm now thinking of adding new token which matches only {{ inside f-string, but will there be better options?

@MichaReiser
Copy link
Member

@dhruvmanila any chance this is fixed in the new parser. I'm asking because you made some changes to string lexing/parsing

@dhruvmanila
Copy link
Member

No, this isn't fixed in the new parser. Although I might know why the problem is occurring. I'll need to look at the W605 rule and string parsing.

A minimal way to reproduce this is using the following code:

# The curly brace escape is important
f"{{}}+-\d"

And running it using:

$ cargo run -- check --select W605 --no-cache --preview --output-format full --isolated ~/playground/ruff/src/play.py
/Users/dhruv/playground/ruff/src/play.py:1:7: W605 [*] Invalid escape sequence: `\d`
  |
1 | f"{{}}+-\d"
  |       ^^ W605
  |
  = help: Use a raw string literal

Found 1 error.
[*] 1 fixable with the `--fix` option.

As you can see, the highlighted range is incorrect.

This creates other errors like for example if there's a newline in the string, which is a valid escape sequence, the fix can't be to prefix the f-string with r but instead to escape the invalid sequence. This leads to an invalid fix:

$ cargo run -- check --select W605 --fix --diff --no-cache --preview --output-format full --isolated ~/playground/ruff/src/play.py
--- /Users/dhruv/playground/ruff/src/play.py
+++ /Users/dhruv/playground/ruff/src/play.py
@@ -1 +1 @@
-f"\n{{}}+-\d+"
+f"\n{{}\}\+\\\-\d+"

Would fix 5 errors.

And, when you include the weird character in the mix, you get the panic:

f"\n{{}}�+-\d+"

And, we can even reproduce it in a way that the fix never converges.

@dhruvmanila
Copy link
Member

I can look at it tonight.

@dhruvmanila dhruvmanila self-assigned this Mar 19, 2024
dhruvmanila added a commit that referenced this issue Mar 19, 2024
## Summary

This PR fixes a panic in the linter for `W605`.

Consider the following f-string:
```python
f"{{}}ab"
```

The `FStringMiddle` token would contain `{}ab`. Notice that the escaped
braces have _reduced_ the string. This means we cannot use the text
value from the token to determine the location of the escape sequence
but need to extract it from the source code.

fixes: #10434 

## Test Plan

Add new test cases and update the snapshots.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fuzzer Surfaced via fuzzing. help wanted Contributions especially welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants