Handle query params correctly for client app urls with no trailing slash

[muffinresearch]

Fixes mozilla/addons#13207

From local testing (to exercise both the prefix and the trailing slash middleware) this results in the following:

/android?foo=1 => /en-US/android/?foo=1
/en-US/android?foo=1 => /en-US/android/?foo=1
/en-US?foo=1 => /en-US/firefox/?foo=1

[muffinresearch]

I'll add some tests for locale only urls to this.

[codecov-io]

Codecov Report

Merging #8120 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   mozilla/addons-frontend#8120      +/-   ##
=========================================
+ Coverage   98.09%   98.1%   +<.01%     
=========================================
  Files         260     260              
  Lines        7363    7372       +9     
  Branches     1331    1333       +2     
=========================================
+ Hits         7223    7232       +9     
  Misses        126     126              
  Partials       14      14

Impacted Files	Coverage Δ
src/core/middleware/prefixMiddleware.js	100% <100%> (ø)	⬆️
src/amo/components/SearchFilters/index.js	100% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32c493b...15777fd. Read the comment docs.

[muffinresearch]

@willdurand this is ready for another look.

[willdurand]

r+wc

[willdurand]

Could we name the two different parts?

const [path, queryString] = req.originalUrl.split('?');

[muffinresearch]

I thought about doing that, but it assumes there's 2 parts, and that's not an unreasonable assumption, but if the url had multiple ? we'd lose the later ones using destructuring. Should we throwaway data if a url contains that mistake?

See below for a possible solution.

[willdurand]

If we have named variables for each url part, then we could build the url as is (I think):

const newURL = [`/${URLParts.join('/')}`, queryString].join('?');

This reads a bit better, no?

[muffinresearch]

Except that in the case of there being no queryString you need to check for it being undefined via the destructuring.

Both of the issues with destructuring are solved if the second parameter is a spread:

function splitURL(url) {
  const [path, ...queryString] = url.split('?');
  const URLParts = path.replace(/^\//, '').split('/');
  const newURL = [`/${URLParts.join('/')}`].concat(queryString).join('?');
  return newURL;
}

// Sane Query string                     
console.log(splitURL('/foo/bar?foo=1'));
// Bad query string
console.log(splitURL('/foo/bar?foo=1?foo=2'));
// No query string
console.log(splitURL('/foo/bar'));

Outputs:

• /foo/bar?foo=1
• /foo/bar?foo=1?foo=2
• /foo/bar

[muffinresearch]

I'm not sure if that's more readable though...

[willdurand]

ah well, maybe not...

Is a URL with more than one ? valid? I thought this was not possible.

[muffinresearch]

As discussed on irc merging as is, we can follow up as needed.