CSRF token: handle failed url-decoding

[alxndrsn]

Related:

• http: handle bad url-encoded Cookie session token #1269
• audit-logs: handle bad url-encoded X-Action-Notes header #1268
• fieldKeyParser: handle bad url-encoded prefix key #1267
• formats/odata: handle bad url-encoded subpaths #1270

Ref #1260

[matthew-white]

My feeling overall when looking at these PRs is that the try/catch blocks make the main logic slightly harder to follow. Is there an alternative to try/catch that we could consider? How about a function that returns something like null or Option.none() if the string can't be decoded? Something like:

const tryDecodeURI = (s) => {
  try {
    return decodeURIComponent(s);
  } catch (error) {
    return null;
  }
};

or

const tryDecodeURI = (s) => {
  try {
    return Option.of(decodeURIComponent(s));
  } catch (error) {
    return Option.none();
  }
};

I think a function like this could reduce the number of nested blocks or otherwise enable us to structure the code in a different way from what try/catch requires.

[alxndrsn]

Is there an alternative to try/catch that we could consider? How about a function that returns something like null or Option.none() if the string can't be decoded?

How would this work for something like #1270:

// Given an OData URL subpath, returns a contextStack: [ [ pathName: String, tableId: String? ] ]
// that the subpath represents.
const extractPathContext = (subpath) => {
  const rawParts = subpath.split('/').slice(1);
  let parts;
  try {
    parts = rawParts.map(part => decodeURIComponent(part));
  } catch (err) {
    throw Problem.user.unparseableODataExpression({ reason: 'URL-decoding failed' });
  }
  return parts.map((part) => {
    const match = /^([^(]+)\('((?:uuid:)?[a-z0-9-]+)'\)$/i.exec(part);
    return (match == null) ? [ part, null ] : [ match[1], match[2] ];
  });
};

[matthew-white]

How about something like:

const extractPathContext = (subpath) =>
  subpath.split('/').slice(1).map((part) => {
    const decoded = tryDecodeURI(part);
    if (decoded == null)
      throw Problem.user.unparseableODataExpression({ reason: 'URL-decoding failed' });
    const match = /^([^(]+)\('((?:uuid:)?[a-z0-9-]+)'\)$/i.exec(decoded);
    return (match == null) ? [ part, null ] : [ match[1], match[2] ];
  });

[alxndrsn]

@matthew-white looks good; is that preferred to:

const extractPathContext = (subpath) =>
  subpath.split('/').slice(1).map((part) => {
    const decoded = decodeURI(part);
    if (decoded.isEmpty())
      throw Problem.user.unparseableODataExpression({ reason: 'URL-decoding failed' });
    const match = /^([^(]+)\('((?:uuid:)?[a-z0-9-]+)'\)$/i.exec(decoded.get());
    return (match == null) ? [ part, null ] : [ match[1], match[2] ];
  });

?

[matthew-white]

An approach that uses Option sounds great to me. 👍