Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

[Bug report] BasicURLNormalizer unmangleQueryString inserts a second `?` when a query value contains a `/`

Open
  • Is something wrong?
#2,198 0 comments 0 reactions 1 assignee View on GitHub

Maintainers usually reply within 1 day

@jnioche is already working on this.

Since Sep 29, 2026.

  • #2231 by @sean0407 — open

Assessment

This issue has not been assessed yet.

Description

bug core
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

Dominant language
Java
Stars
1k
Forks
295
Avg merge
1d 14h
Merged PRs (30d)
62

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from apache/stormcrawler

All issues in apache/stormcrawler

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.