From a81044322b7b3423ccb0f6d74e7dd1065a7258ba Mon Sep 17 00:00:00 2001 From: Rudi Date: Mon, 19 Oct 2026 00:42:19 +0000 Subject: [PATCH] Close memory holes, adjust tests --- main.go | 82 ++++++++++++++++- main_test.go | 256 +++++++++++++++++++++++++++++++++++++++++++++++++++ screens.go | 4 +- types.go | 57 ++++++++---- 4 files changed, 380 insertions(+), 19 deletions(-) diff --git a/main.go b/main.go index 34e6889..2187f69 100644 --- a/main.go +++ b/main.go @@ -546,6 +546,10 @@ func ensureSlotConnected(s *UserMasterSession, slotIdx int, background bool, cli IsBound: isBound, MaxBufferSize: maxBuf, } + if GlobalConfig != nil { + slot.AutoHoldDiscardLimit = GlobalConfig.Server.AutoHoldDiscardLimit + slot.AutoHoldBurstLimit = GlobalConfig.Server.AutoHoldBurstLimit + } s.Slots[slotIdx] = slot go upstreamReader(s, slot, slotIdx) return slot @@ -575,8 +579,7 @@ func connectOrResumeSlot(s *UserMasterSession, slotIdx int, clientConn net.Conn) }() slot.Mu.Lock() - slot.ScreenChanged = false - slot.IsHeld = false + slot.resetHoldAndLoopCountersLocked() if len(slot.LastScreen) > 0 { _, _ = clientConn.Write(slot.LastScreen) } @@ -781,6 +784,21 @@ func upstreamReader(s *UserMasterSession, slot *SessionSlot, slotIdx int) { // Cap buffer at 64KB to avoid unbounded memory use from constant ticks if len(recBuf) > 65536 { recBuf = recBuf[len(recBuf)-16384:] + if !isActive { + slot.Mu.Lock() + slot.DiscardCount++ + discardLimit := slot.AutoHoldDiscardLimit + if discardLimit == 0 { + discardLimit = defaultAutoHoldDiscardLimit + } + if discardLimit > 0 && slot.DiscardCount >= discardLimit { + slot.IsHeld = true + slot.ScreenChanged = false + slot.DiscardCount = 0 + log.Printf("[%s] '%s' System [%d] %s placed on automatic HOLD (raw receive buffer discard loop detected)", displayTag, username, slotIdx+1, slot.System.Name) + } + slot.Mu.Unlock() + } } recBuf = append(recBuf, data...) for { @@ -870,8 +888,41 @@ func upstreamReader(s *UserMasterSession, slot *SessionSlot, slotIdx int) { continue } + now := time.Now() + + // Auto-hold loop detection for background sessions: + // 1. High frequency write updates (excessive update rate loop) + if !isActive { + burstLimit := slot.AutoHoldBurstLimit + if burstLimit == 0 { + burstLimit = defaultAutoHoldBurstLimit + } + if burstLimit > 0 { + if slot.WriteBurstWindow.IsZero() || now.Sub(slot.WriteBurstWindow) > autoHoldBurstWindow { + slot.WriteBurstCount = 1 + slot.WriteBurstWindow = now + } else { + slot.WriteBurstCount++ + } + + if slot.WriteBurstCount >= burstLimit { + slot.IsHeld = true + slot.ScreenChanged = false + slot.WriteBurstCount = 0 + slot.DiscardCount = 0 + log.Printf("[%s] '%s' System [%d] %s placed on automatic HOLD (excessive update rate detected)", displayTag, username, slotIdx+1, slot.System.Name) + if isEraseWrite || len(slot.LastScreen) == 0 { + slot.LastScreen = bytes.Clone(screenData) + } + slot.Mu.Unlock() + continue + } + } + } + if isEraseWrite || len(slot.LastScreen) == 0 { slot.LastScreen = bytes.Clone(screenData) + slot.DiscardCount = 0 } else if len(slot.LastScreen)+len(screenData) <= maxBuf { slot.LastScreen = append(slot.LastScreen, screenData...) } else { @@ -886,6 +937,33 @@ func upstreamReader(s *UserMasterSession, slot *SessionSlot, slotIdx int) { newBuf = append(newBuf, slot.LastScreen[:baseLen]...) newBuf = append(newBuf, screenData...) slot.LastScreen = newBuf + + // Auto-hold loop detection for background sessions: + // 2. Buffer overflow discard loop (repeated discards of older intermediate writes) + if !isActive { + discardLimit := slot.AutoHoldDiscardLimit + if discardLimit == 0 { + discardLimit = defaultAutoHoldDiscardLimit + } + if discardLimit > 0 { + if slot.DiscardWindow.IsZero() || now.Sub(slot.DiscardWindow) > autoHoldDiscardWindow { + slot.DiscardCount = 1 + slot.DiscardWindow = now + } else { + slot.DiscardCount++ + } + + if slot.DiscardCount >= discardLimit { + slot.IsHeld = true + slot.ScreenChanged = false + slot.DiscardCount = 0 + slot.WriteBurstCount = 0 + log.Printf("[%s] '%s' System [%d] %s placed on automatic HOLD (discard loop detected)", displayTag, username, slotIdx+1, slot.System.Name) + slot.Mu.Unlock() + continue + } + } + } } if !isActive { slot.ScreenChanged = true diff --git a/main_test.go b/main_test.go index f23e132..7f58780 100644 --- a/main_test.go +++ b/main_test.go @@ -3635,3 +3635,259 @@ func TestCleanupSessionIfEmpty(t *testing.T) { } } +func TestAutoHoldOnDiscardLoopBufferOverflow(t *testing.T) { + origSystems := ConfiguredSystem + defer func() { + ConfiguredSystem = origSystems + }() + ConfiguredSystem = []TargetSystem{ + {Name: "DISCARD_SYS", Description: "Discard loop system"}, + } + + clientServer, clientUser := net.Pipe() + defer clientServer.Close() + defer clientUser.Close() + + upServer, upClient := net.Pipe() + defer upServer.Close() + defer upClient.Close() + + initialScreen := []byte{CMD_ERASE_WRITE, 0xC3, 0x11, 0x40, 0x40, 'B', 'A', 'S', 'E', IAC, EOR} + + slot := &SessionSlot{ + System: ConfiguredSystem[0], + Upstream: upClient, + LastScreen: initialScreen, + MaxBufferSize: 200, // Small buffer so it easily overflows + AutoHoldBurstLimit: -1, // Disable burst rate check to test discard check specifically + AutoHoldDiscardLimit: 3, // 3 discards triggers auto-hold + } + + s := &UserMasterSession{ + Username: "DISCARDUSER", + DisplayTag: "disc1234", + TerminalType: "IBM-3278-2", + ClientConn: clientServer, + ActiveSlotIdx: -1, // Background + Slots: []*SessionSlot{slot}, + IsDisconnected: false, + } + + go upstreamReader(s, slot, 0) + + // Send 10 writes of 60 bytes each (each > 200/2, quickly overflowing 200-byte MaxBufferSize) + writePayload := []byte{CMD_WRITE, 0xC3, 0x11, 0x40, 0x60} + for i := 0; i < 50; i++ { + writePayload = append(writePayload, 'X') + } + writePayload = append(writePayload, IAC, EOR) + + for i := 0; i < 10; i++ { + _, err := upServer.Write(writePayload) + if err != nil { + t.Fatalf("Failed to write payload %d: %v", i, err) + } + time.Sleep(10 * time.Millisecond) + } + + time.Sleep(150 * time.Millisecond) + + slot.Mu.Lock() + isHeld := slot.IsHeld + screenChanged := slot.ScreenChanged + slot.Mu.Unlock() + + if !isHeld { + t.Fatal("Expected slot.IsHeld == true after buffer overflow discard loop") + } + if screenChanged { + t.Fatal("Expected slot.ScreenChanged == false once placed on automatic HOLD") + } + + // Verify status in main menu is rendered as "HELD" with color Red + screen := buildMainMenuScreen(s) + var rowHeld *go3270.Field + for i := range screen { + f := &screen[i] + if f.Row == 7 && strings.Contains(f.Content, "DISCARD_SYS") { + rowHeld = f + break + } + } + if rowHeld == nil { + t.Fatal("Expected row for DISCARD_SYS on row 7") + } + if !strings.Contains(rowHeld.Content, "HELD") { + t.Fatalf("Expected row content to contain 'HELD', got: %s", rowHeld.Content) + } + if rowHeld.Color != go3270.Red { + t.Fatalf("Expected HELD row color to be Red, got: %v", rowHeld.Color) + } + + // Releasing the hold resets counters + slot.Mu.Lock() + slot.resetHoldAndLoopCountersLocked() + isHeldAfterReset := slot.IsHeld + discardCountAfterReset := slot.DiscardCount + slot.Mu.Unlock() + + if isHeldAfterReset { + t.Fatal("Expected IsHeld == false after resetHoldAndLoopCountersLocked") + } + if discardCountAfterReset != 0 { + t.Fatalf("Expected DiscardCount == 0, got %d", discardCountAfterReset) + } +} + +func TestAutoHoldOnExcessiveUpdateRate(t *testing.T) { + origSystems := ConfiguredSystem + defer func() { + ConfiguredSystem = origSystems + }() + ConfiguredSystem = []TargetSystem{ + {Name: "BURST_SYS", Description: "Burst loop system"}, + } + + clientServer, clientUser := net.Pipe() + defer clientServer.Close() + defer clientUser.Close() + + upServer, upClient := net.Pipe() + defer upServer.Close() + defer upClient.Close() + + initialScreen := []byte{CMD_ERASE_WRITE, 0xC3, 0x11, 0x40, 0x40, 'B', 'A', 'S', 'E', IAC, EOR} + + slot := &SessionSlot{ + System: ConfiguredSystem[0], + Upstream: upClient, + LastScreen: initialScreen, + MaxBufferSize: 32768, // Huge buffer, will NOT overflow + AutoHoldBurstLimit: 5, // 5 rapid updates triggers auto-hold + AutoHoldDiscardLimit: -1, // Disable discard check + } + + s := &UserMasterSession{ + Username: "BURSTUSER", + DisplayTag: "brst1234", + TerminalType: "IBM-3278-2", + ClientConn: clientServer, + ActiveSlotIdx: -1, // Background + Slots: []*SessionSlot{slot}, + IsDisconnected: false, + } + + go upstreamReader(s, slot, 0) + + // Send 6 small updates in rapid succession + smallUpdate := []byte{CMD_WRITE, 0xC3, 0x11, 0x40, 0x50, '1', IAC, EOR} + for i := 0; i < 6; i++ { + _, err := upServer.Write(smallUpdate) + if err != nil { + t.Fatalf("Failed to write small update %d: %v", i, err) + } + } + + time.Sleep(100 * time.Millisecond) + + slot.Mu.Lock() + isHeld := slot.IsHeld + screenChanged := slot.ScreenChanged + slot.Mu.Unlock() + + if !isHeld { + t.Fatal("Expected slot.IsHeld == true after excessive update rate loop") + } + if screenChanged { + t.Fatal("Expected slot.ScreenChanged == false once placed on automatic HOLD") + } + + // Menu renders HELD + screen := buildMainMenuScreen(s) + var rowHeld *go3270.Field + for i := range screen { + f := &screen[i] + if f.Row == 7 && strings.Contains(f.Content, "BURST_SYS") { + rowHeld = f + break + } + } + if rowHeld == nil { + t.Fatal("Expected row for BURST_SYS on row 7") + } + if !strings.Contains(rowHeld.Content, "HELD") { + t.Fatalf("Expected row content to contain 'HELD', got: %s", rowHeld.Content) + } +} + +func TestActiveSlotDoesNotAutoHold(t *testing.T) { + origSystems := ConfiguredSystem + defer func() { + ConfiguredSystem = origSystems + }() + ConfiguredSystem = []TargetSystem{ + {Name: "ACTIVE_SYS", Description: "Active interactive system"}, + } + + clientServer, clientUser := net.Pipe() + defer clientServer.Close() + defer clientUser.Close() + + upServer, upClient := net.Pipe() + defer upServer.Close() + defer upClient.Close() + + // Drain clientUser in background + go func() { + buf := make([]byte, 4096) + for { + _, err := clientUser.Read(buf) + if err != nil { + return + } + } + }() + + initialScreen := []byte{CMD_ERASE_WRITE, 0xC3, 0x11, 0x40, 0x40, 'B', 'A', 'S', 'E', IAC, EOR} + + slot := &SessionSlot{ + System: ConfiguredSystem[0], + Upstream: upClient, + LastScreen: initialScreen, + MaxBufferSize: 200, + AutoHoldBurstLimit: 5, + } + + s := &UserMasterSession{ + Username: "ACTIVEUSER", + DisplayTag: "actv1234", + TerminalType: "IBM-3278-2", + ClientConn: clientServer, + ActiveSlotIdx: 0, // ACTIVE (user is viewing slot 0) + Slots: []*SessionSlot{slot}, + IsDisconnected: false, + } + + go upstreamReader(s, slot, 0) + + // Send 10 writes rapidly + smallUpdate := []byte{CMD_WRITE, 0xC3, 0x11, 0x40, 0x50, 'Z', IAC, EOR} + for i := 0; i < 10; i++ { + _, err := upServer.Write(smallUpdate) + if err != nil { + t.Fatalf("Failed to write update %d: %v", i, err) + } + } + + time.Sleep(100 * time.Millisecond) + + slot.Mu.Lock() + isHeld := slot.IsHeld + slot.Mu.Unlock() + + if isHeld { + t.Fatal("Expected active slot (user watching) to NOT be placed on automatic HOLD") + } +} + + diff --git a/screens.go b/screens.go index 0ff48ee..fc16e4e 100644 --- a/screens.go +++ b/screens.go @@ -309,9 +309,11 @@ func runMenuLoop(s *UserMasterSession, clientConn net.Conn) { if idx < len(s.Slots) && s.Slots[idx] != nil { slot := s.Slots[idx] slot.Mu.Lock() - slot.IsHeld = hold if hold { + slot.IsHeld = true slot.ScreenChanged = false + } else { + slot.resetHoldAndLoopCountersLocked() } slot.Mu.Unlock() if hold { diff --git a/types.go b/types.go index c43360a..a6486fb 100644 --- a/types.go +++ b/types.go @@ -3,6 +3,7 @@ package main import ( "net" "sync" + "time" "github.com/racingmars/go3270" ) @@ -108,12 +109,14 @@ type TargetSystem struct { } type ServerConfig struct { - BindAddress string `toml:"bind_address"` - Codepage string `toml:"codepage"` - TLS bool `toml:"tls"` - CertFile string `toml:"cert_file"` - KeyFile string `toml:"key_file"` - Public bool `toml:"public"` + BindAddress string `toml:"bind_address"` + Codepage string `toml:"codepage"` + TLS bool `toml:"tls"` + CertFile string `toml:"cert_file"` + KeyFile string `toml:"key_file"` + Public bool `toml:"public"` + AutoHoldDiscardLimit int `toml:"auto_hold_discard_limit"` + AutoHoldBurstLimit int `toml:"auto_hold_burst_limit"` } type LoginConfig struct { @@ -146,17 +149,39 @@ type Config struct { Users []UserConfig `toml:"users"` } +const ( + defaultAutoHoldDiscardLimit = 5 + autoHoldDiscardWindow = 5 * time.Second + defaultAutoHoldBurstLimit = 20 + autoHoldBurstWindow = 1 * time.Second +) + type SessionSlot struct { - System TargetSystem - Upstream net.Conn - LastScreen []byte - ScreenChanged bool - IsTN3270E bool - IsBound bool - SeqNumber uint16 - IsHeld bool - MaxBufferSize int - Mu sync.Mutex + System TargetSystem + Upstream net.Conn + LastScreen []byte + ScreenChanged bool + IsTN3270E bool + IsBound bool + SeqNumber uint16 + IsHeld bool + MaxBufferSize int + DiscardCount int + DiscardWindow time.Time + WriteBurstCount int + WriteBurstWindow time.Time + AutoHoldDiscardLimit int + AutoHoldBurstLimit int + Mu sync.Mutex +} + +func (s *SessionSlot) resetHoldAndLoopCountersLocked() { + s.IsHeld = false + s.ScreenChanged = false + s.DiscardCount = 0 + s.DiscardWindow = time.Time{} + s.WriteBurstCount = 0 + s.WriteBurstWindow = time.Time{} } type slotRenderInfo struct {