Invalid URI characters in password or username break $this-path

[lletourn]

https://github.com/thephpleague/openapi-psr7-validator/blob/7dca015645810a5ffffe12341938f83411b3fcb2/src/PSR7/PathFinder.php#L43

Pathfinder uses the Slim URIInterface in parse_url. Problem is if you have a header with an Authorized field that contains a password with ? or $ or #, UtiInterface will build a Uri that cannot be directly parsed by parse_url. It needs to be pct-encoded first.

It's not a slim bug because Uri is meant to return a decoded URL. It's not a php bug because parse_url (per doc) is meant to work with raw encoded HTTP URIs

Either work with the URIInterface object directly (instead of a string from it) or reencode an URL that parse_url can use before a call to PathFinder.

Or am I missing something? Thanks

[scaytrase]

can you please provide a sample test case or code snipped that reproduces the issue?

[lletourn]

slimtest.patch.txt Here's a patchfile. I wonder if it's a slim bug that URI doesn't reencode the user/pwd when passed to the constructor.

PSR7 is unclear if __tostring needs to return an encoded string or not. Guzzle project had the same discussion: https://github.com/guzzle/psr7/issues/252

[scaytrase]

Using guzzle URI works here

//        $uri = new SlimUri('https', 'example.com', null, '/empty' . '', '', 'user', 'abc?def');
        $uri = (new Uri())
            ->withScheme('https')
            ->withHost('example.com')
            ->withPort(null)
            ->withPath('empty')
            ->withFragment('')
            ->withUserInfo('user', 'abc?def');

work perfectly with GuzzleHttp\Psr7\Uri

[scaytrase]

I think there is a problem in Slim Uri. There is a difference in processing userInfo via constructor and via withUserInfo.

__construct https://github.com/slimphp/Slim-Psr7/blob/5dbd98f5a0194db8aeee4da3441e56a1d816360b/src/Uri.php#L105-L106

        $this->user = $user;
        $this->password = $password;

withUserInfo https://github.com/slimphp/Slim-Psr7/blob/5dbd98f5a0194db8aeee4da3441e56a1d816360b/src/Uri.php#L185-L200

    /**
     * {@inheritdoc}
     */
    public function withUserInfo($user, $password = null)
    {
        $clone = clone $this;
        $clone->user = $this->filterUserInfo($user);

        if ($clone->user !== '') {
            $clone->password = $this->filterUserInfo($password);
        } else {
            $clone->password = '';
        }

        return $clone;
    }

[scaytrase]

So like this

        $uri = (new \Slim\Psr7\Uri  ('https', 'example.com', null, '/empty' . '', ''))
            ->withUserInfo('user', 'abc?def');

test also passes

[lletourn]

I already opend an issue with slim. https://github.com/slimphp/Slim-Psr7/issues/181

My problem is with PSR7 it's not clear if it should be encoded or not. If it's allowed to not be encoded then PathFinder should deal with that case too.

If it's not allowed then it's just a slim bug. This is unclear.

[scaytrase]

From my PoV it'a slim bug just because it depends on how you instantiate the Slim Uri. If you pass values to constructor - test fails. If you pass then to withUserInfo - test passes. This should be either documented on slim side (disallow use a constructor and stick to PSR-7 interface, which works good as well as Guzzle's one) or fixed to have consistent behaviour (maybe it's not done because considered BC break)

[lletourn]

I'm ok with this. I agree that slim being inconsistent, even if PSR7 is unclear, complicates things.

Thank you for the followup.

[scaytrase]

You're welcome