Skip to content

Keep regex escapes intact in case-insensitive DeepSearch - #632

Open
x0Lazarus wants to merge 1 commit into
qlustered:devfrom
x0Lazarus:fix/preserve-search-regex-case
Open

x0Lazarus wants to merge 1 commit into
qlustered:devfrom
x0Lazarus:fix/preserve-search-regex-case

Conversation

@x0Lazarus

Copy link
Copy Markdown

DeepSearch lowercases a regular expression before using it for a case-insensitive search. That changes the meaning of escapes: searching for \S+ (non-whitespace) instead searches for \s+ (whitespace), and \D+ becomes a digit search.

Use re.IGNORECASE instead, keeping both the pattern and the searched text intact. This also preserves Unicode escapes and lets groups such as (?-i:ABC) keep their explicitly case-sensitive behavior. Literal searches and the flags of precompiled patterns retain their existing behavior. This addresses the problem also described in the closed PR #631.

The search tests pass all 78 cases, including regressions for escaped characters, dictionary paths, scoped flags, and compiled patterns. Pyright reports no errors for the changed module. On Windows with Python 3.12, the broader run passes 1,319 tests with 8 skips and one failure in test_delta_dump_and_read3; that same text-mode pickle failure occurs on unchanged dev. The security test module and two restricted-pickle tests were excluded from this local run. Other operating systems and Python versions have not been tested locally.

@seperman seperman left a comment

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.

Static review: #632 Keep regex escapes intact in case-insensitive DeepSearch

Summary: This changes use_regexp=True searches with a string pattern when case_sensitive=False, which is the default. The pattern and the searched text are no longer lowercased. The pattern is compiled with re.IGNORECASE instead. This fixes the reported problem: uppercase escapes were inverted (\S→\s, \D→\d, \W→\w) and \N{...}/\U... escapes were corrupted.

Overlap with other PRs

  • The production change is functionally identical to #631, "Preserve regex syntax in case-insensitive DeepSearch". #631 was closed unmerged by its author on 2026-09-23, with no comments.
    • The only differences are how the flags expression is formatted and the tests.
    • #631's tests also covered \B and ran the syntax cases with case_sensitive=True as well.
  • #638 (open) adds an exclusion check just before the new_parent_cased line this PR changes (deepdiff/search.py:240). git merge-tree reports a textual conflict between the two. Resolving it means keeping both changes.
  • Nothing equivalent is on dev.

Security

No security concerns found in static review.

  • The user-supplied pattern is still compiled once, as before. Only the way case-insensitivity is applied has changed: re.IGNORECASE instead of lowercasing.
  • This adds no regex-injection path and does not meaningfully change backtracking.
  • Compiling a caller-supplied pattern, with whatever ReDoS exposure that brings, is existing behavior of use_regexp=True.

Correctness

  • deepdiff/search.py:109,130-131: string patterns are no longer lowercased. When self.case_sensitive is false they are compiled with re.IGNORECASE.
    • For a precompiled pattern, line 108 forces self.case_sensitive to True, so flags is 0 and re.compile(pattern, flags=0) returns the pattern unchanged. This avoids the ValueError that re.compile raises when non-zero flags are passed with a compiled pattern. User-supplied flags are kept.
    • Numbers converted to strings under strict_checking=False also get flags=0, as before.
  • deepdiff/search.py:240 and :286-289: when use_regexp is set, the dictionary path and the string value are no longer lowercased. This is consistent with compiling with IGNORECASE. Literal (non-regex) searches keep their .lower() handling.
  • Other patterns this also fixes, which the description doesn't list and the tests don't cover. These follow from the old item.lower():
    • \A became \a (BEL).
    • \B became \b.
    • \Z became \z, which is a bad escape before Python 3.14.
    • Named groups: (?P<name>...) and (?P=name) became (?p<..., which re rejects. So on dev, a default (case-insensitive) regex search with a named group raises re.error.
  • Side effect in __search_numbers (deepdiff/search.py:304, used only with strict_checking=False): str(obj) was never lowercased there. On dev, an uppercase pattern such as 'E' therefore failed to match Decimal('1E+5'). With IGNORECASE it is now truly case-insensitive. This looks like an improvement, but it is a change in behavior.
  • Behavior change worth a changelog note: scoped flags such as (?-i:ABC) are now honored. On dev they were lowercased together with the text, so they matched case-insensitively.

Tests

  • test_case_insensitive_regex_preserves_syntax (tests/test_search.py:382) and test_case_insensitive_regex_preserves_dictionary_paths (:397) cover the bug. Following dev's code, these cases should fail on dev and pass with this change:
    • \S+, \W+, \D+, \N{...}, \U0001F680 and (?-i:...)
    • the \D+ and (?-i:Name) path cases
  • :387, :403 and :412 are guard tests for case sensitivity, compiled-pattern flags and literal search. They should pass both before and after the change, which is fine.

Suggestions

  • Consider adding cases for named groups ((?P<name>...)), which raise on dev, and for the anchors \A, \Z and \B.
  • A short changelog note that (?-i:...) is now respected in case-insensitive searches may help users.
  • If #638 lands first, this will need a small rebase around deepdiff/search.py:238-241.

AI-assisted static review; the code was not executed.

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.

2 participants