Skip to content

Conversation

@bourgeoa
Copy link
Member

@bourgeoa bourgeoa commented Mar 6, 2022

No description provided.

@bourgeoa bourgeoa marked this pull request as draft March 6, 2022 22:58
@bourgeoa bourgeoa linked an issue Mar 6, 2022 that may be closed by this pull request
@jeff-zucker
Copy link
Member

Hmm, is this really what we want to do? What about a URI with an URI-encoded search string? Should it be forbidden in the URI itsdelf but permitted in the search string?

// URL specified as Express request object
if (!url.pathname && url.path) {
const { hostname, path } = url
if (path.includes('%2F')) throw new Error('Url cannot contain %2F (%encoded /)')
Copy link
Contributor

@RubenVerborgh RubenVerborgh Mar 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above implementation is broken; it only works on 2 out of 3 cases.
Rather than duplicating the code another time, it should be in a single place.

So if you must, do:

  // Parses a URL into hostname and pathname
  _parseUrl (url) {
    let parsed;
    // URL specified as string
    if (typeof url === 'string') {
      parsed = this._parseUURL.parse(url)
    }
    // URL specified as Express request object
    else if (!url.pathname && url.path) {
      const { hostname, path } = url
      parsed = { hostname, pathname: path.replace(/[?#].*/, '') }
    }
    // URL specified as object
    else {
      parsed = url;
    }
    // <=== do your path replacements here
    return parsed
  }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@bourgeoa
Copy link
Member Author

resolved with #1690 merged with #1695

@bourgeoa bourgeoa closed this Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mis handling of percent encoded paths

3 participants