Skip to content
This repository was archived by the owner on Mar 10, 2020. It is now read-only.

Make url-add follow HTTP redirections #514

Merged
merged 4 commits into from
Aug 6, 2017

Conversation

maxlath
Copy link
Contributor

@maxlath maxlath commented Jan 23, 2017

fix #513

That's the shortest path I found to solve this issue, but different solutions could be considered:

  • delegating redirection handling to request.js
  • reintegrating a HTTP request library that handles it, but I guess you had your reasons to remove wreck


const redirection = res.headers.location

if (redirection) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also check for the right redirection header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which header is that if not location? That's the header in the spec and that seem to be used by the request module too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sorry, I meant the status code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. I'm adding a commit to cover that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix 4821ac4

@dignifiedquire
Copy link
Contributor

Why is this not part of node core...

@daviddias
Copy link
Contributor

@maxlath mind doing a rebase of master into this branch?

@maxlath
Copy link
Contributor Author

maxlath commented Jul 28, 2017

@diasdavid rebase done! plus, I added a test 78c0537

@daviddias
Copy link
Contributor

@maxlath mind fixing lint? Looks like that is the last step to get this done :)

@maxlath
Copy link
Contributor Author

maxlath commented Aug 6, 2017

@diasdavid done! 08f785a

@daviddias daviddias merged commit 0f430ef into ipfs-inactive:master Aug 6, 2017
@daviddias
Copy link
Contributor

awesome, thanks @maxlath :D

@maxlath maxlath deleted the follow-redirect branch August 6, 2017 07:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

addFromURL fails to follow redirections
3 participants