diff --git a/README.md b/README.md index 8fc3ff5..6f983ab 100644 --- a/README.md +++ b/README.md @@ -2677,6 +2677,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..db02999 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, } } @@ -56,6 +62,13 @@ func (s *Server) Listener() net.Listener { return s.listener } +// PingTimes returns how long the server's connections wait +// with nothing read before they are sent a PING, and after it +// before they are closed. +func (s *Server) PingTimes() (time.Duration, time.Duration) { + return s.pingInterval, s.pongDeadline +} + // KillWriteWindow is how long Disconnect allows each write // to a killed victim. const KillWriteWindow = killWriteWindow @@ -64,6 +77,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..e7f92d4 100644 --- a/internal/ircserver/server_test.go +++ b/internal/ircserver/server_test.go @@ -13,10 +13,13 @@ import ( "testing" "time" + "go.uber.org/fx/fxtest" "sneak.berlin/go/neoirc/internal/broker" "sneak.berlin/go/neoirc/internal/config" "sneak.berlin/go/neoirc/internal/db" + "sneak.berlin/go/neoirc/internal/globals" "sneak.berlin/go/neoirc/internal/ircserver" + "sneak.berlin/go/neoirc/internal/logger" _ "modernc.org/sqlite" ) @@ -70,6 +73,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 +145,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 +195,34 @@ func (env *testEnv) dial(t *testing.T) *testClient { } } +// The ping interval and PONG deadline for +// newTestEnvWithShortPings. They differ, so that a test can +// tell them apart; the PONG deadline is how long a test +// client has to answer a PING. +const ( + testPingInterval = time.Second + 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 @@ -272,6 +321,24 @@ func (tc *testClient) readUntilClosed() []string { return lines } +// readServerPing reads until the PING the test server sends a +// quiet client, and fails the test if it came sooner than +// testPingInterval after lastSent, a time just before the +// client sent its last line. +func (tc *testClient) readServerPing(lastSent time.Time) { + tc.t.Helper() + + tc.readUntil(isServerPing) + + waited := time.Since(lastSent) + if waited < testPingInterval { + tc.t.Errorf( + "PING came %v after the client's last line, want at least %v", + waited, testPingInterval, + ) + } +} + // assertContains checks that at least one line matches the // given substring. func assertContains( @@ -388,6 +455,129 @@ func TestPingPong(t *testing.T) { assertContains(t, lines, "PONG", "PONG response") } +// TestQuietClientAnsweringPingStaysConnected checks that a +// registered client that sends nothing is sent a PING once +// the ping interval has passed since its last line, and stays +// connected as long as it answers each one, whether with a +// PONG or with another line. +func TestQuietClientAnsweringPingStaysConnected(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + answer string + }{ + {name: "PONG", answer: "PONG :" + testServerName}, + {name: "PING of its own", answer: "PING :answer"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + env := newTestEnvWithShortPings(t) + client := env.dial(t) + + lastSent := time.Now() + + client.register("answers") + + // The second PING comes later than a client that + // answers none stays connected. + for range 2 { + client.readServerPing(lastSent) + + lastSent = time.Now() + + client.send(tt.answer) + } + + client.sendAndExpect("PING :still here", "still here") + }) + } +} + +// TestQuietClientNotAnsweringPingIsClosed checks that a +// registered client that does not answer the PING is +// disconnected once the PONG deadline has passed after it. +func TestQuietClientNotAnsweringPingIsClosed(t *testing.T) { + t.Parallel() + + // latest leaves room for a loaded host, but is sooner than + // a read deadline of two ping intervals would close it. + const ( + earliest = testPingInterval + testPongDeadline + latest = 2 * testPingInterval + ) + + env := newTestEnvWithShortPings(t) + client := env.dial(t) + + lastSent := time.Now() + + client.register("silent") + client.readServerPing(lastSent) + client.readUntilClosed() + + waited := time.Since(lastSent) + if waited < earliest || waited >= latest { + t.Errorf( + "closed %v after the client's last line, want %v to %v", + waited, earliest, latest, + ) + } +} + +// 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") +} + +// TestNewPingTimes checks that a server made by New sends a +// PING after 90 seconds with nothing read, and closes the +// connection 30 seconds after that, as README.md states. +func TestNewPingTimes(t *testing.T) { + t.Parallel() + + const ( + wantInterval = 90 * time.Second + wantDeadline = 30 * time.Second + ) + + log, err := logger.New(nil, logger.Params{ + Globals: &globals.Globals{}, + }) + if err != nil { + t.Fatal(err) + } + + // With no IRC_LISTEN_ADDR, New starts no listener and + // uses nothing else from its params. + srv := ircserver.New(fxtest.NewLifecycle(t), ircserver.Params{ + Logger: log, + Config: &config.Config{}, + }) + + interval, deadline := srv.PingTimes() + if interval != wantInterval || deadline != wantDeadline { + t.Errorf( + "PING after %v and close %v later, want %v and %v", + interval, deadline, wantInterval, wantDeadline, + ) + } +} + // TestIdleCleanupKeepsActiveUser checks that a user who // sends commands is kept by the idle cleanup, and a user who // sends nothing is not.