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

fix($location): add semicolon to whitelist of delimiters to unencode #8377

Closed
wants to merge 1 commit into from

Conversation

jeffbcross
Copy link
Contributor

Some servers require characters within path segments to contain semicolons,
such as /;jsessionid=foo in order to work correctly. RFC-3986 includes
semicolons as acceptable sub-delimiters inside of path and query, but $location
currently encodes semicolons. This can cause an infinite digest to occur since $location
is comparing the internal semicolon-encoded url with the semicolon-unencoded url returned
from window.location.href, causing Angular to believe the url is changing with each digest
loop.

This fix adds ";" to the list of characters to unencode after encoding queries or path segments.

Closes #5019

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#8377)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

});

inject(function($location, $browser, $rootScope) {
$rootScope.$digest();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expect(fn).not.toThrow() maybe? (so that what is being asserted is more clear)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call

@jeffbcross
Copy link
Contributor Author

Also should close #6140

@jeffbcross
Copy link
Contributor Author

Also close #4067

@jeffbcross
Copy link
Contributor Author

Actually should not close #6140 since that issue is recommending an API change to make query param delimiter configurable.

return b;
};
});
var self = this;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so I'm not sure what we're using self for here, I'd remove that --- but with tests passing it looks good to me

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 - I suspect this line is left over from something else

@petebacondarwin
Copy link
Contributor

LGTM

Some servers require characters within path segments to contain semicolons,
such as `/;jsessionid=foo` in order to work correctly. RFC-3986 includes
semicolons as acceptable sub-delimiters inside of path and query, but $location
currently encodes semicolons. This can cause an infinite digest to occur since $location
is comparing the internal semicolon-encoded url with the semicolon-unencoded url returned
from window.location.href, causing Angular to believe the url is changing with each digest
loop.

This fix adds ";" to the list of characters to unencode after encoding queries or path segments.

Closes angular#5019
@jeffbcross
Copy link
Contributor Author

Landed 3625803

@nicholasserra
Copy link
Contributor

So running into the opposite issue of this, from the $http module, and I'm wondering if the issue you were having @jeffbcross was specific to the $location module. Currently looking into making the semicolon encoding toggle-able, or some kind of global flag.

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

Successfully merging this pull request may close these issues.

5 participants