zendframework / zend-uri

Uri component from Zend Framework
BSD 3-Clause "New" or "Revised" License
117 stars 24 forks source link

URI with ending colon without port #34

Closed mtagliab closed 5 years ago

mtagliab commented 5 years ago

Based on issue #33

Previous behaviour

$sourceUri = "http://www.example.com:";
$uri = new \Zend\Uri\Http($sourceUri);

echo 'Scheme: '.$uri->getScheme().PHP_EOL;
echo 'Host: '.$uri->getHost().PHP_EOL;
echo 'Port: '.$uri->getPort().PHP_EOL;
echo 'Path: '.$uri->getPath().PHP_EOL;

Which gives the following result:

Scheme: http
Host: www.example.com:
Port: 
Path: /

New behaviour With this fix, the new result will be:

Scheme: http
Host: www.example.com
Port: 
Path: /

which matches the result of parse_url($uri).

I also added a new test for this case.

mtagliab commented 5 years ago

@webimpress I fixed the error with the last commit. I did all the work on a test machine, but couldn't commit from there, and I rewrote the modifications on web. During the process, I somehow forgot to remove the setPort outside the check, causing the test to fail. Should be correct now

michalbundyra commented 5 years ago

@mtagliab Thanks, I'll have a look on it shortly.

michalbundyra commented 5 years ago

@mtagliab Also, the target branch should be master, not develop. Could you rebase it and change the target, please?

mtagliab commented 5 years ago

@webimpress I never rebased a repo. What should I do, exactly?

michalbundyra commented 5 years ago

@mtagliab

I never rebased a repo. What should I do, exactly?

git rebase origin/master and then push force. On github you can edit PR and change the target from develop to master. If you have any issues I can do it on merge, not a problem. Thanks.

mtagliab commented 5 years ago

@webimpress could it be merged now?

michalbundyra commented 5 years ago

@mtagliab I need to review first, will try to do it today

michalbundyra commented 5 years ago

Thanks, @mtagliab!