You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Serve robots.txt from the root, and keep crawlers off the comparisons - #291
Serves robots.txt at the root and excludes the fingerprint comparison URLs.
The file was never being served
acoustid/web/static/robots.txt has existed for years, and nothing ever served it at /robots.txt — only /favicon.ico had a route. Verified from outside, cache-busted:
/robots.txt 200, 1248 bytes, zero non-comment lines (Cloudflare's generated content-signals file)
/static/robots.txt 200, "User-agent: * / Disallow: /data/" (the real one)
Crawlers only read the root, so that Disallow: /data/ has never applied to anything.
Why now
The site is being crawled hard: up to 830,855 requests/hour, 87% of the resulting errors on /fingerprint/<id>/compare/<id>, and the pool exhaustion behind that has produced roughly 1.4 million 5xx since 4 November. Diagnosis from acoustid-admin, who also confirmed the origin 404 independently against the web pod.
The comparison endpoint is a combinatorial surface rather than a set of pages — every pair of fingerprints is its own URL, the similarity grid on a recording page links up to 144 of them, and each one runs acoustid_compare2 against the database.
The rule, and why the two shorter forms are wrong
Disallow: /fingerprint/*/compare/
Disallow: /compare/ matches nothing. robots.txt matches from the start of the path, and the path is /fingerprint/<id>/compare/<id>. This is the literal reading of "block the /compare/ endpoints" and it would have been a silent no-op.
Disallow: /fingerprint/ would work and is honoured by strictly more crawlers, but takes the /fingerprint/<id> detail pages with it. Keeping those indexed was the explicit call; the broader rule stays available if the narrow one proves insufficient.
So the rule needs the wildcard, which is standard since RFC 9309 and honoured by the major crawlers.
Its limit, measured rather than assumed: Python's urllib.robotparser does not implement * — under this rule it reports /fingerprint/1/compare/2 as allowed. So "wildcards are an extension" is not theoretical; a mainstream stdlib parser ignores the line outright. Anything built on a pre-RFC-9309 parser is unaffected by this change. Crawl-delay: 10 is in the same category — Bing honours it, Google does not.
Served rather than redirected to: Googlebot follows a redirect for robots.txt, but the crawlers causing this are the least likely to, and returning the body with a 200 costs nothing.
Tests
Six tests. The important one checks the rule against a URL built from the route itself via url_for, not against a hardcoded path — if the comparison route is ever renamed, the rule silently stops matching and crawling resumes, and this fails instead. Others assert the fingerprint, recording, track and index pages stay crawlable, because the tempting fix is the broad prefix that quietly de-indexes them.
Each was mutation-tested: using the literal /compare/ rule, blocking /fingerprint/ wholesale, dropping the rule, and removing the root route all fail the suite.
The path matching in the tests is hand-rolled, because robotparser cannot be the oracle for a rule it does not implement.
Full suite 426 passed, 1 skipped. Lint clean.
Two things this does not do
It stops well-behaved crawlers and nothing else. A client ignoring robots.txt needs a rate limit on that route, which is separate and still outstanding. Treat this as the cheap first move.
Cloudflare may mask it. It is currently generating the public robots.txt. It will most likely append its content signals to a real origin file once one exists, but it might keep overriding. So after deploy, verify from outside rather than against the origin:
acoustid/web/static/robots.txt has existed for years and nothing ever served
it at /robots.txt, so its Disallow has never applied to anything. Verified
from outside: /robots.txt returns 200 with 1248 bytes of Cloudflare's
generated content-signal comments and zero directives, while
/static/robots.txt returns the real file. Crawlers only read the root, so the
rule was never in force.
It is in force now because this is live: the site has been crawled at up to
830,855 requests/hour, 87% of the resulting errors are on
/fingerprint/<id>/compare/<id>, and the pool exhaustion behind that has
produced around 1.4 million 5xx since 4 November. The comparison endpoint is
a combinatorial surface rather than a set of pages -- every pair of
fingerprints is its own URL, the similarity grid on a recording page links up
to 144 of them, and each one runs acoustid_compare2 against the database.
Served rather than redirected to. Googlebot follows a redirect for
robots.txt; the crawlers causing this are the least likely to, and a 200 with
the body costs nothing.
The rule needs a wildcard, and the obvious shorter forms are both wrong.
Disallow: /compare/ matches nothing, because robots.txt matches from the
start of the path and the path is /fingerprint/<id>/compare/<id>. Disallow:
/fingerprint/ would match, but takes the fingerprint detail pages with it,
and keeping those indexed was the call. So the rule is
Disallow: /fingerprint/*/compare/ -- standard since RFC 9309 and honoured by
the major crawlers.
That has a real limit, measured rather than assumed: Python's
urllib.robotparser does not implement "*", and reports
/fingerprint/1/compare/2 as allowed under this rule. So anything using a
pre-RFC-9309 parser ignores the line. Crawl-delay is in the same category --
Bing honours it, Google does not.
The tests check the rule against a URL built from the route itself with
url_for, rather than against a hardcoded path. That is the pairing that
matters: if the comparison route is renamed, the rule silently stops matching
and crawling resumes, and the test fails instead. They also assert the
fingerprint, recording, track and index pages stay crawlable, since the
tempting fix is the broad prefix that quietly de-indexes them.
The matching in the tests is hand-rolled because robotparser cannot be the
oracle for a rule it does not support.
This stops well-behaved crawlers and nothing else. It is the cheap first
move, not the fix -- a client ignoring robots.txt needs a rate limit, which
is separate and outstanding.
No actionable comments were generated in the recent review.
Walkthrough
The application now serves a root /robots.txt file. The file sets a crawl delay, retains the /data/ disallow rule, and disallows fingerprint comparison paths. Tests check the response and verify blocked and allowed route paths.
Changes: Robots.txt serving and rules
Layer / File(s)
Summary
Crawler rules acoustid/web/static/robots.txt
The file sets a crawl delay, retains the /data/ rule, and disallows fingerprint comparison paths.
Root endpoint and route checks acoustid/web/views/general.py, tests/test_web.py
The /robots.txt route serves the static file directly. Tests check its plain-text response and confirm comparison paths match a Disallow rule while selected routes remain allowed.
The root robots.txt response and crawler rules are ready to merge after normal checks. Verify the public response after deployment because Cloudflare may override it.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title identifies the root robots.txt route and the comparison URL restriction.
Description check
The description covers the root robots.txt route, the comparison URL rule, and the related tests.
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Tip
🎯 Customize your risk assessments
Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.
✅ Automatic approvals, with your rules
Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Reviewing files that changed from the base of the PR and between 80a4324 and b772e3f.
Files selected for processing: 3
acoustid/web/static/robots.txt
acoustid/web/views/general.py
tests/test_web.py
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Serves
robots.txtat the root and excludes the fingerprint comparison URLs.The file was never being served
acoustid/web/static/robots.txthas existed for years, and nothing ever served it at/robots.txt— only/favicon.icohad a route. Verified from outside, cache-busted:Crawlers only read the root, so that
Disallow: /data/has never applied to anything.Why now
The site is being crawled hard: up to 830,855 requests/hour, 87% of the resulting errors on
/fingerprint/<id>/compare/<id>, and the pool exhaustion behind that has produced roughly 1.4 million 5xx since 4 November. Diagnosis from acoustid-admin, who also confirmed the origin 404 independently against the web pod.The comparison endpoint is a combinatorial surface rather than a set of pages — every pair of fingerprints is its own URL, the similarity grid on a recording page links up to 144 of them, and each one runs
acoustid_compare2against the database.The rule, and why the two shorter forms are wrong
Disallow: /compare/matches nothing. robots.txt matches from the start of the path, and the path is/fingerprint/<id>/compare/<id>. This is the literal reading of "block the /compare/ endpoints" and it would have been a silent no-op.Disallow: /fingerprint/would work and is honoured by strictly more crawlers, but takes the/fingerprint/<id>detail pages with it. Keeping those indexed was the explicit call; the broader rule stays available if the narrow one proves insufficient.So the rule needs the wildcard, which is standard since RFC 9309 and honoured by the major crawlers.
Its limit, measured rather than assumed: Python's
urllib.robotparserdoes not implement*— under this rule it reports/fingerprint/1/compare/2as allowed. So "wildcards are an extension" is not theoretical; a mainstream stdlib parser ignores the line outright. Anything built on a pre-RFC-9309 parser is unaffected by this change.Crawl-delay: 10is in the same category — Bing honours it, Google does not.Served rather than redirected to: Googlebot follows a redirect for robots.txt, but the crawlers causing this are the least likely to, and returning the body with a 200 costs nothing.
Tests
Six tests. The important one checks the rule against a URL built from the route itself via
url_for, not against a hardcoded path — if the comparison route is ever renamed, the rule silently stops matching and crawling resumes, and this fails instead. Others assert the fingerprint, recording, track and index pages stay crawlable, because the tempting fix is the broad prefix that quietly de-indexes them.Each was mutation-tested: using the literal
/compare/rule, blocking/fingerprint/wholesale, dropping the rule, and removing the root route all fail the suite.The path matching in the tests is hand-rolled, because
robotparsercannot be the oracle for a rule it does not implement.Full suite 426 passed, 1 skipped. Lint clean.
Two things this does not do
It stops well-behaved crawlers and nothing else. A client ignoring robots.txt needs a rate limit on that route, which is separate and still outstanding. Treat this as the cheap first move.
Cloudflare may mask it. It is currently generating the public robots.txt. It will most likely append its content signals to a real origin file once one exists, but it might keep overriding. So after deploy, verify from outside rather than against the origin:
If no
Disallowlines appear there, the app change is being shadowed and it needs a Cloudflare-side fix instead.