search

ngrok / ngrok-rust

Embed ngrok secure ingress into your Rust apps with a single line of code.
Apache License 2.0
319 stars 19 forks source link

Switch to forwarding based on URL and add TLS forwarding #108

Closed jrobsonchase closed 1 year ago

jrobsonchase commented 1 year ago

Switches us from the explicit tcp, http, and pipe forwarding methods to accepting a URL. This should make it easier for us to update what protocols we support without having to change the public API.

Eventually, we'll also be able to forward smarter with regards to tls, proxyproto, etc. like the agent, but that'll come later.

bobzilladev commented 1 year ago

I wanted to make sure this would keep working, so I've gone through the upgrade process for the mutable builders and the forwarding changes. I've got everything working except one test with a unix file without a full path specified. Trying to get to the bottom of that before +1'ing here. The WIP branch: https://github.com/ngrok/ngrok-nodejs/compare/main...bob/upgrade-ngrok-rust

jrobsonchase commented 1 year ago

Ah, @bobzilladev - I think your unix socket is missing a //. You've got format!("unix:{addr}"), which for addrs like /tmp/some.sock would give you unix:/tmp/some.sock, where you actually want unix:///tmp/some.sock. Though now that I'm reading this article, maybe yours is supposed to be equivalent. I'll fiddle with it on my end a bit to get those to be considered the same.

bobzilladev commented 1 year ago

Ah, @bobzilladev - I think your unix socket is missing a //. You've got format!("unix:{addr}"), which for addrs like /tmp/some.sock would give you unix:/tmp/some.sock, where you actually want unix:///tmp/some.sock. Though now that I'm reading this article, maybe yours is supposed to be equivalent. I'll fiddle with it on my end a bit to get those to be considered the same.

Yeah, I tried that variation but it wasn't happy for the case that is unhappy, which is with a relative path. e.g. tunnel.sock used to work, creating a file in current working directory. Appears unix:tunnel.sock or unix://tunnel.sock seem to not be ok, but I need to play with it more.

bobzilladev commented 1 year ago

Relative is probably just off the table. I'll have to do a little preprocessing to convert to absolute path.

jrobsonchase commented 1 year ago

OK, I think I got it fixed. Here's the playground where I fiddled with various formats.

bobzilladev commented 1 year ago

The changes help a lot, relative unix pathing is good on nodejs again. A couple issues building on windows noted in comments. As you note in the rust playground, you get some odd results if you do pipe with a blank host, or no host and prepending forward slash. I think it's correct but we'll have to be explicit in documentation.

pipe:pipe_ngrok -> \\.\pipe\pipe_ngrok looks good! pipe://./pipe_ngrok -> \\.\pipe\pipe_ngrok looks good! pipe:/pipe_ngrok -> \\.\pipe\/pipe_ngrok wacky, but i guess correct pipe:///pipe_ngrok -> \\.\pipe\/pipe_ngrok wacky, but i guess correct

jrobsonchase commented 1 year ago

OK, got it building. And I think I covered all of the unix socket and pipe variants in the forward doc comment.