Parse domain + port as path

[kelunik]

Currently the following test fails to resolve correctly:

Base: 'http://localhost/1/2/a.php'
Resolve: 'a.com:80'
Expected result: 'http://localhost/1/2/a.com'

PHP currently parses domain or IP + port as URI without scheme and not as path.

[kelunik]

In fact this should probably error? @daverandom

[rdlowrey]

This appears to be the correct functionality according to RFC3986: http://tools.ietf.org/html/rfc3986#section-5.2.2

 For each URI reference (R), the following pseudocode describes an
   algorithm for transforming R into its target URI (T):

      -- The URI reference is parsed into the five URI components
      --
      (R.scheme, R.authority, R.path, R.query, R.fragment) = parse(R);

      -- A non-strict parser may ignore a scheme in the reference
      -- if it is identical to the base URI's scheme.
      --
      if ((not strict) and (R.scheme == Base.scheme)) then
         undefine(R.scheme);
      endif;

      if defined(R.scheme) then
         T.scheme    = R.scheme;
         T.authority = R.authority;
         T.path      = remove_dot_segments(R.path);
         T.query     = R.query;
      else
         if defined(R.authority) then
            T.authority = R.authority;
            T.path      = remove_dot_segments(R.path);
            T.query     = R.query;
         else
            ...

Because an authority exists (a.com) for the parsing of the URI parts in R, no further modification is necessary and the end result of the resolution must be a.com:80. This appears to match the current resolve() behavior minus the default port 80.

Am I reading that wrong?

[kelunik]

The issue is that a.com:80 should be considered a path, not an authority. It requires // to be considered an authority.

[rdlowrey]

Ah I see

[rdlowrey]

So the root issue is the permissiveness of parse_url() in the first place ... replacing that with parsing logic should solve the problem ... will work on it

[kelunik]

Exactly.

[kelunik]

We should internally upgrade to https://github.com/thephpleague/uri-parser.

[DaveRandom]

I'm all for reducing code duplication, as long as that implementation actually works correctly.

[kelunik]

We probably won't fix this package. I tried it, it'll break BC in some cases.

[duronrulez]

@kelunik is there any plan to upgrade to thephpleague package? There are a few places where artax depends on amphp/uri

[kelunik]

@duronrulez We probably won't upgrade amphp/artax, but it's successor amphp/http-client does no longer depend on amphp/uri. We'll have a final 4.0.0 release soon, bringing many features, such as easier customization and HTTP/2.

[duronrulez]

oooh thats perfect, i didnt know there is a successor. I'll start upgrading immediately.

Thank you