[Bug report] BasicURLNormalizer unmangleQueryString inserts a second `?` when a query value contains a `/`
メンテナーはふだん 1 日以内に返信
評価
この issue はまだ評価されていません。
説明
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
- 主要言語
- Java
- スター
- 1k
- フォーク
- 295
- 平均マージ
- 1日 14時間
- マージ済み PR(30日)
- 62
環境構築
- Dockerfile・Docker Compose ファイルなし
- プルリクエストのテンプレートあり
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
apache/stormcrawler のほかの issue
-
Remove dependency on httpcomponents.core5対応中かも @rzo1 が今日担当しました。 オープンBREAKING CHANGE core
難易度 5/5 1週間以上 初心者へのやさしさ 15/100
apache/stormcrawler#2266 ·
メンテナーはふだん 1 日以内に返信
-
URLNormalizer The query is split on `;` as well as `&`, which rewrites URLs対応中かも @jnioche が 27 日前に担当しました。 オープンbug
apache/stormcrawler#2199 · 担当者 1 名 ·
メンテナーはふだん 1 日以内に返信
-
難易度 3/5 1〜2日 初心者へのやさしさ 70/100
apache/stormcrawler#2131 ·
メンテナーはふだん 1 日以内に返信
-
[Improvement] Work towards Maven 4 Compatibility対応中かも @mvolikas が 351 日前に担当しました。 オープンgood first issue help wanted
apache/stormcrawler#1662 · コメント 2 件 · 担当者 1 名 ·
メンテナーはふだん 1 日以内に返信
-
Delete redirected pages再び着手できるかも @tomatotomata が 84 日前に担当しましたが、オープン中のプルリクエストはありません。 オープンcore good first issue
難易度 4/5 3〜5日 初心者へのやさしさ 48/100
apache/stormcrawler#994 · コメント 3 件 ·
メンテナーはふだん 1 日以内に返信
apache/stormcrawler の issue をすべて見る
似ている issue
-
pdfium-drift
難易度 1/5 1時間未満 初心者へのやさしさ 88/100
Stirling-Tools/JPDFium#188 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
openremote/openremote#3487 · コメント 1 件 ·
-
Type/Bug U2
難易度 2/5 1〜3時間 初心者へのやさしさ 90/100
wso2/product-is#28675 ·
メンテナーはふだん 1 日以内に返信
-
a:bug
難易度 2/5 1〜3時間 初心者へのやさしさ 83/100
メンテナーはふだん 1 日以内に返信
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
メンテナーはふだん 1 日以内に返信