http->https 302 redirects are improperly rewritten

[quartzjer]

The pull req #165 introduced broken logic, it rewrites every redirect, such that if a proxied services 302's any url (in our instance, our https-fronted and http-back-end sends a redirect to instagram's http cdn).

https://github.com/elfsternberg/node-http-proxy/blob/411bb51cc6/lib/node-http-proxy/http-proxy.js#L216

It rewrites every single redirect regardless :(

[abh]

It also breaks redirects to other protocols (however uncommon that is).

[indexzero]

@quartzjer How would you go about fixing it? Honestly I'm not sure I follow what's wrong.

[quartzjer]

If there's a front-end of https://external/ and a back-end of http://internal/ when the back-end sends a redirect to http://some-other-site.com/foo.bar this code will rewrite it to https://sime-other-site.com/foo.bar, which is generally a bad thing to be rewriting every redirect :)

I honestly don't understand the original issue to suggest a fix as I'm not sure what the desired behavior would be, all I know is the current one is not desirable.

[quartzjer]

Is there any way this can be fixed in master? I'm (undesirably) using a fork that just has this code commented out, and I don't know how this condition isn't horribly broken for everyone, every single redirect response is rewritten in this case.

[ovaillancourt]

+1

@quartzjer -> You could create a clean pull request from that fork to accelerate the fix.

[quartzjer]

I would, but I don't know what the problem is actually being solved so
I just commented it out :)

On Thu, Mar 15, 2012 at 4:59 PM, Olivier Vaillancourt
reply@reply.github.com
wrote:

+1

@quartzjer -> You could create a clean fix + pull request from that fork to accelerate the fix.

---

Reply to this email directly or view it on GitHub:
#192 (comment)

[ovaillancourt]

Yeah, looking at it, these lines (those commented out) do solve the specific problem of the user that submitted the fix but I'm pretty sure it's not really a desirable behaviour for a proxy...

You pretty much irreversibly loose control over your protocol with this "fix". I think it should either be configurable in the proxy or managed on the app side.

We're running an internal build of http-proxy that has the exact same lines commented out :P.

[mansona]

Has there been any update on this issue?

[indexzero]

Should be fixed in 6a278b3