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

fix($location): support IPv6 addresses #2950

Closed

Conversation

chrisirhc
Copy link
Contributor

Fixes #2864

@petebacondarwin
Copy link
Contributor

PR Checklist (Minor Bugfix)

  • Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name)
  • PR doesn't introduce new api
  • PR doesn't contain a breaking change
  • PR contains unit tests
  • PR contains e2e tests (if suitable)
  • PR contains documentation update (if suitable)
  • PR passes all tests on Travis (sanity)
  • PR passes all tests on ci.angularjs.org (cross-browser compatibility)
  • PR is rebased against recent master
  • PR is squashed into one commit per logical change
  • PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
  • All changes requested in review have been implemented

@chrisirhc
Copy link
Contributor Author

Just signed the CLA as Kien Chuan Chua.

@chrisirhc
Copy link
Contributor Author

Made regex stricter and added more tests for valid IPv6 addresses.

@jeffbcross
Copy link
Contributor

Thanks for the PR. @mhevery and I are looking at it, and are going to squash in some changes to the $location service to fall back on a private urlUtils service that falls back on browsers to parse URLs instead of RegEx.

@ghost ghost assigned jeffbcross and mhevery Jul 24, 2013
@chrisirhc
Copy link
Contributor Author

Sounds like a better way to go. Previously, I've used the HTMLAnchorElement
for URL handling.
On Jul 24, 2013 1:14 PM, "Jeff Cross" [email protected] wrote:

Thanks for the PR. @mhevery https://github.com/mhevery and I are
looking at it, and are going to squash in some changes to the $location
service to fall back on a private urlUtils service that falls back on
browsers to parse URLs instead of RegEx.


Reply to this email directly or view it on GitHubhttps://github.com//pull/2950#issuecomment-21512610
.

@jeffbcross
Copy link
Contributor

That's the approach $$UrlUtils uses as well.

@chrisirhc
Copy link
Contributor Author

Sounds great. 👍 This PR would be unnecessary then.

@chrisirhc chrisirhc closed this Jul 24, 2013
@jeffbcross jeffbcross reopened this Jul 24, 2013
@jeffbcross
Copy link
Contributor

The tests are useful.

@jeffbcross
Copy link
Contributor

I moved the refactoring effort into a new PR (#3533) since the effort was significant enough not to be amended to this commit. Closing.

@jeffbcross jeffbcross closed this Aug 9, 2013
IgorMinar pushed a commit to IgorMinar/angular.js that referenced this pull request Oct 10, 2013
The location service, and other portions of the application,
were relying on a complicated regular expression to get parts of a URL.
But there is already a private urlUtils provider,
which relies on HTMLAnchorElement to provide this information,
and is suitable for most cases.

In order to make urlUtils more accessible in the absence of DI,
its methods were converted to standalone functions available globally.

The urlUtils.resolve method was renamed urlResolve,
and was refactored to only take 1 argument, url,
and not the 2nd "parse" boolean.
The method now always returns a parsed url.
All places in code which previously wanted a string instead of a parsed
url can now get the value from the href property of the returned object.

Tests were also added to ensure IPv6 addresses were handled correctly.

Closes angular#3533
Closes angular#2950
Closes angular#3249
@chrisirhc chrisirhc deleted the feature/support-ipv6-addresses branch December 17, 2013 00:52
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
The location service, and other portions of the application,
were relying on a complicated regular expression to get parts of a URL.
But there is already a private urlUtils provider,
which relies on HTMLAnchorElement to provide this information,
and is suitable for most cases.

In order to make urlUtils more accessible in the absence of DI,
its methods were converted to standalone functions available globally.

The urlUtils.resolve method was renamed urlResolve,
and was refactored to only take 1 argument, url,
and not the 2nd "parse" boolean.
The method now always returns a parsed url.
All places in code which previously wanted a string instead of a parsed
url can now get the value from the href property of the returned object.

Tests were also added to ensure IPv6 addresses were handled correctly.

Closes angular#3533
Closes angular#2950
Closes angular#3249
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
The location service, and other portions of the application,
were relying on a complicated regular expression to get parts of a URL.
But there is already a private urlUtils provider,
which relies on HTMLAnchorElement to provide this information,
and is suitable for most cases.

In order to make urlUtils more accessible in the absence of DI,
its methods were converted to standalone functions available globally.

The urlUtils.resolve method was renamed urlResolve,
and was refactored to only take 1 argument, url,
and not the 2nd "parse" boolean.
The method now always returns a parsed url.
All places in code which previously wanted a string instead of a parsed
url can now get the value from the href property of the returned object.

Tests were also added to ensure IPv6 addresses were handled correctly.

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

Successfully merging this pull request may close these issues.

angular breaks with ipv6 location
4 participants