Skip to content

Commit

Permalink
charon: Redirect to the fully-resolved prefix more robustly
Browse files Browse the repository at this point in the history
Compare (canonicalized) prefixes, not URLs, because that's more
precisely what we're concerned about here.

Construct the relative URL for redirection using proper URL handling.
The previous method for constructing the new URL was problematic for
several reasons, including that whether other query params were passed
along or not with the baseUrl depended on where they were relative to
the prefix param.  Also, while unlikely in this case, if a prefix value
appeared elsewhere in the URL earlier than the query param, it all broke
because of the split().
  • Loading branch information
tsibley committed Apr 8, 2021
1 parent 3278918 commit 9488d34
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 10 deletions.
19 changes: 11 additions & 8 deletions src/getDataset.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,18 @@ const getDataset = async (req, res) => {
return helpers.unauthorized(req, res);
}

const baseUrl = req.url.split(query.prefix)[0];
let redirectUrl = baseUrl + '/' + resolvedPrefix;
if (query.type) {
redirectUrl += `&type=${query.type}`;
}
/* If we got a partial prefix and resolved it into a full one, redirect to
* that. Auspice will notice and update its displayed URL appropriately.
*/
if (resolvedPrefix !== helpers.canonicalizePrefix(query.prefix)) {
// A absolute base is required but we won't use it, so use something bogus.
const resolvedUrl = new URL(req.originalUrl, "http://x");
resolvedUrl.searchParams.set("prefix", resolvedPrefix);

const relativeResolvedUrl = resolvedUrl.pathname + resolvedUrl.search;

if (redirectUrl !== req.url) {
utils.log(`Redirecting client to: ${redirectUrl}`);
res.redirect(redirectUrl);
utils.log(`Redirecting client to resolved dataset URL: ${relativeResolvedUrl}`);
res.redirect(relativeResolvedUrl);
return undefined;
}

Expand Down
9 changes: 8 additions & 1 deletion src/getDatasetHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,17 @@ const parsePrefix = (prefix) => {
};


/* Round-trip prefix through split/join to canonicalize it for comparison.
*/
const canonicalizePrefix = (prefix) =>
joinPartsIntoPrefix(splitPrefixIntoParts(prefix));


module.exports = {
splitPrefixIntoParts,
joinPartsIntoPrefix,
handle500Error,
unauthorized,
parsePrefix
parsePrefix,
canonicalizePrefix,
};
2 changes: 1 addition & 1 deletion test/smoke-test/auspice_client_requests.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"url": "/charon/getDataset?prefix=/flu",
"expectStatusCode": 302,
"responseIsJson": false,
"redirectsTo": "/charon/getDataset?prefix=/flu/seasonal/h3n2/ha/2y"
"redirectsTo": "/charon/getDataset?prefix=flu%2Fseasonal%2Fh3n2%2Fha%2F2y"
},
{
"name": "Check that the main getAvailable API call returns a JSON",
Expand Down

0 comments on commit 9488d34

Please sign in to comment.