Skip to content

fix(robots): don't patch robotparser on Python 3.14+ - #2278

Merged
ntohidi merged 2 commits into
developfrom
fix/robots-py314-wildcard-patch
Sep 22, 2026
Merged

ntohidi merged 2 commits into
developfrom
fix/robots-py314-wildcard-patch

Conversation

@ntohidi

@ntohidi ntohidi commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #2229, which fixed the Disallow: /*? half of #2225. This fixes two more bugs in the same block of code, both of which only bite on Python 3.14+.

Python 3.14 rewrote urllib.robotparser: native wildcards, $, and RFC 9309 longest-match ranking. Crawl4AI carries two workarounds for the old parser, and on 3.14 each one now makes correct stdlib behaviour worse.

1. The wildcard monkey patch flattens rule ranking

crawl4ai/utils.py replaces RuleLine.applies_to unconditionally. On 3.14 that method returns the match length, used to rank competing rules; our patch returns a bool, so True == 1 and every wildcard rule drops to the lowest possible priority.

User-agent: *
Disallow: /
Allow: /public/*.html
URL 3.14 stdlib 3.14 + patch
/public/a.html allowed denied 🐞
/private/a.html denied denied

2. The bare-? rewrite outranks a narrower Allow:

_preserve_bare_query rewrites /*? to /*?* so the ? survives path normalization. Below 3.14 that is exactly right and still needed. On 3.14 it is wrong: /*?* matches to end of string, so it now outranks a narrower Allow: that the raw /*? would have lost to.

User-agent: *
Allow: /*?q=
Disallow: /*?
URL 3.14 stdlib 3.14 + rewrite
/search?q=1 allowed denied 🐞
/search?x=1 denied denied

Net effect of both: on 3.14, Crawl4AI skipped pages robots.txt explicitly permits.

The change

Gate both workarounds behind sys.version_info < (3, 14). Below 3.14 nothing changes — the old parser has no wildcard support and does drop the ?, so both are still required. From 3.14 the stdlib is already correct and we get out of its way. The matcher itself is untouched.

Also: moved the RuleLine import into the branch that uses it, and dropped a duplicate import re.

Tests

Five tests in tests/unit/test_robots_query_rules.py:

  • test_wildcard_patch_is_scoped_to_old_pythons — the patch is installed exactly where needed
  • test_wildcard_rules_work_on_every_pythonDisallow: /*.php still works after the gate
  • test_allow_overrides_broad_disallow — regression for bug 1 (3.14+)
  • test_query_allow_outranks_bare_query_disallow — regression for bug 2 (3.14+)
  • test_query_allow_still_wins_below_py314 — same rules, same answers below 3.14, via the rewrite
Check Result
test_robots_query_rules.py, Python 3.14.7 32 passed, 3 skipped
same, Python 3.12.9 29 passed, 6 skipped
each fix reverted individually on 3.14 its regression test fails — both are real
full tests/unit, 3.12.9 identical to baseline (16 pre-existing failures, pypdf not installed)

tests/general/test_robot_parser.py fails on develop as well — its assert duration < 0.03 is timing- and network-sensitive. Unrelated to this change.

Not fixed here

On Python <= 3.13 the $ end-anchor (Disallow: /*.pdf$) still does not work: quote() mangles the $ before the patch sees it. Pre-existing, untouched by this PR, and fixed by the stdlib from 3.14 on. Happy to do it separately.

Refs #2225

The wildcard monkey patch in utils.py overrides RuleLine.applies_to
unconditionally. Python 3.14 rewrote urllib.robotparser with native
wildcard, '$' and RFC 9309 longest-match support, where applies_to
returns the match *length* used to rank competing rules. The patch
returns a bool, so every wildcard rule collapses to the lowest
priority and Allow: overrides stop working:

    User-agent: *
    Disallow: /
    Allow: /public/*.html

denied /public/a.html on 3.14. Gate the patch to Python < 3.14, where
robotparser has no wildcard support and still needs it.

Follow-up to #2229, which fixed the 'Disallow: /*?' half of #2225.

Refs #2225

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ntohidi

ntohidi commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

Kept this separate from #2229 instead of rolling it in — it's really a different bug that just happens to sit in the same few lines. #2229 fixed the rule text; this one is about the patch itself doing the wrong thing on 3.14.

If you're on 3.14, this is the one to look at. The stdlib parser there is already RFC 9309 compliant, so we were overriding good behavior with worse. Nothing changes below 3.14.

Thanks @Nalhin for #2229 — the /*?/*?* trick is what sent me digging here in the first place.

Review follow-up on the previous commit, which gated the RuleLine
monkey patch but left _preserve_bare_query running on every Python.

On 3.14 that rewrite is not just unnecessary, it is wrong. The stdlib
ranks rules by match length, and '/*?*' matches to end of string, so
it outranks a narrower competing Allow:

    User-agent: *
    Allow: /*?q=
    Disallow: /*?

/search?q=1 is allowed by the stdlib and denied after the rewrite, so
Crawl4AI skipped pages robots.txt permits. Gate the call with the same
sys.version_info < (3, 14) as the patch, and say so in the docstring,
which claimed the two forms were always equivalent.

Also move the RuleLine import inside the branch that uses it and drop
a duplicate 'import re'.

Refs #2225

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ntohidi

ntohidi commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

Both findings were right — fixed in 7e17809.

1 (medium). Confirmed before fixing, on 3.14.7: with Allow: /*?q= + Disallow: /*?, the stdlib allows /search?q=1 and we denied it. My last commit gated the monkey patch and left the rewrite running everywhere, which is half a fix — thanks for catching it. _preserve_bare_query is now behind the same sys.version_info < (3, 14), and the docstring says why the two forms are only equivalent below 3.14.

Worth noting the split is real, not something to delete: on 3.12 the rewrite gives the right answers (/search?q=1 allowed, /search?x=1 denied), because those parsers take the first matching rule and length can't change the winner.

2 (low). Done — RuleLine import moved inside the branch, duplicate import re dropped. import sys stays at module scope since the condition needs it.

Added two tests. test_query_allow_outranks_bare_query_disallow is the regression for your case; reverting just the call-site gate fails it on 3.14 and nothing else. test_query_allow_still_wins_below_py314 pins the same rules from the other side so the version split can't silently drift. The existing test_allow_overrides_broad_disallow never would have caught this — no ? in its rules, so the rewrite never fired.

Suite: 32 passed / 3 skipped on 3.14.7, 29 passed / 6 skipped on 3.12.9, full tests/unit unchanged from baseline. PR description rewritten — it was overclaiming the Allow-override fix.

@ntohidi
ntohidi merged commit 6218bb8 into develop Sep 22, 2026
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.

1 participant