rzo1 opened a new pull request, #2146:
URL: https://github.com/apache/stormcrawler/pull/2146
Fixes #2096.
## Problem
`IndexerBolt.buildQuery()` joins the metadata labels straight into the
column list and into the `ON DUPLICATE KEY UPDATE` clause:
```java
final String columns = String.join(", ", keys);
...
.map(k -> String.format(Locale.ROOT, "%s=VALUES(%s)", k, k))
```
Values are bound as parameters, but identifiers cannot be. With a glob
mapping (an `indexer.md.mapping` entry ending in `*`) the label is the raw
metadata key — `AbstractIndexerBolt.filterMetadata()` returns `matchingKey`
unchanged for a glob — and metadata keys come from crawled content: the Tika
`ParserBolt` copies every `<meta name="...">` of a page to `parse.<name>`, and
response header names are stored the same way.
## Fix
Labels are checked against `^[A-Za-z0-9_]+$` before they reach
`buildQuery()` and the positional binding loop, and dropped when they do not
match. A dropped label is logged once per distinct label (capped, since crawled
content can mint unbounded distinct keys) and counted as `unusable_column_name`.
Dropping loses nothing that worked before: any label outside that pattern
already produced a broken unquoted identifier and a guaranteed `SQLException`.
Alias mappings (`parse.title=title`) remain the supported way to index a dotted
key. The table name and URL field are operator-owned topology config and are
left as they are.
## This also ends a replay loop
`execute()` fails the tuple without emitting to the status stream, so a
label that breaks the SQL fails the same tuple for as long as the topology
runs. That was not limited to hostile input: a glob mapping produces dotted
labels such as `parse.title` on entirely ordinary pages, and MySQL reads those
as `table.column`, so `Unknown column 'parse.title' in 'field list'` made the
glob mapping unusable with this bolt. Those labels are now dropped and the
document is indexed.
## Testing
- `ColumnNameValidationTest` — no Docker needed; pins the allowlist against
dotted labels and against key shapes a hostile page can mint.
- `IndexerBoltTest.testHostileMetadataKeyIsNotUsedAsColumnName` — MySQL
testcontainer; a hostile `parse.*` key under a glob mapping is dropped, the
document is still indexed and the tuple acked.
- `IndexerBoltTest.testDottedLabelFromGlobDoesNotFailTheTuple` — the
ordinary `parse.title` case is acked rather than replayed.
Separately, I ran nine payloads against MySQL 8.4 on the unfixed code —
comment-based truncation, backtick escapes, stacked statements, and a two-key
variant pairing an opened comment with a closed one, under both `parse.*` and
bare `*` mappings. None executed injected SQL; each was rejected by the server,
because the label is interpolated three times (once in the column list, twice
in `K=VALUES(K)`) and because the `?` placeholders and the `setString()` loop
are both derived from `keys.size()`, so a payload that comments out the tail
removes the placeholders and the binding throws first. The observed effect on
unfixed code is the failed-tuple replay above rather than executed SQL. That is
nine payloads, not a proof of impossibility, and the interpolation is the wrong
construction for a content-derived value either way.
`mvn -pl external/sql test` — 24 tests, all passing.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_017F5AsveKcSTHkpfXFLjXWQ
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
