Skip to content

Limit search queries to the search fields and stop indexing comment email addresses - #225

Merged
snoopdave merged 6 commits into
roller-6.1.xfrom
search-public-fields
Oct 30, 2026
Merged

snoopdave merged 6 commits into
roller-6.1.xfrom
search-public-fields

Conversation

@snoopdave

Copy link
Copy Markdown
Contributor

Weblog search changes:

  • A query may name only the fields that search covers (entry content, title and comment text). A query that names another index field, such as username:…, is treated like any other query that cannot be parsed.
  • Comment email addresses are no longer added to the search index. Indexes built by earlier versions still hold the field until they are rebuilt; the query check covers them in the meantime.

Tests: SearchFieldsQueryParserTest (new) and a new check in IndexManagerTest. Full build and tests pass on JDK 11.

This PR and the search-results PR both add a few lines to IndexManagerTest; whichever merges second needs a trivial rebase.

…mail addresses

Queries that name another index field are a parse error. Indexes built by earlier versions still hold the email field; the query check covers them, and a rebuild drops the field.
@snoopdave

Copy link
Copy Markdown
Contributor Author

🤖 GLM: PR-Review — General Issues

The following issues were found but cannot be attached to a specific line in the diff:

  • Important: CHANGES.md has no entry for this change. Other PRs in the 6.1.7 batch each carry one. The user-facing parts belong in the 6.1.7 notes: queries that name an index field outside the search fields are now a parse error (so e.g. name:-prefixed or email:-prefixed terms no longer work in the public search box), and a search index rebuilt after upgrading drops the email field entirely — existing indexes keep the field but the query check covers them.

Strengths (not action items): overriding the full set of parser hooks (field/fuzzy/prefix/wildcard/regexp/range, both getFieldQuery signatures) closes the bypass shapes, not just the plain prefix form, and the tests enumerate exactly those shapes.

@snoopdave

Copy link
Copy Markdown
Contributor Author

🤖Claude: fixed by adding a CHANGES.md entry under 6.1.7 behaviour changes, including a note to rebuild the search index (e24deaf).

@mraible mraible left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the field check against about 20 query forms: quoted, grouped, boosted, prefixed with +/-, ranges, wildcards and mixed case. Nothing gets around it, and plain, title and comment queries still parse. LGTM, with one small, non-blocking note inline.

@snoopdave snoopdave left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is right. Some ways to make it harder to break, inline, plus:

  • username is still indexed and stored (IndexOperation.getDocument, the FieldConstants.USERNAME field). Nothing outside IndexOperation reads it: not for filtering, sorting or display. Same as c_name (inline). Dropping both means the index holds only what search needs, and the parser check becomes a second line of defense.
  • No test that the document no longer holds c_email. A test on IndexOperation.getDocument that checks the field is gone (and username and c_name, if dropped) would keep it from coming back.

Comment thread CHANGES.md Resolved
…the index

SearchFieldsQueryParser now walks the parsed query with a QueryVisitor
instead of overriding each query builder. The index no longer holds author
user names or comment author names. At startup, an index that still holds
a removed field is deleted and rebuilt in the background.
@snoopdave
snoopdave merged commit 666e793 into roller-6.1.x Oct 30, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants