Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes autodiscover max redirection from multi-origin #541

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

songyiyang
Copy link

When the first to-be-discovered url returning a response type of redirection, it will increase the currentHop until it reaches AutodiscoverMaxRedirections, then if next to-be-discovered url (not from redirection) has a redirection url, the new redirection url will not get through the system since the current hop has reached max, this fix will make a copy from the original urls from getAutodiscoverServiceUrls and check if the new to-be-discovered url is from this origin, then it will reset current hop if so.
Aslo, changed the urls.add method to urls.set for not increasing the urls' list size with a lot of dummy urls ( e.g. if detecting a new redirect url, it will add the redirect target to current index and continue with the current index, however the old url will be shifted to next index so when the next index of url being discovered, it will do the same loop until hitting the AutodiscoverMaxRedirections), this will decrease the number of requests that fetching unnecessary urls and increase performance.

@songyiyang songyiyang changed the title Fixes autodiscover max redirection msdn Fixes autodiscover max redirection Jun 29, 2016
@songyiyang songyiyang changed the title Fixes autodiscover max redirection Fixes autodiscover max redirection from multi-origin Jun 29, 2016
@codecov-io
Copy link

Current coverage is 10.43%

No coverage report found for master at 7f8793a.

Powered by Codecov. Last updated by 7f8793a...4f854c8

@serious6
Copy link
Member

Please fix codecov check and provide unit-tests.

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

Successfully merging this pull request may close these issues.

None yet

5 participants