Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

ngResource does not replace parameters in the host #14542

Closed
bluesliverx opened this issue Apr 29, 2016 · 2 comments
Closed

ngResource does not replace parameters in the host #14542

bluesliverx opened this issue Apr 29, 2016 · 2 comments

Comments

@bluesliverx
Copy link

bluesliverx commented Apr 29, 2016

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
When you use a parameter (i.e. :domain) in the hostname, it is not replaced correctly. Instead, it is kept as the parameter name with the colon intact.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).
Here is the broken plunkr using 1.4.5: http://plnkr.co/edit/9m0b2er2P6Gb3Jx9Bppo?p=preview
Here is the working version using 1.4.3: http://plnkr.co/edit/RsI3TgKwcjGEXcTMKoQR?p=preview

What is the expected behavior?
It should replace any parameters in the domain, ignoring IPv6 hosts and ports correctly.

What is the motivation / use case for changing the behavior?
This behavior was standard in ngResource 1.4.3. As of 1.4.5, it changed in order to support IPv6 URLs (see #12512). The fix implemented in that issue was too aggressive and effectively ignores hostnames/domains from parameter replacement.

I make use of this behavior heavily in order to support a multi-tenanted application where the the sub-domain decides the tenant. I am not the only one either: http://stackoverflow.com/questions/32463584/passing-domain-name-as-parameter-with-resource-in-angularjs#

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.
1.4.5 is where the issue first appeared (I didn't see a 1.4.4 of ngResource), and 1.4.3 worked correctly as described above.

Other information (e.g. stacktraces, related issues, suggestions how to fix)
I think if we can simply add a regex to catch IPv6 addresses and for all other cases keep the previous behavior, including the same handling of port numbers (if the param contains only numbers, ignore it). I'm not sure of IPv6 syntax, but in the examples given in the linked issue, something as simple as looking for square brackets around the hostname may catch it.

@Narretz
Copy link
Contributor

Narretz commented May 9, 2016

I'm not sure replacing the host was ever officially supported. At least there are no mentions for this in the docs. Of course you are welcome to open a PR with the proposed changes and we can see if it makes sense.

I also can't see a difference between the plnkrs you posted. Both throw an exception for me. Looks like it doesn't show the url in Firefox, but it does in Chrome. As a general guideline, you should also reduce your demos so that only the relevant parts remain (I see ui-view and factory abstractions which seem unrelated).

@petebacondarwin
Copy link
Member

Looks like a regression. Let's fix this. Do you want to take a look @Narretz if @lgalfaso doesn't see to it first?

@petebacondarwin petebacondarwin modified the milestones: 1.4.x, Ice Box, 1.5.x May 10, 2016
@Narretz Narretz modified the milestones: 1.5.7, 1.5.x May 27, 2016
@gkalpak gkalpak modified the milestones: 1.5.7, 1.5.8 Jun 15, 2016
@gkalpak gkalpak self-assigned this Jul 13, 2016
gkalpak added a commit to gkalpak/angular.js that referenced this issue Jul 13, 2016
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 angular#14542
@petebacondarwin petebacondarwin modified the milestones: 1.5.8, 1.5.9 Jul 22, 2016
gkalpak added a commit to gkalpak/angular.js that referenced this issue Oct 13, 2016
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 angular#14542
@petebacondarwin petebacondarwin modified the milestones: 1.5.9, 1.5.10 Nov 24, 2016
gkalpak added a commit to gkalpak/angular.js that referenced this issue Dec 2, 2016
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 angular#14542
@gkalpak gkalpak closed this as completed in 752b1e6 Dec 3, 2016
gkalpak added a commit that referenced this issue Dec 3, 2016
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

Closes #14906
ellimist pushed a commit to ellimist/angular.js that referenced this issue Mar 15, 2017
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 angular#14542

Closes angular#14906
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants