diff --git a/README.md b/README.md index cfb9eb1..eda7e7d 100644 --- a/README.md +++ b/README.md @@ -2667,6 +2667,9 @@ IRC_LISTEN_ADDR: "" An optional `PASS` before registration sets the session password (minimum 8 characters). Registering creates a session without a hashcash stamp; closing the connection ends the session, as `QUIT` does. +- **PING**: a connection that sends nothing for 90 seconds is sent a `PING`, and + is closed if it sends nothing in the 30 seconds after that; any line, such as + the client's `PONG`, starts both waits again. - **CAP negotiation**: `CAP LS` gets an empty capability list and `CAP END` is ignored, for compatibility with modern clients. - **Channel prefixes**: Channels must start with `#`. `JOIN` prepends it if diff --git a/internal/ircserver/conn.go b/internal/ircserver/conn.go index 8fc9224..bba99ca 100644 --- a/internal/ircserver/conn.go +++ b/internal/ircserver/conn.go @@ -20,7 +20,6 @@ import ( const ( maxLineLen = 512 - readTimeout = 5 * time.Minute writeTimeout = 30 * time.Second killWriteWindow = 2 * time.Second dnsTimeout = 3 * time.Second @@ -214,17 +213,31 @@ func resolveHost(ctx context.Context, addr string) string { // serve is the main loop for a single IRC client connection. // Cancelling ctx when it returns stops the relay goroutine. -func (c *Conn) serve(ctx context.Context) { +// When nothing has been read for interval the client is sent +// a PING, and when nothing is read within deadline after that +// the connection is closed. +func (c *Conn) serve( + ctx context.Context, + interval, deadline time.Duration, +) { ctx, cancel := context.WithCancel(ctx) defer cancel() defer c.cleanup(ctx) + ping := time.AfterFunc(interval, func() { + c.send("PING :" + c.serverSfx) + }) + defer ping.Stop() + scanner := bufio.NewScanner(c.conn) scanner.Buffer(make([]byte, maxLineLen), maxLineLen) for { + // Every line read starts both waits again. + ping.Reset(interval) + _ = c.conn.SetReadDeadline( - time.Now().Add(readTimeout), + time.Now().Add(interval + deadline), ) if !scanner.Scan() { @@ -366,7 +379,7 @@ func (c *Conn) handleMessage( msg *Message, ) { // Before registration, only NICK, USER, PASS, PING, - // QUIT, and CAP are accepted. + // PONG, QUIT, and CAP are accepted. if !c.registered { c.handlePreRegistration(ctx, msg) @@ -452,6 +465,10 @@ func (c *Conn) handlePreRegistration( case irc.CmdPing: c.handlePing(msg) + return + case irc.CmdPong: + // The answer to serve's PING, which unregistered + // clients are sent too. return case irc.CmdQuit: c.handleQuit(msg) diff --git a/internal/ircserver/export_test.go b/internal/ircserver/export_test.go index 60d039b..24d3007 100644 --- a/internal/ircserver/export_test.go +++ b/internal/ircserver/export_test.go @@ -13,13 +13,16 @@ import ( "sneak.berlin/go/neoirc/internal/service" ) -// NewTestServer creates a Server suitable for testing. +// NewTestServer creates a Server suitable for testing, whose +// connections use interval and deadline in place of +// pingInterval and pongDeadline. // The caller must call Stop() when finished. func NewTestServer( log *slog.Logger, cfg *config.Config, database *db.Database, brk *broker.Broker, + interval, deadline time.Duration, ) *Server { globs := &globals.Globals{ Appname: "neoirc", @@ -38,6 +41,9 @@ func NewTestServer( brk: brk, svc: svc, conns: make(map[*Conn]struct{}), + + pingInterval: interval, + pongDeadline: deadline, } } @@ -64,6 +70,14 @@ const KillWriteWindow = killWriteWindow // output queue when no broker notification arrives. const PollInterval = pollInterval +// PingInterval is how long the server waits with nothing read +// from a connection before it sends a PING. +const PingInterval = pingInterval + +// PongDeadline is how long the server waits after the PING +// for a line before it closes the connection. +const PongDeadline = pongDeadline + // NewTestConn returns a Conn for nick on tcpConn, without a // server, database or service behind it. func NewTestConn( diff --git a/internal/ircserver/server.go b/internal/ircserver/server.go index e3b6d2e..8d5ce86 100644 --- a/internal/ircserver/server.go +++ b/internal/ircserver/server.go @@ -6,6 +6,7 @@ import ( "log/slog" "net" "sync" + "time" "go.uber.org/fx" "sneak.berlin/go/neoirc/internal/broker" @@ -38,6 +39,11 @@ type Server struct { mu sync.Mutex conns map[*Conn]struct{} cancel context.CancelFunc + + // pingInterval and pongDeadline are the constants of the + // same names, except in tests, which shorten them. + pingInterval time.Duration + pongDeadline time.Duration } // New creates a new IRC Server and registers its lifecycle @@ -57,6 +63,9 @@ func New( listener: nil, cancel: nil, mu: sync.Mutex{}, + + pingInterval: pingInterval, + pongDeadline: pongDeadline, } listenAddr := params.Config.IRCListenAddr @@ -153,7 +162,7 @@ func (s *Server) acceptLoop(ctx context.Context) { s.mu.Unlock() }() - client.serve(ctx) + client.serve(ctx, s.pingInterval, s.pongDeadline) }() } } diff --git a/internal/ircserver/server_test.go b/internal/ircserver/server_test.go index 6aba3a8..845bd4b 100644 --- a/internal/ircserver/server_test.go +++ b/internal/ircserver/server_test.go @@ -9,6 +9,7 @@ import ( "net" "os" "runtime" + "slices" "strings" "testing" "time" @@ -70,6 +71,22 @@ func newTestEnvWithConfig( ) *testEnv { t.Helper() + return newTestEnvWithPingTimes( + t, cfg, ircserver.PingInterval, ircserver.PongDeadline, + ) +} + +// newTestEnvWithPingTimes is newTestEnvWithConfig, except +// that the server sends a connection a PING after interval +// with nothing read, and closes it when nothing is read +// within deadline after that. +func newTestEnvWithPingTimes( + t *testing.T, + cfg *config.Config, + interval, deadline time.Duration, +) *testEnv { + t.Helper() + // A random name, so that no other test environment, not // even an earlier run of the same test, can share it. dsn := fmt.Sprintf( @@ -126,7 +143,9 @@ func newTestEnvWithConfig( &slog.HandlerOptions{Level: slog.LevelError}, //nolint:exhaustruct )) - srv := ircserver.NewTestServer(log, cfg, database, brk) + srv := ircserver.NewTestServer( + log, cfg, database, brk, interval, deadline, + ) err = srv.Start(addr) if err != nil { @@ -174,6 +193,34 @@ func (env *testEnv) dial(t *testing.T) *testClient { } } +// The ping interval and PONG deadline for +// newTestEnvWithShortPings. They are equal, so that three +// answered PINGs take longer than a client that answers none +// stays connected. +const ( + testPingInterval = 300 * time.Millisecond + testPongDeadline = 300 * time.Millisecond +) + +// newTestEnvWithShortPings starts an IRC server that uses +// testPingInterval and testPongDeadline in place of +// pingInterval and pongDeadline. +func newTestEnvWithShortPings(t *testing.T) *testEnv { + t.Helper() + + return newTestEnvWithPingTimes( + t, + &config.Config{ServerName: testServerName}, + testPingInterval, testPongDeadline, + ) +} + +// isServerPing reports whether line is the PING the test +// server sends a quiet client. +func isServerPing(line string) bool { + return line == "PING :"+testServerName +} + // testClient wraps a raw TCP connection with helpers. type testClient struct { t *testing.T @@ -388,6 +435,57 @@ func TestPingPong(t *testing.T) { assertContains(t, lines, "PONG", "PONG response") } +// TestQuietClientAnsweringPingStaysConnected checks that a +// registered client that sends nothing is sent a PING each +// time the ping interval passes, and stays connected as long +// as it answers each one. +func TestQuietClientAnsweringPingStaysConnected(t *testing.T) { + t.Parallel() + + env := newTestEnvWithShortPings(t) + client := env.dial(t) + client.register("answers") + + for range 3 { + client.readUntil(isServerPing) + client.send("PONG :" + testServerName) + } + + client.sendAndExpect("PING :still here", "PONG") +} + +// TestQuietClientNotAnsweringPingIsClosed checks that a +// registered client that does not answer the PING is +// disconnected. +func TestQuietClientNotAnsweringPingIsClosed(t *testing.T) { + t.Parallel() + + env := newTestEnvWithShortPings(t) + client := env.dial(t) + client.register("silent") + + lines := client.readUntilClosed() + if !slices.ContainsFunc(lines, isServerPing) { + t.Errorf("closed without a PING, got lines %q", lines) + } +} + +// TestUnregisteredClientAnswersPing checks that a PONG before +// registration, the answer to the PING unregistered clients +// are sent too, gets no ERR_NOTREGISTERED. +func TestUnregisteredClientAnswersPing(t *testing.T) { + t.Parallel() + + env := newTestEnvWithShortPings(t) + client := env.dial(t) + + client.readUntil(isServerPing) + client.send("PONG :" + testServerName) + + lines := client.sendAndExpect("PING :after", "PONG") + assertNotContains(t, lines, " 451 ", "ERR_NOTREGISTERED") +} + // TestIdleCleanupKeepsActiveUser checks that a user who // sends commands is kept by the idle cleanup, and a user who // sends nothing is not.