ureq icon indicating copy to clipboard operation
ureq copied to clipboard

add HTTPS proxy support

Open puffi opened this issue 3 years ago • 4 comments

The addition of HTTPS proxy support requires a change of the connect_host interface. The return type is changed, but since this is only used internally in this one file and has visibility pub(crate) this should not be a breaking change.

The TCP Stream now gets wrapped inside the connect_host function which makes the from_tcp_stream function of Stream unused.

Signed-off-by: Mira Limbeck [email protected]

puffi avatar Sep 28 '22 14:09 puffi

Would you mind rebasing off latest main? So we can see a clean test run.

algesten avatar Sep 29 '22 20:09 algesten

It should be rebased now, at least I see the force-push mentioned here.

Sorry, first time doing that here on Github.

puffi avatar Sep 30 '22 13:09 puffi

Sorry about that, it seems to be the unused from_tcp_stream function I left in which leads to the errors. Do you want me to remove it from the source?

puffi avatar Oct 03 '22 09:10 puffi

@puffi Yeah, that be great. Makes it compile cleanly in the CI. 👍

algesten avatar Oct 03 '22 12:10 algesten

@jsha what do you think about the extra Box<dyn> indirection we get in this PR?

I think the PR looks fine overall, but a bit concerned about this.

algesten avatar Oct 15 '22 17:10 algesten

Any reason this shouldn't be re-opened? I started implementing the same functionality before I found this PR. I ran into a small issue where we're calling Response::do_from_stream now which takes ownership of the tls_stream that we ultimately need to return, and there's no easy way to clone a Box<dyn ReadWrite> returned from tls_conf.connect(..), but it seems surmountable.

DanGould avatar Feb 04 '24 00:02 DanGould