thephpleague / uri-src

URI manipulation Library
https://uri.thephpleague.com
MIT License
30 stars 7 forks source link

$uri->withQuery(Query::createFromUri($uri)) can fail #4

Closed arnaud-lb closed 2 years ago

arnaud-lb commented 2 years ago

Bug Report

Information Description
Version 6.5.0
PHP version 8.1
OS Platform Linux

Summary

Uri::withQuery() does not accept the URI's original query string after is has been parsed with Query::createFromUri().

Standalone code, or other way to reproduce the problem

$url = \League\Uri\Uri::createFromString("http://example.com/?foo=" . urlencode("\v\xED"));
$url = $url->withQuery(\League\Uri\Components\Query::createFromUri($url));
echo $url->toString(), "\n";

Expected result

http://example.com/?foo=%0B%ED

Actual result

League\Uri\Exceptions\SyntaxError: The component `foo=
                                                      �` contains invalid characters. in vendor/league/uri/src/Uri.php:1276
Stack trace:
#0 vendor/league/uri/src/Uri.php(1369): League\Uri\Uri->filterString('foo=\v\xED')
thephpleague/uri#1 test.php(49): League\Uri\Uri->withQuery(Object(League\Uri\Components\Query))
nyamsprod commented 2 years ago

@arnaud-lb thanks for the report. I will investigate it a bit further as this as more implications. Might be a bug or not 🤔 . Either way I already took a note of the issue and found the root cause. The main issue is not on the URI package but on the URI components one. But I will leave the issue open here and not transfert it to the other package.

nyamsprod commented 2 years ago

@arnaud-lb a fix has landed on the uri-components package. Please review it and if all is good I will publish the fix next week

arnaud-lb commented 2 years ago

Awesome, thank you!

The issue also exists in pair names, e.g.

$url = \League\Uri\Uri::createFromString("http://example.com/?".urlencode("\v\xED")."=" . urlencode("\v\xED"));
$url = $url->withQuery(\League\Uri\Components\Query::createFromUri($url));
echo $url->toString(), "\n";

can also fail.

Otherwise, I confirm that this fixes the original issue :)

nyamsprod commented 2 years ago

Will have to look at the pair name then ... issue still not totally fixed but I guess the same fix should be applied there 😉 Once the named are fixed and the patch updated I will let you know

nyamsprod commented 2 years ago

@arnaud-lb you are lucky the pair name is now also patch 🎉

arnaud-lb commented 2 years ago

I confirm :tada:

Thank you !

nyamsprod commented 2 years ago

league/uri-components:2.4.1 is released with the fix 🔥

nyamsprod commented 2 years ago

@arnaud-lb as a side note currently uri-components requires PHP7.4 at least while league/uri still supports PHP7.3 when I get time I will drop support for PHP7.3 and maybe PHP7 as the whole. I know it is not directly related to your issue but keep that in mind 😉