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

Composer should not send authorization header when following redirect #6717

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@maartenba
Contributor

maartenba commented Oct 2, 2017

Fixes the issue described in #6716

@dulfe

This comment has been minimized.

Show comment
Hide comment
@dulfe

dulfe Oct 11, 2017

+1
Completely fixed the issue I had with authorization headers... thank you!

dulfe commented Oct 11, 2017

+1
Completely fixed the issue I had with authorization headers... thank you!

@alcohol

alcohol approved these changes Nov 2, 2017

@Seldaek

This comment has been minimized.

Show comment
Hide comment
@Seldaek

Seldaek Nov 29, 2017

Member

Merged as 128e424 and then I realized we actually can do it much simpler by using the correct host to check for credentials at every request instead of re-using the originUrl .. 743153e

Hopefully this won't break anything though, it's kinda sensitive code.

Member

Seldaek commented Nov 29, 2017

Merged as 128e424 and then I realized we actually can do it much simpler by using the correct host to check for credentials at every request instead of re-using the originUrl .. 743153e

Hopefully this won't break anything though, it's kinda sensitive code.

@Seldaek Seldaek closed this Nov 29, 2017

@Seldaek

This comment has been minimized.

Show comment
Hide comment
@Seldaek

Seldaek Nov 29, 2017

Member

And FWIW @maartenba @dulfe I'd appreciate if you can try it out with the latest composer snapshot.

Member

Seldaek commented Nov 29, 2017

And FWIW @maartenba @dulfe I'd appreciate if you can try it out with the latest composer snapshot.

@Seldaek Seldaek added this to the 1.5 milestone Nov 29, 2017

@maartenba

This comment has been minimized.

Show comment
Hide comment
@maartenba

maartenba Nov 29, 2017

Contributor

@Seldaek do you happen to have the .phar somewhere?

Contributor

maartenba commented Nov 29, 2017

@Seldaek do you happen to have the .phar somewhere?

@alcohol

This comment has been minimized.

Show comment
Hide comment
@alcohol

alcohol Nov 29, 2017

Member

composer self-update --snapshot

Member

alcohol commented Nov 29, 2017

composer self-update --snapshot

@dulfe

This comment has been minimized.

Show comment
Hide comment
@dulfe

dulfe Nov 29, 2017

@Seldaek, it worked great. (I used @alcohol command to download the snapshot)

dulfe commented Nov 29, 2017

@Seldaek, it worked great. (I used @alcohol command to download the snapshot)

@maartenba

This comment has been minimized.

Show comment
Hide comment
@maartenba

maartenba Nov 29, 2017

Contributor

Verified it as well, works just fine. Thanks @Seldaek !

Contributor

maartenba commented Nov 29, 2017

Verified it as well, works just fine. Thanks @Seldaek !

@Seldaek

This comment has been minimized.

Show comment
Hide comment
@Seldaek

Seldaek Nov 29, 2017

Member

Alright great thanks

Member

Seldaek commented Nov 29, 2017

Alright great thanks

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