jnioche opened a new issue, #2199:
URL: https://github.com/apache/stormcrawler/issues/2199
## Summary
`BasicURLNormalizer` parses the query with HttpClient's
`URLEncodedUtils.parse(query,
StandardCharsets.UTF_8)`. That overload treats **both** `&` and `;` as
parameter separators,
while the matching `format(...)` only ever writes `&`. A `;` that a site
uses as an ordinary
character inside a query is therefore read as the start of a new parameter
and written back
as `&`, so the normalised URL asks the server for something different from
the original.
This contradicts the current URL standard, which splits on `&` only — see
[Standards](#standards) below.
## Reproduction
The query is only parsed when `queryElementsToRemove` is non-empty or
`removeHashes` is
true, so:
```java
BasicURLNormalizer normalizer = new BasicURLNormalizer();
normalizer.configure(
new HashMap<>(),
new
ObjectMapper().readTree("{\"queryElementsToRemove\":[\"utm_source\"]}"));
String url = "http://a.example/search?q=a;b";;
System.out.println(normalizer.filter(URLUtil.toURL(url), new Metadata(),
url));
```
| input | actual | expected |
|---|---|---|
| `http://a.example/search?q=a;b` | `http://a.example/search?b&q=a` |
`http://a.example/search?q=a;b` |
| `http://a.example/p?a=1;b=2` | `http://a.example/p?a=1&b=2` | unchanged |
| `http://a.example/books?dq=x+;&hl=fr` |
`http://a.example/books?dq=x+&hl=fr` | unchanged |
| `http://a.example/p?q=a%3Bb&z=1` | `http://a.example/p?q=a%3Bb&z=1` |
unchanged, correct |
The first row is the clearest: a search term `a;b` becomes two parameters,
`q=a` and a bare
`b`. The third shows a value ending in `;`, which produces an empty
parameter that is then
dropped — the `;` disappears from the value. An escaped `%3B` is safe, as it
should be.
(The reordering in the first row comes from the unconditional sort in the
same method, not
from this issue.)
## Cause
`URLEncodedUtils` in HttpClient 4.5.14:
```java
private static final char QP_SEP_A = '&';
private static final char QP_SEP_S = ';';
public static List<NameValuePair> parse(final String s, final Charset
charset) {
...
return parse(buffer, charset, QP_SEP_A, QP_SEP_S); // ← both
separators
}
public static List<NameValuePair> parse(final String s, final Charset
charset,
final char... separators) { ... } // ← separators
can be chosen
```
`format(...)` always joins with `QP_SEP_A`, which is what turns `;` into `&`
on the way out.
## Standards
**WHATWG** is the Web Hypertext Application Technology Working Group, the
body (Apple,
Google, Mozilla, Microsoft) that maintains the "living standards" browsers
actually
implement — HTML, DOM, Fetch and the URL Standard among them. Its URL
Standard is what
defines URL parsing for the web today, in place of the older RFCs.
- [URL Standard, `application/x-www-form-urlencoded`
parsing](https://url.spec.whatwg.org/#urlencoded-parsing):
the input is split on `0x26 (&)`, and only on `&`. `;` has no special
meaning.
- [RFC 3986 §3.4](https://www.rfc-editor.org/rfc/rfc3986#section-3.4) lists
`;` as an
allowed character in a query but gives it no meaning: how a query is
divided is left to
the server.
- The `;` convention comes from
[HTML 4.01, appendix
B.2.2](https://www.w3.org/TR/html401/appendix/notes.html#h-B.2.2)
(1999), which *recommended* that servers accept `;` in place of `&` so
that authors did
not have to write `&` in HTML attributes. It was a recommendation to
server authors,
never a rule for URL parsers, and it is absent from the living standard.
Since a crawler has to ask the server for exactly what the link said,
following the URL
Standard here seems the safer reading: a `;` that the server treats as data
should survive
normalisation.
## Impact
Every affected URL is fetched with a different query than the one the link
carried, which
can change or break the response. It also splits one page into two entries
when the same
content is linked with and without the `;` form, and defeats
`queryElementsToRemove` in the
opposite direction: a parameter can appear or vanish depending on where a
`;` falls.
## Suggested fix
Ask for `&` only — the overload already exists, so it is one argument:
```java
List<NameValuePair> pairs = URLEncodedUtils.parse(query,
StandardCharsets.UTF_8, '&');
```
If some crawls rely on the old behaviour, this could be a parameter
(`querySeparators: "&;"`) defaulting to `&` alone.
--
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]
