Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
Work aroundparse_url() bug (bis)#58836
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
6c067aa to5e3509bCompareUh oh!
There was an error while loading.Please reload this page.
parse_url() bug (bis)0cbc0dd to6cd3610ComparePR ready. I figured out a way to fix#58313 |
| $scheme = $parts['scheme'] ?? null; | ||
| $host = $parts['host'] ?? null; | ||
| if (!$scheme && $host && !str_starts_with($url, '//')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I wondered if we could add the same logic to Request::create() but we have a test case that ensureRequest::create("test.com:80") is parsed as host+port (which is not how the URL spec would parse it)
| @@ -416,7 +417,7 @@ private static function createRedirectResolver(array $options, string $host, ?ar | |||
| [$host, $port] = self::parseHostPort($url, $info); | |||
| if (false !== (parse_url($location.'#', \PHP_URL_HOST) ?? false)) { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
stop using parse_url on user-input
| $options['max_redirects'] = curl_getinfo($ch, \CURLINFO_REDIRECT_COUNT); | ||
| curl_setopt($ch, \CURLOPT_FOLLOWLOCATION, false); | ||
| curl_setopt($ch, \CURLOPT_MAXREDIRS, $options['max_redirects']); | ||
| } else { | ||
| $url = parse_url($location ?? ':'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
stop using parse_url on user-input here also
| unset($server['HTTPS']); | ||
| $server['SERVER_PORT'] = 80; | ||
| } else { | ||
| throw new BadRequestException('Invalid URI: http(s) scheme expected.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
looks like a missing check here,Request::create('random-scheme:foo') cannot give port 80
6cd3610 to80257eaCompare4b8695c intosymfony:5.4Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This PR is a follow up of#58218
parse_url behaves incorrectly when parsing some URLs that don't contain
?or#.This PR ensures that one of those chars is always found when calling the function.