Subject: [PATCH links] Adding domain middleware integration test to help test for db transaction leaks.

From: Peter Sanchez <[email-masked]>
Date: Thu, 4 Sep 2025 18:31:14 -0600

gobwebs bump
---
 domain/integration_test.go | 200 +++++++++++++++++++++++++++++++++++++
 go.mod                     |  14 +--
 go.sum                     |  36 +++----
 3 files changed, 219 insertions(+), 31 deletions(-)
 create mode 100644 domain/integration_test.go

diff --git a/domain/integration_test.go b/domain/integration_test.go
new file mode 100644
index 0000000..6f671d7
--- /dev/null
+++ b/domain/integration_test.go
@@ -0,0 +1,200 @@
+package domain
+
+import (
+	"context"
+	"database/sql"
+	"links/models"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"testing"
+	"time"
+
+	"github.com/labstack/echo/v4"
+	"netlandish.com/x/gobwebs/config"
+	"netlandish.com/x/gobwebs/database"
+	"netlandish.com/x/gobwebs/server"
+	"netlandish.com/x/gobwebs/timezone"
+
+	_ "github.com/lib/pq"
+)
+
+// testDB creates a test database connection for domain tests
+func testDB(t *testing.T) *sql.DB {
+	t.Helper()
+
+	// Try to get test database URL from environment
+	dbURL := os.Getenv("TEST_DATABASE_URL")
+	if dbURL == "" {
+		// Default to local test database
+		dbURL = "postgres://postgres:postgres@localhost/links_test?sslmode=disable"
+	}
+
+	db, err := sql.Open("postgres", dbURL)
+	if err != nil {
+		t.Skip("Test database not available")
+	}
+
+	// Verify connection
+	if err := db.Ping(); err != nil {
+		t.Skip("Test database not available")
+	}
+
+	// Clean up on test completion
+	t.Cleanup(func() {
+		db.Close()
+	})
+
+	return db
+}
+
+// TestDomainValidationContextCancellation tests the exact scenario that was causing transaction leaks:
+// Domain validation middleware calling ValidDomain() with context cancellation during database lookup
+func TestDomainValidationContextCancellation(t *testing.T) {
+	db := testDB(t)
+
+	// Set up basic test data - create a test domain
+	// First try to delete any existing test domain
+	db.Exec("DELETE FROM domains WHERE lookup_name = 'test.example.com'")
+	
+	_, err := db.Exec(`
+		INSERT INTO domains (lookup_name, name, service, status, level, is_active)
+		VALUES ($1, $2, $3, $4, $5, $6)
+	`, "test.example.com", "test.example.com", models.DomainServiceLinks, models.DomainStatusApproved, models.DomainLevelSystem, true)
+	if err != nil {
+		t.Fatalf("Failed to create test domain: %v", err)
+	}
+
+	// Clean up after test
+	defer func() {
+		db.Exec("DELETE FROM domains WHERE lookup_name = 'test.example.com'")
+	}()
+
+	// Track connection stats before test
+	statsBefore := db.Stats()
+
+	// Create Echo server with middleware stack similar to production
+	e := echo.New()
+	cfg := &config.Config{Debug: true}
+	server.New(e, db, cfg).Initialize()
+
+	// Add middleware in same order as production  
+	e.Use(
+		// Custom context middleware (matches server.DefaultMiddlewareWithConfig)
+		func(next echo.HandlerFunc) echo.HandlerFunc {
+			return func(c echo.Context) error {
+				ctx := &server.Context{Context: c, Server: server.New(e, db, cfg)}
+				return next(ctx)
+			}
+		},
+		database.DBIMiddleware(db),                               // DBI must be first
+		func(next echo.HandlerFunc) echo.HandlerFunc {           // Add timezone context
+			return func(c echo.Context) error {
+				ctx := timezone.Context(c.Request().Context(), "UTC")
+				c.SetRequest(c.Request().WithContext(ctx))
+				return next(c)
+			}
+		},
+		DomainContext(models.DomainServiceLinks),                 // Domain validation
+	)
+
+	// Simple handler that should never be reached due to domain validation failure
+	e.GET("/test", func(c echo.Context) error {
+		return c.String(http.StatusOK, "success")
+	})
+
+	// Test 1: Context cancellation during domain validation
+	req := httptest.NewRequest(http.MethodGet, "/test", nil)
+	req.Host = "test.example.com" // Valid domain that exists in DB
+	
+	// Create cancellable context
+	ctx, cancel := context.WithCancel(req.Context())
+	req = req.WithContext(ctx)
+	rec := httptest.NewRecorder()
+
+	// Start request processing in goroutine
+	done := make(chan bool)
+	go func() {
+		defer func() { done <- true }()
+		e.ServeHTTP(rec, req)
+	}()
+
+	// Cancel context immediately to simulate client disconnect
+	// This should trigger the "context canceled" error in ValidDomain()
+	cancel()
+
+	// Wait for request to complete
+	<-done
+
+	// The request should fail (exact status depends on error handling)
+	if rec.Code == http.StatusOK {
+		t.Error("Expected request to fail due to context cancellation")
+	}
+
+	// Wait for cleanup
+	time.Sleep(300 * time.Millisecond)
+
+	// Verify no connection leak occurred - this is the critical test
+	statsAfter := db.Stats()
+	if statsAfter.OpenConnections > statsBefore.OpenConnections {
+		t.Errorf("Connection leak detected after context cancellation: before=%d, after=%d", 
+			statsBefore.OpenConnections, statsAfter.OpenConnections)
+	}
+
+	t.Logf("Context cancellation test - Connection stats before: %d, after: %d", 
+		statsBefore.OpenConnections, statsAfter.OpenConnections)
+
+	// Test 2: Multiple rapid cancellations (stress test)
+	for i := 0; i < 5; i++ {
+		req := httptest.NewRequest(http.MethodGet, "/test", nil)
+		req.Host = "test.example.com"
+		
+		ctx, cancel := context.WithCancel(req.Context())
+		req = req.WithContext(ctx)
+		rec := httptest.NewRecorder()
+
+		go func() {
+			e.ServeHTTP(rec, req)
+		}()
+
+		// Cancel quickly
+		cancel()
+		time.Sleep(50 * time.Millisecond)
+	}
+
+	// Final connection check after stress test
+	time.Sleep(500 * time.Millisecond)
+	statsFinal := db.Stats()
+	
+	if statsFinal.OpenConnections > statsBefore.OpenConnections+2 { // Allow some tolerance
+		t.Errorf("Connection leak detected after stress test: before=%d, final=%d", 
+			statsBefore.OpenConnections, statsFinal.OpenConnections)
+	}
+
+	t.Logf("Stress test complete - Connection stats before: %d, final: %d", 
+		statsBefore.OpenConnections, statsFinal.OpenConnections)
+}
+
+// TestValidDomainWithContextCancellation tests the ValidDomain function directly
+func TestValidDomainWithContextCancellation(t *testing.T) {
+	db := testDB(t)
+
+	// Create context with both DBI and timezone (required by models.GetDomains)
+	ctx := database.DBIContext(context.Background(), database.NewDBI(db))
+	ctx = timezone.Context(ctx, "UTC") // Add required timezone context
+	ctx, cancel := context.WithCancel(ctx)
+
+	// Cancel context immediately
+	cancel()
+
+	// Call ValidDomain with cancelled context
+	_, err := ValidDomain(ctx, "test.example.com", models.DomainServiceLinks, false)
+	
+	// Should get context cancelled error
+	if err == nil {
+		t.Error("Expected error due to context cancellation")
+	}
+
+	// The key is that this doesn't leak connections
+	// The WithTx fix should handle the rollback automatically
+}
\ No newline at end of file
diff --git a/go.mod b/go.mod
index f1f6fb8..f00b0af 100644
--- a/go.mod
+++ b/go.mod
@@ -13,6 +13,7 @@ require (
 	github.com/alexedwards/scs/v2 v2.8.0
 	github.com/jarcoal/httpmock v1.3.0
 	github.com/labstack/echo/v4 v4.13.3
+	github.com/lib/pq v1.10.9
 	github.com/mattermost/mattermost-plugin-apps v1.1.0
 	github.com/mattermost/mattermost-server/v6 v6.6.0
 	github.com/oschwald/geoip2-golang v1.9.0
@@ -30,12 +31,12 @@ require (
 	golang.org/x/text v0.27.0
 	golang.org/x/time v0.8.0
 	hg.code.netlandish.com/~netlandish/sendygo v0.0.0-20230124192435-bbf347776232
-	netlandish.com/x/gobwebs v0.1.7
-	netlandish.com/x/gobwebs-auditlog v0.2.4
-	netlandish.com/x/gobwebs-formguard v0.2.5
-	netlandish.com/x/gobwebs-graphql v0.2.5
-	netlandish.com/x/gobwebs-oauth2 v0.2.4
-	netlandish.com/x/gobwebs-ses-feedback v0.2.5
+	netlandish.com/x/gobwebs v0.1.8
+	netlandish.com/x/gobwebs-auditlog v0.2.5
+	netlandish.com/x/gobwebs-formguard v0.2.6
+	netlandish.com/x/gobwebs-graphql v0.2.6
+	netlandish.com/x/gobwebs-oauth2 v0.2.5
+	netlandish.com/x/gobwebs-ses-feedback v0.2.6
 	petersanchez.com/x/carrier v0.2.3
 	petersanchez.com/x/carrier/ses v0.0.0-20250114214955-7f5d9b835a85
 	petersanchez.com/x/carrier/smtp v0.0.0-20250114214955-7f5d9b835a85
@@ -109,7 +110,6 @@ require (
 	github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect
 	github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect
 	github.com/leodido/go-urn v1.4.0 // indirect
-	github.com/lib/pq v1.10.9 // indirect
 	github.com/mattermost/go-i18n v1.11.1-0.20211013152124-5c415071e404 // indirect
 	github.com/mattermost/ldap v0.0.0-20201202150706-ee0e6284187d // indirect
 	github.com/mattermost/logr/v2 v2.0.15 // indirect
diff --git a/go.sum b/go.sum
index d16509f..d101e72 100644
--- a/go.sum
+++ b/go.sum
@@ -2578,30 +2578,18 @@ modernc.org/z v1.0.1-0.20210308123920-1f282aa71362/go.mod h1:8/SRk5C/HgiQWCgXdfp
 modernc.org/z v1.0.1/go.mod h1:8/SRk5C/HgiQWCgXdfpb+1RvhORdkz5sw72d3jjtyqA=
 modernc.org/z v1.2.20/go.mod h1:zU9FiF4PbHdOTUxw+IF8j7ArBMRPsHgq10uVPt6xTzo=
 modernc.org/zappy v1.0.0/go.mod h1:hHe+oGahLVII/aTTyWK/b53VDHMAGCBYYeZ9sn83HC4=
-netlandish.com/x/gobwebs v0.1.6 h1:/fXVU2hoS6FyIk/Refl1FqxCGJxjcCvCybnrxr2JYlI=
-netlandish.com/x/gobwebs v0.1.6/go.mod h1:RAf0VNiujzXen/pu0k6yyhvPWMgCY9tKD7ftXrkT53E=
-netlandish.com/x/gobwebs v0.1.7 h1:YPqieX9F8lQpjBRq9mrhG4ZKA6SoWMZ0Inz2xu5ZSOM=
-netlandish.com/x/gobwebs v0.1.7/go.mod h1:RAf0VNiujzXen/pu0k6yyhvPWMgCY9tKD7ftXrkT53E=
-netlandish.com/x/gobwebs-auditlog v0.2.3 h1:U9Kcbacy+6gUbm2wwXUKg4q7RpbRwHgaLXebQWiYlXk=
-netlandish.com/x/gobwebs-auditlog v0.2.3/go.mod h1:E5K6BDkD6BTgwGDz2GWOE/h/PfKuyswLDnhU1gDWNPQ=
-netlandish.com/x/gobwebs-auditlog v0.2.4 h1:IloSnIc8yGKeEwX5ylKx0I3VoLoa9Bezf98XUnLFIN0=
-netlandish.com/x/gobwebs-auditlog v0.2.4/go.mod h1:l1EJbQ79+vWc6l6yFnzRjE4NSRjGTAiwLPxUPQqzy/w=
-netlandish.com/x/gobwebs-formguard v0.2.4 h1:YkLudnPrwr9k/giURyohnjlfbuC+Ws8hnrrU+MY8vUc=
-netlandish.com/x/gobwebs-formguard v0.2.4/go.mod h1:0UZ/lWNAqHCwDYotp2WlX0XgfsF8PfV8XBr29kSJmP8=
-netlandish.com/x/gobwebs-formguard v0.2.5 h1:+Fa5T2qeP2ldv+sIXCVqsLFzp8NXQyI8ZvxVD5TsOA4=
-netlandish.com/x/gobwebs-formguard v0.2.5/go.mod h1:Qv+qymuUOCgKzhNmk31DOxT4PLu5bEwXFe2tMEBaljA=
-netlandish.com/x/gobwebs-graphql v0.2.4 h1:BWuWiTkTVJNdBITV36r42w7Uh+6HGgZvxOj3+XICsF4=
-netlandish.com/x/gobwebs-graphql v0.2.4/go.mod h1:vegE1Sb69L19qk/Urx7HWFhJ3FCjZZ5e0n5PlXDE1Qc=
-netlandish.com/x/gobwebs-graphql v0.2.5 h1:h1tr7PP+PbKABD+gP58ldi6GMPRLjAzp6sjXpcozEKg=
-netlandish.com/x/gobwebs-graphql v0.2.5/go.mod h1:nNGgCWC098JgaASCsvzZLl1rePzXgGcUtBEB+eJThhg=
-netlandish.com/x/gobwebs-oauth2 v0.2.3 h1:gtxVwa/TFquvPm7sL9CYPN6qF9dayovK09bhv8s/bkE=
-netlandish.com/x/gobwebs-oauth2 v0.2.3/go.mod h1:OXmkl1iL3MMy4CzdtAc9oOQbX/2kbR8/2t2GCahHBho=
-netlandish.com/x/gobwebs-oauth2 v0.2.4 h1:B5V5HNCeRU3TNXK0/59Vnc5Dn7nOA54snxXDnBjn+0g=
-netlandish.com/x/gobwebs-oauth2 v0.2.4/go.mod h1:LqCD8OZJkFr8rcDaNYeX8L6tsv/joAQ1k1iriCUlf+E=
-netlandish.com/x/gobwebs-ses-feedback v0.2.4 h1:0jSGszEAKxKOJrhiO/vQuuY6lQv7fcsP4s6Iw5EcOJk=
-netlandish.com/x/gobwebs-ses-feedback v0.2.4/go.mod h1:s7BCQ4lYPLdvzDuDvZmQn+7IHKsHNIkecs4jcjoZ0ZM=
-netlandish.com/x/gobwebs-ses-feedback v0.2.5 h1:o/9aP9CkijohA61834S2AFPk8mN07SkytLvoSAybOeY=
-netlandish.com/x/gobwebs-ses-feedback v0.2.5/go.mod h1:Nj7NSW936hRXq2QL1wa95C+g03GLhBIV5MN2K7zorGk=
+netlandish.com/x/gobwebs v0.1.8 h1:hCTpIeNxAccBWq00X3PmwSd9juu/xo2oQjqocfI+nRU=
+netlandish.com/x/gobwebs v0.1.8/go.mod h1:RAf0VNiujzXen/pu0k6yyhvPWMgCY9tKD7ftXrkT53E=
+netlandish.com/x/gobwebs-auditlog v0.2.5 h1:ABrrl/ghIRVZyTEEqJsLNH6Fi4gtbkvpjUQoFcGU6zs=
+netlandish.com/x/gobwebs-auditlog v0.2.5/go.mod h1:s1H4a8JdeXfaaU/q5tHzoXxNka7PyWGOdwochjAD47M=
+netlandish.com/x/gobwebs-formguard v0.2.6 h1:MOucfAQP1EK/PKHks5FnELwBFDrxdITaGRNPFGdiaoc=
+netlandish.com/x/gobwebs-formguard v0.2.6/go.mod h1:7CiNHjRh7/vh6YKzWcZmBao3dc9uSw9RNg+PhHNtPFY=
+netlandish.com/x/gobwebs-graphql v0.2.6 h1:MXlKxFEoTayvw9Ro5nyHSSqgNVdXSqIJk3lADLuv/wQ=
+netlandish.com/x/gobwebs-graphql v0.2.6/go.mod h1:rOH7fKL4DTeNy6XzVGyrLiamZmADsU1J1eIpKbdxtBs=
+netlandish.com/x/gobwebs-oauth2 v0.2.5 h1:SPGp/HzC6UYfNES+8YcsSO23ElcMbCcT2bVXydOT8yA=
+netlandish.com/x/gobwebs-oauth2 v0.2.5/go.mod h1:NO4ewCxCwv5EwACf1zRKdg5tQpk6UHLch1RQLFAGWc8=
+netlandish.com/x/gobwebs-ses-feedback v0.2.6 h1:dTx8IXCqu9s7JovUWhQsZ2A+AoJLg1E3/Hy5sKZDSb8=
+netlandish.com/x/gobwebs-ses-feedback v0.2.6/go.mod h1:cls3Jkuwj3yaHC85XCf9bpzAgfHZjFj/tEwcjMZyezE=
 petersanchez.com/x/carrier v0.2.3 h1:6ScWG2HVFqeqafQp2D9kChNdXYkou4rduzppc3SDYMg=
 petersanchez.com/x/carrier v0.2.3/go.mod h1:GLiDI9OThDmruufk/VHlR6Ihvq/hIJQyA5beU6AFNYk=
 petersanchez.com/x/carrier/ses v0.0.0-20250114214955-7f5d9b835a85 h1:yy81/rrGdI+YWuTvv7JPvVnY104/g10vnloBnKxVAHk=
-- 
2.49.1