fix($resource): allow params in hostname (except for IPv6 addresses)

[gkalpak]

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix (regression).

What is the current behavior? (You can also link to an open issue here)
Params inside the hostname part of a URL are ignored.
See #14542.

What is the new behavior (if this is a feature change)?
Params inside the hostname part of a URL are not ignored (except if the hostname is a [...] enclosed IPv6 address).

Does this PR introduce a breaking change?
Hardly.

Please check if the PR fulfills these requirements

• The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

Other information:
Support for IPv6 addresses (in b643f0d) was too aggressive and broke support for params in the
hostname part of a URL.
This commit restores support for params in the hostname, as long as it is not an IPv6 address.

Fixes #14542

[Narretz]

Didn't Pete comment on this?
Anyway, my regex foo isn't very strong ...
So inside Ipv6 urls, you cannot make param replacements. But can you put ipv6 urls in params that replace the host?

[gkalpak]

@Narretz: Pete asked for a clarification, but didn't comment further iirc 😟

But can you put ipv6 urls in params that replace the host?

Good question. You can't actually, because we are encoding the leading/trailing [/], so the URL breaks.

There is also a more general bug (that could be a feature, but I don't think it is), that :param inside of parameter values are also replaced:

$resource('http://:foo/:bar').get({foo: ':bar.com', bar: 'baz'});
//  --> GET http://baz.com/baz

This shouldn't be a problem usually, but it may affect IPv6 URLs as param values.

I will take another look at how we can address these issues.

[petebacondarwin]

Any update on this one @gkalpak ?

[gkalpak]

Not yet. Will work on it this week.

[gkalpak]

A couple of thoughts after having looked into it again:

Currently, we have the following limitations:

1. There can be no param replacements inside a static IPv6 URL. E.g. $resource('http://[2620:0:861:ed1a::1]').get({ed1a: 'foo'}) will call http://[2620:0:861:ed1a::1].
   AFAICT, this is a good thing, since you don't want accidental replacements and IPv6 URLs are full of :xyz (which look like params).
2. While you can set the hostname via a param (e.g. $resource('http://:hostname')), you can't set it to an IPv6 URL, because the quare brackets ([...]) will be URI-encoded.
   This is minor imo, but we might be able to address it.
3. A :xyz inside a param value may be treated as a param itself and replaced (depending on the relative order of params). E.g. $resource('http://:foo/:bar').get({foo: ':bar.com', bar: 'baz'}) will call http://baz.com/baz.
   This could also be a feature, and "fixing" it might break apps relying on it. On the other hand it was never documented (not intended probably) and is not hard to do manually. It doesn't currently affect IPv6 URLs, due to (2), but if (2) were to be addressed, this would become more of a concern.

[petebacondarwin]

Can we interleave these tests so that the associated bits are near to each other?

E.g.

var R1 = $resource('http://[2620:0:861:ed1a::1]', null, null, keepSlashes);
$httpBackend.expect('GET', 'http://[2620:0:861:ed1a::1]').respond(null);
R1.get({ed1a: 'foo'});

var R2 = $resource('http://[2620:0:861:ed1a::1]/', null, null, keepSlashes);
$httpBackend.expect('GET', 'http://[2620:0:861:ed1a::1]/').respond(null);
R2.get({ed1a: 'foo'});

and so on?

[petebacondarwin]

And similarly for the other new test?

[petebacondarwin]

I think this is good enough for now. Being able to insert non-IPv6 hosts via parameters is better than we had before. If someone then asks for that support we can look at it again.
The whole thing about inserting parameters into parameters has not been a problem so far, so I don't think we should try to fix that either, right now.

So let's get this PR merged. Thanks @gkalpak