fix(nodejs): include auth in same domain redirects #8437
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
I've seen the CI fails are also failing in some jobs in master also.... I also don't see how they could be related with my changes. Even though if I can be of any help in fixing them let me know. |
|
@harsha509 could you please help me to review this one? |
|
Hi @diemol , PR looks good to me! we can merge it. Regards, |
|
Thanks for reviewing @harsha509! |
|
Thanks you both @diemol and @harsha509! |
titusfortner
pushed a commit
to titusfortner/selenium
that referenced
this pull request
Aug 13, 2020
Co-authored-by: Diego Molina <diemol@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Authorization information is not being sent in same domain redirects.
Motivation and Context
TLDR; we need to send redirects in session creation with the authorization sent in the original post command.
We have long running commands (specially session creation) that may last for several minutes in our hub. Our hub is in AWS infrastructure and also under a bunch of reverse proxies and we don't (can't) control request timeouts in all those pieces (i.e. NLB has a hardcoded 350s timeout for requests).
Our hub is behind also a basic auth security proxy, so we need to provider that information in all requests.
We have implemented a redirect logic (sending 303 redirects is commands are not resolved in for example 120s) so that we can extend the command duration.
Additionally, to detect if the client dropped the connection and clear the browser "almost" immediatelly after it's provisioned, we always send a redirect before giving the session (so that if he doesn't come back for the session values we know that he has dropped the waitting)
Types of changes
Checklist