-
Notifications
You must be signed in to change notification settings - Fork 296
Conversation
src/api/util/url-add.js
Outdated
|
||
const redirection = res.headers.location | ||
|
||
if (redirection) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix 4821ac4
Why is this not part of node core... |
@maxlath mind doing a rebase of master into this branch? |
@diasdavid rebase done! plus, I added a test 78c0537 |
@maxlath mind fixing lint? Looks like that is the last step to get this done :) |
@diasdavid done! 08f785a |
awesome, thanks @maxlath :D |
fix #513
That's the shortest path I found to solve this issue, but different solutions could be considered:
request.js
wreck