search

amphp / http-client

An advanced async HTTP client library for PHP, enabling efficient, non-blocking, and concurrent requests and responses.
https://amphp.org/http-client
MIT License
706 stars 66 forks source link

Error if request URI provides relative path #269

Closed remorhaz closed 4 years ago

remorhaz commented 4 years ago

The build is failing but it seems to have been failing for some time. Tests are segfaulting for some reason under 7.2 even locally (I failed to understand why) but run okay on 7.3 and 7.4. I've re-checked that at least my tests are running okay on all three versions.

kelunik commented 4 years ago

If you rebase onto master or merge master, the tests should be working fine again. I've skipped the tests causing segfaults on PHP 7.2 and 7.3.

remorhaz commented 4 years ago

The same check needs to be done in Http2Connection, should we put the check into DefaultConnectionFactory or duplicate it into Http2Connection?

Hmm, I need to take a closer look...

remorhaz commented 4 years ago

should we put the check into DefaultConnectionFactory or duplicate it into Http2Connection?

I think it's wise to make checks just before contract protects us; and in our case contract of UriInterface doesn't protect us from this particular problem, so I vote for duplication. Relying on DefaultConnectionFactory is out of contact at all, I believe that code shouldn't rely on some checks made by external code.

remorhaz commented 4 years ago

Seems that some tests are flaky.

kelunik commented 4 years ago

Thanks!