Skip to content
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

Proxy fails when using HTTP CONNECT (E.G. HTTPS via a HTTP only proxy) #2259

Open
PeterC89 opened this issue Apr 5, 2024 · 2 comments
Open
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@PeterC89
Copy link

PeterC89 commented Apr 5, 2024

release-please fails when trying to connect to a HTTP only proxy via HTTP CONNECT
See:

const {host, port} = defaultProxy;
if (new URL(baseUrl).protocol.replace(':', '') === 'http') {
return new HttpProxyAgent(`http://${host}:${port}`);
} else {
return new HttpsProxyAgent(`https://${host}:${port}`);
}

It would probably make sense just to take the whole proxy URL (E.G. http://my-proxy:80) and apply it to the HTTP agent instead of making assumptions about the users environment?

Happy to raise a PR for this 🙂

Note: This would also require an update in the GitHub action to stop it splitting the proxy input and would probably therefore be a breaking change for both release-please and the action.

Environment details

  • OS: Ubuntu 22.04 x64 and Windows 10 x64
  • Node.js version: 20.12.1
  • npm version: 10.5.1
  • release-please version: 16.10.1

Steps to reproduce

  1. Use a proxy value such as my-proxy:80 which only accepts http connections
  2. Get an error when release-please incorrectly tries to use https connection to the proxy.
@PeterC89 PeterC89 added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Apr 5, 2024
@PeterC89
Copy link
Author

PeterC89 commented Apr 5, 2024

Possible solution here: PeterC89@13913fb

Just noticed that the existing unit tests would have produced some very entertaining results with the current setup:

it('should return a https agent', () => {
expect(
GitHub.createDefaultAgent(GH_API_URL, {
host: 'http://proxy.com',
port: 3000,
})
).instanceof(HttpsProxyAgent);
});

Would have produced a proxy URL of https://http://proxy.com:3000

@PeterC89
Copy link
Author

PeterC89 commented Apr 9, 2024

There may also be an issue in the newer version of octokit requiring a different approach to proxies
octokit/rest.js#43

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

2 participants