search

ngrok / ngrok-go

Embed ngrok secure ingress into your Go apps as a net.Listener with a single line of code.
MIT License
667 stars 64 forks source link

Propagate half-closes correctly in forward #163

Closed euank closed 3 months ago

euank commented 3 months ago

Before, the following would not work as you would expect:

// # One terminal
// $ ncat --recv-only -l 9090

// ngrok-go code
fwd, err := sess.ListenAndForward(
  ctx,
  "127.0.0.1:9090",
  config.TCPEndpoint(),
)

// fwd.URL() is 0.tcp.jp.ngrok.io:14517 for this example

// another terminal
// $ ncat --send-only 0.tcp.jp.ngrok.io 14517 < hello-world.txt

What we would expect from the above would be for the send side to send "hello world" and exit, and then the recv side to print "hello world" and also exit.

This is what happens if you do ncat --send-only localhost 9090 instead of copying through the ngrok tcp tunnel.

Before this change, when copying through ngrok the recv side would not exit because the 'Close' of the connection did not get propagated through the 'join'.

I've also added a unit test showing this.

Thank you to @abakum for originally noticing this issue and offering a fix over in #137. In the hopes of landing this more quickly, I've written a new version, derived from the internal ngrok agent's join code, which should thus be easier to review etc.

To try and give credit correctly, I've maintained the original commit from #137 as well.

bobzilladev commented 3 months ago

Ah, need the authtoken to run tests, will work out of the box as a branch on ngrok org if moved there.

euank commented 3 months ago

Thanks for the review; I created a dummy PR over in #165 to verify it's happy wrt CI! It would be nice if we could figure something out to allow external PRs to get proper CI too. Regardless, merging based on review + CI here