[Bug report] BasicURLNormalizer unmangleQueryString inserts a second `?` when a query value contains a `/`
Maintainer thường phản hồi trong vòng 1 ngày
Đánh giá
Issue này chưa được đánh giá.
Mô tả
Version
main branch
Describe what's wrong
Summary
unmangleQueryString is meant to repair a URL whose query starts with & instead of ?
(http://foo.example&a=b → http://foo.example?a=b). It decides whether a query is already
present by looking at the last /-separated element of the URL only. When a query parameter
value contains a /, the real ? sits in an earlier element and is not seen, so the check
passes and the first & of a perfectly valid query is rewritten to ?.
Everything after that point stops being a query parameter: it becomes part of the value of
the parameter before it.
It is on by default (unmangleQueryString defaults to true).
Reproduction
With an empty params block, so nothing else touches the query:
BasicURLNormalizer normalizer = new BasicURLNormalizer();
normalizer.configure(new HashMap<>(), new ObjectMapper().readTree("{}"));
String url = "http://a.example/article.pl?sid=01/10/23/1816257&mode=thread&tid=107";
System.out.println(normalizer.filter(URLUtil.toURL(url), new Metadata(), url));
| input | actual | expected |
|---|---|---|
http://a.example/article.pl?sid=01/10/23/1816257&mode=thread&tid=107 |
http://a.example/article.pl?sid=01/10/23/1816257?mode=thread&tid=107 |
unchanged |
http://a.example/p?u=http://b.example/x&z=1 |
http://a.example/p?u=http://b.example/x?z=1 |
unchanged |
http://a.example/login/?next=/a/b/&utm_source=ref&z=1 |
http://a.example/login/?next=/a/b/?utm_source=ref&z=1 |
unchanged |
Any URL carrying a path-like or URL-like value is affected — next=, url=, redirect=,
return_to=, and ids such as sid=01/10/23/1816257.
Consequence for parameter removal
With {"queryElementsToRemove":["utm_source"]}, the third URL above becomes:
http://a.example/login/?next=%2Fa%2Fb%2F%3Futm_source%3Dref&z=1
utm_source is still there, and is no longer removable: by the time the query is parsed it
is part of the value of next, not a parameter of its own. So the bug also silently defeats
queryElementsToRemove and removeHashes for these URLs.
Cause
private String unmangleQueryString(String urlToFilter) {
String[] pathElements = urlToFilter.split("/");
final String lastPathElement = pathElements[pathElements.length - 1];
int firstAmp = lastPathElement.indexOf('&');
if (firstAmp == -1) {
return urlToFilter;
}
int firstQuestionMark = lastPathElement.indexOf('?'); // ← only the last element
if (firstQuestionMark == -1 && lastPathElement.indexOf("=") > 0) {
pathElements[pathElements.length - 1] = lastPathElement.replaceFirst("&", "?");
return String.join("/", pathElements);
}
return urlToFilter;
}
For http://a.example/article.pl?sid=01/10/23/1816257&mode=thread&tid=107, splitting on /
makes the last element 1816257&mode=thread&tid=107. It holds an &, no ? and an =, so
it looks exactly like a query that lost its ?.
Impact
The rewritten URL points at a different resource, since the server sees a single parameter
whose value now contains the rest of the query. Redirect and login URLs, which are precisely
the ones carrying a URL as a parameter value, are the most affected.
Suggested fix
Decide whether a query is present by looking at the whole URL rather than at its last /
element, and leave the URL alone when there is already a ?:
final int hash = urlToFilter.indexOf('#');
final String beforeFragment = hash == -1 ? urlToFilter : urlToFilter.substring(0, hash);
if (beforeFragment.indexOf('?') != -1) {
return urlToFilter; // already has a query, nothing to unmangle
}
// ... unchanged from here: last element, first '&', '=' check, replaceFirst("&", "?")
Keeping the rest of the method as it is preserves the repair it was written for, and the
restriction to the last element still stops an & in the path from being mistaken for the
start of a query. The fragment is excluded from the check so that a ? inside #… does not
suppress a genuine repair.
These must keep working, and do with the change above:
| input | output |
|---|---|
http://a.example&a=b |
http://a.example?a=b |
http://a.example/p&a=b&c=d |
http://a.example/p?a=b&c=d |
http://a.example/p?a=1&b=2 |
unchanged |
Error message and/or stacktrace
N/A
How to reproduce
With an empty params block, so nothing else touches the query:
BasicURLNormalizer normalizer = new BasicURLNormalizer();
normalizer.configure(new HashMap<>(), new ObjectMapper().readTree("{}"));
String url = "http://a.example/article.pl?sid=01/10/23/1816257&mode=thread&tid=107";
System.out.println(normalizer.filter(URLUtil.toURL(url), new Metadata(), url));
| input | actual | expected |
|---|---|---|
http://a.example/article.pl?sid=01/10/23/1816257&mode=thread&tid=107 |
http://a.example/article.pl?sid=01/10/23/1816257?mode=thread&tid=107 |
unchanged |
http://a.example/p?u=http://b.example/x&z=1 |
http://a.example/p?u=http://b.example/x?z=1 |
unchanged |
http://a.example/login/?next=/a/b/&utm_source=ref&z=1 |
http://a.example/login/?next=/a/b/?utm_source=ref&z=1 |
unchanged |
Any URL carrying a path-like or URL-like value is affected — next=, url=, redirect=,
return_to=, and ids such as sid=01/10/23/1816257.
Additional context
No response
- Ngôn ngữ chính
- Java
- Star
- 1k
- Fork
- 295
- Merge trung bình
- 1 ngày 14 giờ
- Pull request đã merge (30 ngày)
- 62
Chuẩn bị môi trường
- Không có Dockerfile hay tệp Docker Compose
- Có mẫu pull request
- Đọc hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của apache/stormcrawler
-
Remove dependency on httpcomponents.core5Có thể đã có người làm @rzo1 đã nhận hôm nay. Đang mởBREAKING CHANGE core
Độ khó 5/5 Hơn một tuần Mức phù hợp với người mới 15/100
apache/stormcrawler#2266 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
URLNormalizer The query is split on `;` as well as `&`, which rewrites URLsCó thể đã có người làm @jnioche đã nhận 27 ngày trước. Đang mởbug
apache/stormcrawler#2199 · 1 người được giao ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 3/5 1-2 ngày Mức phù hợp với người mới 70/100
apache/stormcrawler#2131 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
[Improvement] Work towards Maven 4 CompatibilityCó thể đã có người làm @mvolikas đã nhận 351 ngày trước. Đang mởgood first issue help wanted
apache/stormcrawler#1662 · 2 bình luận · 1 người được giao ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Delete redirected pagesCó thể làm lại được @tomatotomata đã nhận 84 ngày trước và không có pull request nào đang mở. Đang mởcore good first issue
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 48/100
apache/stormcrawler#994 · 3 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
Tất cả issue của apache/stormcrawler
Issue tương tự
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 67/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 76/100
linagora/twake-calendar-side-service#1218 · 1 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
[Bug] [Go] Ack returns nil when the server answers with a non-OK statusCó thể đã có người làm @leizhiyuan đã nhận hôm nay. Đang mở
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 84/100
apache/rocketmq-clients#1465 ·
Maintainer thường phản hồi trong vòng 2 ngày
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 70/100
Maintainer thường phản hồi trong vòng 1 ngày
-
pdfium-drift
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 88/100
Stirling-Tools/JPDFium#188 ·
Maintainer thường phản hồi trong vòng 1 ngày