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

redirect: skip URL encoding for host names #1762

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@salshaaban
Contributor

salshaaban commented Aug 11, 2017

This fixes redirects to IDN URLs

Bug: #1441
Reported by: David Lord

redirect: skip URL encoding for host names
This fixes redirects to IDN URLs

Bug: #1441
Reported by: David Lord
@mention-bot

This comment has been minimized.

mention-bot commented Aug 11, 2017

@salshaaban, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bagder, @dfandrich and @yangtse to be potential reviewers.

@coveralls

This comment has been minimized.

coveralls commented Aug 11, 2017

Coverage Status

Coverage increased (+0.05%) to 75.125% when pulling 636a62b on salshaaban:IDN into 77cd4e7 on curl:master.

@bagder bagder added HTTP URL labels Aug 11, 2017

@bagder

This comment has been minimized.

Member

bagder commented Aug 11, 2017

Thanks, I'm working on writing up a test for this. In fact, I already did but the problem now is that the test uses a hard-coded port number in the HTTP response header and that's not how we like to do things, so I need to come up with a solution to that ...

@salshaaban

This comment has been minimized.

Contributor

salshaaban commented Aug 12, 2017

I'm working on writing up a test for this. In fact, I already did

I should've done that myself. Sorry!
I will try to think of something too.

@bagder

This comment has been minimized.

Member

bagder commented Aug 12, 2017

check out test1448 (remove the .txt extension)

@bagder

This comment has been minimized.

Member

bagder commented Aug 12, 2017

Aha! I figured it out. I can use --connect-to, to redirect that port 8990 to the port it is actually on! ... I'll post a separate PR to see what the CI says.

@bagder

This comment has been minimized.

Member

bagder commented Aug 12, 2017

That's now in #1772

@bagder bagder closed this in d6ecb2c Aug 12, 2017

@bagder

This comment has been minimized.

Member

bagder commented Aug 12, 2017

Thanks!

@salshaaban salshaaban deleted the salshaaban:IDN branch Aug 13, 2017

@lock lock bot locked as resolved and limited conversation to collaborators May 21, 2018

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