Panic with large body sizes

[bibo38]

Panic with large body sizes

[antoniomika]

You can disable the idle connection logic by setting idle-connection=false either in the command line or in your sish config.

[bibo38]

Thanks, but this issue shows that the idle connection logic is implemented wrong. The documentation says Duration to wait for activity before closing a connection for all reads and writes (default 5s), but in the example the connection is closed after 5 seconds regardless of activity

[antoniomika]

The idle timeout exists for both sides of the tunnel. If one side of the tunnel (the writer) doesn't receive any messages, the connection is considered dead. This is currently implemented with golang's connection deadlines, which doesn't really have a way to apply deadlines differently (single deadline which includes read or write as extending the deadline). This is the expected behavior of how this is implemented.

We could maybe make sish smarter by looking for when a deadline occurs and having retry logic in if the other side of the connection has still not hit its deadline, but this feature can just as easily be disabled and timeouts handled by the ping timeout instead

[antoniomika]

I'm gonna open a PR with https://github.com/antoniomika/sish/blob/main/utils/conn.go#L181 changed to just call SetDeadline instead of the individual read or write deadlines. This would theoretically push both deadlines continuously as either read or write is called. Mind helping me test out those changes @bibo38 ?

[bibo38]

Thanks. Pushing both deadlines solves my problem 👍

Here is the diff that worked for my downloading file usecase:

diff --git a/utils/conn.go b/utils/conn.go
index 45b1df1..6927c94 100644
--- a/utils/conn.go
+++ b/utils/conn.go
@@ -184,7 +184,7 @@ type IdleTimeoutConn struct {
 
 // Read is needed to implement the reader part.
 func (i IdleTimeoutConn) Read(buf []byte) (int, error) {
-       err := i.Conn.SetReadDeadline(time.Now().Add(viper.GetDuration("idle-connection-timeout")))
+       err := i.Conn.SetDeadline(time.Now().Add(viper.GetDuration("idle-connection-timeout")))
        if err != nil {
                return 0, err
        }
@@ -194,7 +194,7 @@ func (i IdleTimeoutConn) Read(buf []byte) (int, error) {
 
 // Write is needed to implement the writer part.
 func (i IdleTimeoutConn) Write(buf []byte) (int, error) {
-       err := i.Conn.SetWriteDeadline(time.Now().Add(viper.GetDuration("idle-connection-timeout")))
+       err := i.Conn.SetDeadline(time.Now().Add(viper.GetDuration("idle-connection-timeout")))
        if err != nil {
                return 0, err
        }

Handling the errors is probably also a good idea to print meaningful status errors like The [HTTP/SSH/TCP] connection with X was longer than Y idle and therefore terminated, instead of a panic (which let's you think it's a bug in sish).

[antoniomika]

Cool! Appreciate you went through building and testing. I'll make a PR and get a version patched!