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

Bug fix: thirdparty site cookie leak #73

Merged
merged 3 commits into from Jan 25, 2022
Merged

Conversation

ranjit-git
Copy link
Contributor

@ranjit-git
Copy link
Contributor Author

Plz check this if Patch is working

Copy link
Owner

@feross feross left a comment

Choose a reason for hiding this comment

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

This only works for the first request because you're putting the flag and original_host variables in the module scope (at the top). Please fix.

Also, please run npm test to ensure you're matching the existing code style.

@ranjit-git
Copy link
Contributor Author

I will fix the code style .

This only works for the first request because you're putting the flag and original_host variables in the module scope (at the top)

I have check this and it working for all request .
First let me clearify ,what i checking
http://mysite.com --->302--> http://redirect_1.com/ --> 302--> http://redirect_2.com/
Here i always checking mysite.com with redirect_1.com and mysite.com with redirect_2.com . if redirect host is different then dont send cookie .
So, when i set url like http://mysite.com/redirect.php?url=http://redirect_1.com/redirect.php?url=http://redirect_2.com then it does not send cookie to redirect_1.com,redirect_2.com .

@ranjit-git
Copy link
Contributor Author

ranjit-git commented Jan 18, 2022

So, basically I checking with provided host with all redirect host and thats why i uses flag varriable .
There is another way
Check mysite.com with redirect_1.com and if again redirect found then check redirect_1.com with redirect_2.com and so on . And it does not required above flag varriable .
Plz let me know your idea

@ranjit-git
Copy link
Contributor Author

i think you want the second way
like http://mysite.com -->302--> http://redirect_1.com -->302--> http://redirect_2.com
it should check mysite.com with redirect_1.com if again redirect found then check redirect_1.com with redirect_2.com and so on. right?
I will change PR to this method

Copy link
Contributor Author

@ranjit-git ranjit-git left a comment

Choose a reason for hiding this comment

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

Hi, i just made a changes

@feross feross merged commit e4af095 into feross:master Jan 25, 2022
SimonGodefroid added a commit to SimonGodefroid/prebuild-install that referenced this pull request Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants