Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Uri::__toString() can yield malformed URIs #172

Open
TimWolla opened this issue Oct 17, 2023 · 3 comments · May be fixed by #196
Open

Uri::__toString() can yield malformed URIs #172

TimWolla opened this issue Oct 17, 2023 · 3 comments · May be fixed by #196
Labels
Bug Something isn't working

Comments

@TimWolla
Copy link
Contributor

Bug Report

Q A
Version(s) Current git 3.3.x

Summary

The Uri class is able to parse malformed URIs but this results in Uri::__toString() generating a malformed URI. Attempting to pass that URI back into Uri will yield a InvalidArgumentException.

Current behavior

Certain malformed URIs do not round-trip through \Laminas\Diactoros\Uri.

How to reproduce

<?php

require('vendor/autoload.php');

$value = 'http://invalid:%20https://example.com';
$uri = new \Laminas\Diactoros\Uri($value);
$uri2 = new \Laminas\Diactoros\Uri($uri->__toString()); // Exception is thrown here.

Expected behavior

Either both constructors throw, or neither.

Note: This issue also exists in guzzlehttp/psr7 and was reported at guzzle/psr7#583.
@TimWolla TimWolla added the Bug Something isn't working label Oct 17, 2023
@TimWolla TimWolla changed the title Uri::__toString() can yield malformed URIs Uri::__toString() can yield malformed URIs Oct 17, 2023
@froschdesign
Copy link
Member 

$value = 'http://invalid:%20https://example.com';

PHP's function parse_url can not handle this wrong URL: https://www.php.net/manual/function.parse-url.php#refsect1-function.parse-url-notes

array(3) {
  'scheme' =>
  string(4) "http"
  'host' =>
  string(16) "invalid:%20https"
  'path' =>
  string(13) "//example.com"
}

laminas-diactoros/src/Uri.php

Lines 80 to 87 in 4c6ae00

public function __construct(string $uri = '')
{
if ('' === $uri) {
return;
}
$this->parseUri($uri);
}

laminas-diactoros/src/Uri.php

Lines 384 to 392 in 4c6ae00

private function parseUri(string $uri): void
{
$parts = parse_url($uri);
if (false === $parts) {
throw new Exception\InvalidArgumentException(
'The source URI string appears to be malformed'
);
}

@boesing
Copy link
Member

boesing commented Oct 17, 2023

Thanks, @TimWolla for cross posting.

Lets see how guzzle will handle this, after my latest hassle with php-http/discovery I would prefer to keep this in-sync.
We might also want to cross-post this to https://github.com/php-http/psr7-integration-tests (which is used by diactoros as well).

@Xerkus Xerkus added this to the 3.4.0 milestone Sep 11, 2024
@Xerkus Xerkus linked a pull request Sep 11, 2024 that will close this issue
Add basic host validation in Uri and add port validation to the Uri constructor #196
Draft
@Xerkus
Copy link
Member

Xerkus commented Sep 11, 2024

I introduce basic host validation with the linked PR but anything more comprehensive would need to be done via integration tests.

The changes I introduced need to be elevated to the integration tests as well to ensure it is handled across all implementations.

@Xerkus Xerkus modified the milestones: 3.4.0, 3.5.0 Sep 11, 2024
@gsteel gsteel removed this from the 3.5.0 milestone Oct 14, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
Bug Something isn't working
Projects
None yet
Milestone
No milestone
5 participants
@froschdesign @TimWolla @Xerkus @boesing @gsteel