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

Refactor/redirector #111

Merged
merged 4 commits into from
Mar 23, 2014
Merged

Refactor/redirector #111

merged 4 commits into from
Mar 23, 2014

Conversation

ixti
Copy link
Member

@ixti ixti commented Mar 23, 2014

Closes #94

This preparation will meke further redirector 100% backward compatible without
any unexpected "surprises".
tarcieri added a commit that referenced this pull request Mar 23, 2014
@tarcieri tarcieri merged commit 05d10c5 into master Mar 23, 2014
@tarcieri tarcieri deleted the refactor/redirector branch March 23, 2014 22:56
@tarcieri
Copy link
Member

Nice! Aside from #96, I think this was the last blocker for 0.6 :shipit:

@penso
Copy link

penso commented Jun 12, 2014

@ixti @tarcieri There is an issue with the current redirector code, craigslist gives a Location header like "//charleston.craigslist.org/search/(...)" which won't work, it needs to keep the previous protocol if url looks like "^//".

wget -S "http://charleston.craigslist.org/search/sss?query=vanagon&catAbb=sss&srchType=A&format=rss" for a header sample.

@ixti
Copy link
Member Author

ixti commented Jun 12, 2014

Well, as HTTP/1.1 RFC was updated now it became valid indeed (previous RFC was actually telling that Location should be an absoluteURI). I'll fix this as part of #112

@ixti
Copy link
Member Author

ixti commented Jun 12, 2014

@penso I just realized that I'm unsure if you experience issue or not, because (just reviewed) current code should handle this situation perfectly fine. Added spec (in master) to cover this. Please open an issue if current redirector gives you an error (works perfectly fine for me on 0.6.1 and master).

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.

Modify HTTP method upon redirects when needed
3 participants