-
-
Notifications
You must be signed in to change notification settings - Fork 350
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
feat(corporate proxy #1188): support in verifier #1199
feat(corporate proxy #1188): support in verifier #1199
Conversation
Add native support of HTTPS_PROXY and HTTP_PROXY for verifier
} | ||
|
||
if (bodyData) { | ||
proxyReq.setHeader('Content-Length', Buffer.byteLength(bodyData)); |
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 is one piece of behavior that did go away - I didn't see any tests around content length header specifically, so I thought this might have just been a best practice with the rewrite that was used here
if this creates a gap, let me know and I can see about doing something similar with the proxy request options
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.
Thanks. I believe this was required given the way the previous body was parsed and then re-written into the subsequent request. Given that you're now using the buffer
option on the HttpProxy
itself, perhaps it's no longer needed.
The main behaviour we'll need to check is if we can intercept and modify the incoming body using the hooks/filters. I'll pull this in locally and do some testing, I don't think it's an area with great test coverage if I'm honest.
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 looks great. i'm going to test locally with a few scenarios and if they work, will bring this in asap!
} | ||
|
||
if (bodyData) { | ||
proxyReq.setHeader('Content-Length', Buffer.byteLength(bodyData)); |
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.
Thanks. I believe this was required given the way the previous body was parsed and then re-written into the subsequent request. Given that you're now using the buffer
option on the HttpProxy
itself, perhaps it's no longer needed.
The main behaviour we'll need to check is if we can intercept and modify the incoming body using the hooks/filters. I'll pull this in locally and do some testing, I don't think it's an area with great test coverage if I'm honest.
Add native support of HTTPS_PROXY and HTTP_PROXY for verifier for #1188
This change adds two pieces of behavior:
This results in out-of-the-box support for corporate proxies in the js verifier