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

feat(http): enable ssl tunneling through proxy #5781

Closed
wants to merge 1 commit into from

Conversation

Thore1954
Copy link

This is similar to #5037 but has a simpler implementation adapted from hpagent and additionally supports HTTPS proxies.

@robertpatrick
Copy link

any update on this? I am running into the same issue

@Matthiasvanderhallen
Copy link

This looks like a good improvement. I just wanted to remark that it looks like this does not include the fix you provided in PR #5772 which was closed without merging?

@Thore1954 Thore1954 closed this Nov 4, 2023
@ShemTovYosef
Copy link

Hi @Thore1954 don’t you plan to merge it? Any workaround that you can advice ?

@Matthiasvanderhallen
Copy link

If it's not against etiquette, I'm willing to make any updates necessary to get this to a point where it can be merged.
Only if @Thore1954 is not planning to champion this further in another branch or something like that, of course...

@Thore1954
Copy link
Author

I just wanted to remark that it looks like this does not include the fix you provided in PR #5772 which was closed without merging?

I realized these two PRs would have merge conflicts so I decided to close it and create a new one later. Besides, the issue I pointed out is not only for proxy url auth, but also for target url auth:

axios/lib/adapters/http.js

Lines 357 to 361 in 2701911

if (!auth && parsed.username) {
const urlUsername = parsed.username;
const urlPassword = parsed.password;
auth = urlUsername + ':' + urlPassword;
}

Hi @Thore1954 don’t you plan to merge it? Any workaround that you can advice ?

I was indeed expecting a maintainer to merge it at one time. As for the workaround, I am currently using this proxy server which I modified to handle HTTPS requests without tunneling, although it should be used with caution. Alternatively, you could try patching your axios dependency using patch-package.

I'm willing to make any updates necessary to get this to a point where it can be merged. Only if @Thore1954 is not planning to champion this further in another branch

The reason I closed this PR is that it would introduce a breaking change. For the moment, it should be implemented behind a flag, but I would set it as default in the next major release since it aligns with all proxy servers.
I am presently not incentivized to pursue this further since my workaround is working adequately for the time being.

@ShemTovYosef
Copy link

Unfortunately we cannot modify the code or use additional server .
As I understood, you don’t have plans to introduce fix in the near future, maybe @Matthiasvanderhallen can continue with it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants