Recognize 303, 307 and 308 and do not crash on malformed responses #361

Closed
Rob--W opened this Issue Jan 5, 2013 · 4 comments

Comments

Projects
None yet
4 participants

Rob--W commented Jan 5, 2013

This part of the code needs to be changed: https://github.com/nodejitsu/node-http-proxy/blob/v0.8.7/lib/node-http-proxy/http-proxy.js#L250-L257

to

    var statusCode = response.statusCode;
    if (statusCode === 301 || statusCode === 302 || statusCode === 303 || statusCode === 307 || statusCode === 308) {
      if (self.source.https && !self.target.https) {
        response.headers.location = (response.headers.location || '').replace(/^http:/, 'https:');
      }
      if (self.target.https && !self.source.https) {
        response.headers.location = (response.headers.location || '').replace(/^https:/, 'http:');
      }
    }

The changes are:

  1. Status code 303, 307 and 308 are recognized and correctly rewritten.
  2. When some stupid server does not include the Location header with the status, the server will still stay alive (opposed to crashing with message "Cannot call method 'replace' of undefined")

(and a minor change: Removed the backslash before the colon in the RegExp)

genbit commented Jan 9, 2013

this patch, works for me, thanks!

Contributor

Rush commented Sep 15, 2013

Why would it not be accepted by upsteam??

Rob--W commented Sep 15, 2013

The existence-check of the location header has already been added some time ago: https://github.com/nodejitsu/node-http-proxy/blob/ebbba73eda49563ade09f38bdc8aef13d1cf6c00/lib/node-http-proxy/http-proxy.js#L272-L273

Though 307 and 308 redirects are still not supported. Are you referring to that?

Owner

jcrugzz commented Oct 30, 2015

This is related to an older version of http-proxy. Closing.

jcrugzz closed this Oct 30, 2015

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