TooTallNate / proxy-agents

Node.js HTTP Proxy Agents Monorepo
https://proxy-agents.n8.io
872 stars 229 forks source link

[https-proxy-agent] Slice the headers when parsing proxy server response #212

Closed byjrack closed 11 months ago

byjrack commented 12 months ago

I have not done heavy testing, but has not broken yet. Hopefully meets the need.

vercel[bot] commented 12 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
proxy-agents ✅ Ready (Inspect) Visit Preview Jul 10, 2023 7:32pm
TooTallNate commented 11 months ago

Code looks good. Could you possible add a regression test for this? And also add a changeset please.

byjrack commented 11 months ago

Will do, hopefully they meet the need, but up for suggestions if not as I am treading into a bit of new territory.

TooTallNate commented 11 months ago

For the changeset, run pnpm exec changeset at the root of the repo and follow the prompts. Select https-proxy-agent as the package being updated and "patch" since this is a bug fix. Commit and push the file that gets generated.

As for the test, apply this change to the proxy module to get the existing tests in https-proxy-agent to start failing with the issue you are facing:

diff --git a/packages/proxy/src/proxy.ts b/packages/proxy/src/proxy.ts
index 4a81516..1c2d55d 100644
--- a/packages/proxy/src/proxy.ts
+++ b/packages/proxy/src/proxy.ts
@@ -462,5 +462,5 @@ function requestAuthorization(
                'Proxy-Authenticate': 'Basic realm="' + realm + '"',
        };
        res.writeHead(407, headers);
-       res.end();
+       res.end('Proxy authorization required');
 }
byjrack commented 11 months ago

Thanks. Yeah did an npx changeset and it seemed to assume a major so I need to clean that up before I push it upstream. And good call on adding the text to the return as that is an easy tweak to add a content body and generate the error if it's not checked.

changeset-bot[bot] commented 11 months ago

🦋 Changeset detected

Latest commit: 687607eeb0b1b78e128dfca6597794835efb44b9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ----------------- | ----- | | https-proxy-agent | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

byjrack commented 11 months ago

Thanks for the assist on the patch as I am a bit rusty. 😅