jnioche opened a new pull request, #2207: URL: https://github.com/apache/stormcrawler/pull/2207
Fixes #2197 ## What changed When `BasicURLNormalizer.filter()` rebuilt a URL, it passed `URL.getFile()` (path **and** query) as the *path* argument of the multi-argument `URI` constructor. That constructor quotes anything illegal in the component it is given and always quotes `%`, so: - the `?` separating the query became `%3F`, folding the query into the path (`HTTP://A.Example/p?b=2&a=1` → `http://a.example/p%3Fb=2&a=1`); - existing escapes were double-encoded (`%21` → `%2521`, `%20` → `%2520`). A rebuild happens only when the scheme or host is not lowercase, the host is converted by `hostIDNtoASCII`, or `unescapePath`/`escapePath` changes the path. URLs that need none of these kept their query, which is why the bug went unnoticed. The fix: - only the path goes through `unescapePath`/`escapePath`, and only a changed path sets `hasChanged`; - on a rebuild, the already percent-encoded components (scheme, host, port, escaped path, original query) are joined as they are and validated with the single-argument `new URI(String)`, which parses strictly without re-encoding. ## Notes for reviewers - Nothing is lost by dropping the multi-argument constructor: its input has already been parsed by the strict `URI` parser in `URLUtil.toURL` (after `sanitizeForURI`), and `unescapePath` only decodes unreserved characters, so there is nothing illegal left to quote. If something illegal did get through, the URL is now dropped (returns `null`) instead of being silently rewritten. - The issue's example expects `~!q` to end up as `~%21q`; in fact `URLEncodedUtils.format` in `processQueryElements` writes `~` as `%7E`, so the correct output is `t=%7E%21q`. The test asserts that. - Unchanged: as before, user-info is dropped when a URL is rebuilt. That could be a separate issue. ## Tests Two new tests in `BasicURLNormalizerTest`, which both fail on `main`: - `testQueryKeptWhenURLRebuilt`: rebuilds caused by uppercase scheme/host, `%7e` → `~`, `\` → `%5C`, and a URL with a port; plus an unchanged URL. - `testEscapesNotDoubleEncodedWhenURLRebuilt`: `%20`, `%21`, `%3D` survive a rebuild, including the `queryElementsToRemove` example from the issue. Full `core` test suite passes (651 tests) and the code is formatted with `git-code-format:format-code`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
