Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

URL prefix is misused and not well documented #19

Open
pmarien opened this issue Aug 29, 2019 · 2 comments
Open

URL prefix is misused and not well documented #19

pmarien opened this issue Aug 29, 2019 · 2 comments

Comments

@pmarien
Copy link
Member

pmarien commented Aug 29, 2019

Let's look at where url/api prefix is eventually used \Enm\JsonApi\Model\Request\Request::parseUriPath:

        preg_match(
            '/^(([a-zA-Z0-9\_\-\.\/]+.php)(\/)|)(' . $this->apiPrefix . ')([\/a-zA-Z0-9\_\-\.]+)$/',
            trim($path, '/'),
            $matches
        );

So the prefix is used in a regex using / as open/close characters. This basically means the prefix cannot contain that character (unless escaped).

Why does the default/test implementation work? Because the prefix is trimmed and the example doesn't try subpaths.

What's the problem with this?

  • the /character is very common in paths, unlike other regex special characters
  • documentation here doesn't say that this string is part of a regex - it also gives the impression that I have to write {type} somewhere in the string when in fact, it doesn't seem to be the case
  • the example here works by coincidence - it should have been '\\/api' instead

What are the solutions?

  • change the regex open/close to something less disruptive (eg: #theregex#)
    • pro: ideal case
    • con: backward compatibility break
  • escape prefix before injecting it into the regex (eg: preg_match('/...' . preg_quote(...) . '.../'...)
    • pro: no need to care that the prefix will be part of a regex
    • con: backward compatibility break, prefix will become plain text match and cannot be part of regex (eg: api(-v\d+) will now not work)
  • update documentation appropriately (warn about escaping / and maybe example code for subpaths)
    • pro: backward compatible
    • con: code will still be ambiguous, not ideal case (from code perspective) - this is less of a problem if the parameter is renamed appropriately (eg: url_prefix => url_prefix_regex)

See original issue: eosnewmedia/JSON-API-Server#9

@javi-p-nt
Copy link

Ended using preg_quote(trim($prefix,'/'),'/') to make it work for subpaths.

@ruuds
Copy link

ruuds commented Mar 11, 2021

I fixed it by escaping the / within the $prefix. preg_quote didn't work for me:

preg_match(
    '/^(([a-zA-Z0-9\_\-\.\/]+.php)(\/)|)(' . str_replace('/','\\/',$this->apiPrefix) . ')([\/a-zA-Z0-9\_\-\.]+)$/',
    trim($path, '/'),
    $matches
);

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

No branches or pull requests

3 participants