-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Allow optional redirect host rewriting. #741
Allow optional redirect host rewriting. #741
Conversation
727cced
to
74782bd
Compare
@@ -144,7 +144,7 @@ web_o = Object.keys(web_o).map(function(pass) { | |||
proxyReq.on('response', function(proxyRes) { | |||
if(server) { server.emit('proxyRes', proxyRes, req, res); } | |||
for(var i=0; i < web_o.length; i++) { | |||
if(web_o[i](req, res, proxyRes)) { break; } | |||
if(web_o[i](req, res, proxyRes, options)) { break; } |
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.
passing options down to web-outgoing gives us a level on configuration that is nice(/needed) for this kinda stuff :)
@samccone this looks good! Could you add a test for this and update the docs related to this option? Thanks for the contribution! |
awww snap sweet, I was also thinking about But I will save that work for another PR. |
hey @jcrugzz specs and docs are all added. thanks again for your time, and work on this handy lib. |
a1fc98e
to
d1da981
Compare
@samccone looks great. I forgot the only one nit here that I will post on the code. Will merge once thats done. Thanks! |
@@ -43,6 +44,13 @@ var passes = exports; | |||
} | |||
}, | |||
|
|||
function setRedirectHostRewrite(req, res, proxyRes, options) { | |||
if (options.hostRewrite && /^30(1|2|7|8)$/.test(proxyRes.statusCode)) { |
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.
Can we just precompile the regex at the top? (minor perf thing) Forgot to mention it before
d1da981
to
add8133
Compare
all updated @jcrugzz 🎋 |
Hey @jcrugzz was there anything else you needed from me? |
@samccone sorry got caught up with things so I didn't get to it. LGTM. Ill post back here with some style nits that I make for next time :). Thanks! Im also curious about the use of a |
Allow optional redirect host rewriting.
Hey all, really appreciate this great lib, here is a solution I implemented for host rewriting for redirects.
If you are interested in this I can add tests and docs. thanks again