fix(citation): match special-case hosts by domain suffix, not substring - #158
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d9e1760f8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let Some((_tld, without_tld)) = labels.split_last() else { | ||
| return false; | ||
| }; | ||
| SPECIAL_CASE_DOMAINS | ||
| .iter() | ||
| .any(|needle| host.contains(needle)) | ||
| .any(|domain| without_tld.ends_with(domain)) |
There was a problem hiding this comment.
Because this drops exactly one label before matching, Google Books country hosts with multi-label public suffixes such as https://books.google.co.uk/... leave without_tld as ['books', 'google', 'co'], which does not end with ['books', 'google']. These hosts were covered by the previous books.google. rule and by the intended books.google.* viewer-shell class, so they now fall through to normal body classification instead of returning ViewerShell.
Useful? React with 👍 / 👎.
8d9e176 to
c9ff592
Compare
is_special_case_host used a raw substring check (host.contains("books.google.")),
so an attacker-controlled host such as books.google.com.evil.example — or
xbooks.google.com — was misclassified as the Google Books viewer shell and its
body suppressed as unusable. Match on DNS label boundaries instead: split the
host, drop the TLD, and require the remaining labels to end with the special-case
domain labels. Subdomains (www.books.google.de) still match.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
c9ff592 to
3d94b48
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d94b48b00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }; | ||
| let host = host.to_ascii_lowercase(); | ||
| SPECIAL_CASE_HOSTS | ||
| let labels: Vec<&str> = host.split('.').collect(); |
There was a problem hiding this comment.
For a valid fully qualified URL such as https://books.google.com./books?id=x, host_str() retains the trailing dot, so splitting produces ['books', 'google', 'com', '']; this code drops the empty root label rather than com, and the suffix check then returns false. The previous substring rule classified this equivalent Google Books host as ViewerShell, so normalize a trailing root label before removing the TLD.
Useful? React with 👍 / 👎.
2 participants
is_special_case_hostusedhost.contains("books.google."), so an attacker-controlled host such asbooks.google.com.evil.example(orxbooks.google.com) was misclassified as the Google Books viewer shell and its body suppressed as unusable.Now matches on DNS label boundaries: split the host, drop the TLD, require the remaining labels to end with the special-case domain. Subdomains (
www.books.google.de) still match.🤖 Generated with Claude Code