Merge pull request #303 from bnema/fix/audit-hardening
fix(audit): harden local browser boundaries
27 files · +1045 -197
M .github/workflows/ci.yml +20 -4
| @@ -1,5 +1,8 @@ | |||
| 1 | 1 | name: CI | |
| 2 | 2 | ||
| 3 | + | permissions: | |
| 4 | + | contents: read | |
| 5 | + | ||
| 3 | 6 | on: | |
| 4 | 7 | push: | |
| 5 | 8 | branches: [main] | |
| @@ -49,6 +52,13 @@ jobs: | |||
| 49 | 52 | pacman -Syu --noconfirm | |
| 50 | 53 | pacman -S --noconfirm git go webkitgtk-6.0 gtk4 gobject-introspection base-devel brotli | |
| 51 | 54 | - uses: actions/checkout@v6 | |
| 55 | + | - name: Allow git commands in container checkout | |
| 56 | + | run: git config --global --add safe.directory "$GITHUB_WORKSPACE" | |
| 57 | + | - name: Set up pinned Go toolchain | |
| 58 | + | uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c | |
| 59 | + | with: | |
| 60 | + | go-version-file: go.mod | |
| 61 | + | cache: false | |
| 52 | 62 | - name: Cache Go modules | |
| 53 | 63 | uses: actions/cache@v5 | |
| 54 | 64 | with: | |
| @@ -65,13 +75,14 @@ jobs: | |||
| 65 | 75 | ${{ runner.os }}-go-build- | |
| 66 | 76 | - name: Build application and systemviews assets | |
| 67 | 77 | run: make build | |
| 78 | + | - name: Verify generated systemviews artifacts | |
| 79 | + | run: make verify-generated | |
| 68 | 80 | - name: Install golangci-lint | |
| 69 | 81 | run: | | |
| 70 | - | curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b /usr/local/bin latest | |
| 82 | + | make install-golangci-lint | |
| 83 | + | echo "$(go env GOPATH)/bin" >> "$GITHUB_PATH" | |
| 71 | 84 | - name: Run golangci-lint | |
| 72 | - | run: | | |
| 73 | - | git config --global --add safe.directory "$GITHUB_WORKSPACE" | |
| 74 | - | golangci-lint run --timeout=20m | |
| 85 | + | run: golangci-lint run --timeout=20m | |
| 75 | 86 | ||
| 76 | 87 | test: | |
| 77 | 88 | needs: changes | |
| @@ -85,6 +96,11 @@ jobs: | |||
| 85 | 96 | pacman -Syu --noconfirm | |
| 86 | 97 | pacman -S --noconfirm git go webkitgtk-6.0 gtk4 libadwaita gobject-introspection base-devel brotli dbus | |
| 87 | 98 | - uses: actions/checkout@v6 | |
| 99 | + | - name: Set up pinned Go toolchain | |
| 100 | + | uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c | |
| 101 | + | with: | |
| 102 | + | go-version-file: go.mod | |
| 103 | + | cache: false | |
| 88 | 104 | - name: Cache Go modules | |
| 89 | 105 | uses: actions/cache@v5 | |
| 90 | 106 | with: | |
| @@ -1,5 +1,8 @@ | |||||
| 1 | name: CI | 1 | name: CI | ||
| 2 | 2 | ||||
| 3 | + | permissions: | |||
| 4 | + | contents: read | |||
| 5 | + | ||||
| 3 | on: | 6 | on: | ||
| 4 | push: | 7 | push: | ||
| 5 | branches: [main] | 8 | branches: [main] | ||
| @@ -49,6 +52,13 @@ jobs: | |||||
| 49 | pacman -Syu --noconfirm | 52 | pacman -Syu --noconfirm | ||
| 50 | pacman -S --noconfirm git go webkitgtk-6.0 gtk4 gobject-introspection base-devel brotli | 53 | pacman -S --noconfirm git go webkitgtk-6.0 gtk4 gobject-introspection base-devel brotli | ||
| 51 | - uses: actions/checkout@v6 | 54 | - uses: actions/checkout@v6 | ||
| 55 | + | - name: Allow git commands in container checkout | |||
| 56 | + | run: git config --global --add safe.directory "$GITHUB_WORKSPACE" | |||
| 57 | + | - name: Set up pinned Go toolchain | |||
| 58 | + | uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c | |||
| 59 | + | with: | |||
| 60 | + | go-version-file: go.mod | |||
| 61 | + | cache: false | |||
| 52 | - name: Cache Go modules | 62 | - name: Cache Go modules | ||
| 53 | uses: actions/cache@v5 | 63 | uses: actions/cache@v5 | ||
| 54 | with: | 64 | with: | ||
| @@ -65,13 +75,14 @@ jobs: | |||||
| 65 | ${{ runner.os }}-go-build- | 75 | ${{ runner.os }}-go-build- | ||
| 66 | - name: Build application and systemviews assets | 76 | - name: Build application and systemviews assets | ||
| 67 | run: make build | 77 | run: make build | ||
| 78 | + | - name: Verify generated systemviews artifacts | |||
| 79 | + | run: make verify-generated | |||
| 68 | - name: Install golangci-lint | 80 | - name: Install golangci-lint | ||
| 69 | run: | | 81 | run: | | ||
| 70 | - | curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b /usr/local/bin latest | 82 | + | make install-golangci-lint |
| 83 | + | echo "$(go env GOPATH)/bin" >> "$GITHUB_PATH" | |||
| 71 | - name: Run golangci-lint | 84 | - name: Run golangci-lint | ||
| 72 | - | run: | | 85 | + | run: golangci-lint run --timeout=20m |
| 73 | - | git config --global --add safe.directory "$GITHUB_WORKSPACE" | |||
| 74 | - | golangci-lint run --timeout=20m | |||
| 75 | 86 | ||||
| 76 | test: | 87 | test: | ||
| 77 | needs: changes | 88 | needs: changes | ||
| @@ -85,6 +96,11 @@ jobs: | |||||
| 85 | pacman -Syu --noconfirm | 96 | pacman -Syu --noconfirm | ||
| 86 | pacman -S --noconfirm git go webkitgtk-6.0 gtk4 libadwaita gobject-introspection base-devel brotli dbus | 97 | pacman -S --noconfirm git go webkitgtk-6.0 gtk4 libadwaita gobject-introspection base-devel brotli dbus | ||
| 87 | - uses: actions/checkout@v6 | 98 | - uses: actions/checkout@v6 | ||
| 99 | + | - name: Set up pinned Go toolchain | |||
| 100 | + | uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c | |||
| 101 | + | with: | |||
| 102 | + | go-version-file: go.mod | |||
| 103 | + | cache: false | |||
| 88 | - name: Cache Go modules | 104 | - name: Cache Go modules | ||
| 89 | uses: actions/cache@v5 | 105 | uses: actions/cache@v5 | ||
| 90 | with: | 106 | with: | ||
M Makefile +28 -3
| @@ -1,6 +1,6 @@ | |||
| 1 | 1 | # Makefile for dumber (Clean Architecture - puregotk) | |
| 2 | 2 | ||
| 3 | - | .PHONY: build build-systemviews generate-systemviews build-quick install-local test lint clean install-tools dev generate help init man flatpak-deps flatpak-build flatpak-install flatpak-run flatpak-clean stress-omnibox-callbacks verify-purego check | |
| 3 | + | .PHONY: build build-systemviews generate-systemviews build-quick install-local test lint staticcheck clean install-tools install-golangci-lint install-staticcheck dev generate help init man flatpak-deps flatpak-build flatpak-install flatpak-run flatpak-clean stress-omnibox-callbacks verify-purego verify-generated check | |
| 4 | 4 | ||
| 5 | 5 | # Load local overrides from .env.local if present (Makefile syntax) | |
| 6 | 6 | ifneq (,$(wildcard .env.local)) | |
| @@ -13,6 +13,9 @@ BINARY_NAME=dumber | |||
| 13 | 13 | MAIN_PATH=./cmd/dumber | |
| 14 | 14 | DIST_DIR=dist | |
| 15 | 15 | LOCAL_BIN_DIR?=$(HOME)/.local/bin | |
| 16 | + | TOOL_BIN_DIR?=$(shell go env GOPATH)/bin | |
| 17 | + | GOLANGCI_LINT_VERSION?=v2.12.2 | |
| 18 | + | STATICCHECK_VERSION?=v0.7.0 | |
| 16 | 19 | ||
| 17 | 20 | # Detect number of CPU cores for parallel compilation | |
| 18 | 21 | NPROCS?=$(shell nproc 2>/dev/null || echo 1) | |
| @@ -126,6 +129,13 @@ stress-omnibox-callbacks: ## Run placeholder omnibox callback stress harness | |||
| 126 | 129 | verify-purego: ## Ensure callback path stays cgo/export free | |
| 127 | 130 | bash ./scripts/verify_purego_only.sh | |
| 128 | 131 | ||
| 132 | + | verify-generated: ## Verify generated systemviews artifacts are committed | |
| 133 | + | @echo "Verifying generated systemviews artifacts..." | |
| 134 | + | @git diff --exit-code -- assets/systemviews internal/ui/systemviews || { \ | |
| 135 | + | echo "Generated systemviews artifacts are out of date. Run 'make build-systemviews' and commit the result."; \ | |
| 136 | + | exit 1; \ | |
| 137 | + | } | |
| 138 | + | ||
| 129 | 139 | # Linting | |
| 130 | 140 | lint: ## Run golangci-lint | |
| 131 | 141 | @echo "Running golangci-lint..." | |
| @@ -135,16 +145,31 @@ lint-fix: ## Run golangci-lint with --fix | |||
| 135 | 145 | @echo "Running golangci-lint with --fix..." | |
| 136 | 146 | golangci-lint run --fix | |
| 137 | 147 | ||
| 148 | + | staticcheck: ## Run Staticcheck | |
| 149 | + | @echo "Running staticcheck..." | |
| 150 | + | staticcheck ./... | |
| 151 | + | ||
| 138 | 152 | # Format code | |
| 139 | 153 | fmt: ## Format Go code with gofmt | |
| 140 | 154 | @echo "Formatting Go code..." | |
| 141 | 155 | go fmt ./... | |
| 142 | 156 | ||
| 143 | 157 | # Tools installation | |
| 144 | - | install-tools: ## Install development tools | |
| 158 | + | install-golangci-lint: ## Install pinned golangci-lint | |
| 159 | + | @echo "Installing golangci-lint $(GOLANGCI_LINT_VERSION) to $(TOOL_BIN_DIR)..." | |
| 160 | + | @mkdir -p $(TOOL_BIN_DIR) | |
| 161 | + | # Build the pinned version with the active Go toolchain instead of using the | |
| 162 | + | # upstream install.sh helper, which currently misidentifies release assets now | |
| 163 | + | # that golangci-lint publishes sibling .sbom.json files for each tarball. | |
| 164 | + | GOBIN=$(TOOL_BIN_DIR) go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) | |
| 165 | + | ||
| 166 | + | install-staticcheck: ## Install pinned Staticcheck with the active Go toolchain | |
| 167 | + | @echo "Installing staticcheck $(STATICCHECK_VERSION)..." | |
| 168 | + | go install honnef.co/go/tools/cmd/staticcheck@$(STATICCHECK_VERSION) | |
| 169 | + | ||
| 170 | + | install-tools: install-golangci-lint install-staticcheck ## Install development tools | |
| 145 | 171 | @echo "Installing development tools..." | |
| 146 | 172 | go install github.com/sqlc-dev/sqlc/cmd/sqlc@latest | |
| 147 | - | go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest | |
| 148 | 173 | @echo "Tools installed successfully" | |
| 149 | 174 | ||
| 150 | 175 | # Cleanup | |
| @@ -1,6 +1,6 @@ | |||||
| 1 | # Makefile for dumber (Clean Architecture - puregotk) | 1 | # Makefile for dumber (Clean Architecture - puregotk) | ||
| 2 | 2 | ||||
| 3 | - | .PHONY: build build-systemviews generate-systemviews build-quick install-local test lint clean install-tools dev generate help init man flatpak-deps flatpak-build flatpak-install flatpak-run flatpak-clean stress-omnibox-callbacks verify-purego check | 3 | + | .PHONY: build build-systemviews generate-systemviews build-quick install-local test lint staticcheck clean install-tools install-golangci-lint install-staticcheck dev generate help init man flatpak-deps flatpak-build flatpak-install flatpak-run flatpak-clean stress-omnibox-callbacks verify-purego verify-generated check |
| 4 | 4 | ||||
| 5 | # Load local overrides from .env.local if present (Makefile syntax) | 5 | # Load local overrides from .env.local if present (Makefile syntax) | ||
| 6 | ifneq (,$(wildcard .env.local)) | 6 | ifneq (,$(wildcard .env.local)) | ||
| @@ -13,6 +13,9 @@ BINARY_NAME=dumber | |||||
| 13 | MAIN_PATH=./cmd/dumber | 13 | MAIN_PATH=./cmd/dumber | ||
| 14 | DIST_DIR=dist | 14 | DIST_DIR=dist | ||
| 15 | LOCAL_BIN_DIR?=$(HOME)/.local/bin | 15 | LOCAL_BIN_DIR?=$(HOME)/.local/bin | ||
| 16 | + | TOOL_BIN_DIR?=$(shell go env GOPATH)/bin | |||
| 17 | + | GOLANGCI_LINT_VERSION?=v2.12.2 | |||
| 18 | + | STATICCHECK_VERSION?=v0.7.0 | |||
| 16 | 19 | ||||
| 17 | # Detect number of CPU cores for parallel compilation | 20 | # Detect number of CPU cores for parallel compilation | ||
| 18 | NPROCS?=$(shell nproc 2>/dev/null || echo 1) | 21 | NPROCS?=$(shell nproc 2>/dev/null || echo 1) | ||
| @@ -126,6 +129,13 @@ stress-omnibox-callbacks: ## Run placeholder omnibox callback stress harness | |||||
| 126 | verify-purego: ## Ensure callback path stays cgo/export free | 129 | verify-purego: ## Ensure callback path stays cgo/export free | ||
| 127 | bash ./scripts/verify_purego_only.sh | 130 | bash ./scripts/verify_purego_only.sh | ||
| 128 | 131 | ||||
| 132 | + | verify-generated: ## Verify generated systemviews artifacts are committed | |||
| 133 | + | @echo "Verifying generated systemviews artifacts..." | |||
| 134 | + | @git diff --exit-code -- assets/systemviews internal/ui/systemviews || { \ | |||
| 135 | + | echo "Generated systemviews artifacts are out of date. Run 'make build-systemviews' and commit the result."; \ | |||
| 136 | + | exit 1; \ | |||
| 137 | + | } | |||
| 138 | + | ||||
| 129 | # Linting | 139 | # Linting | ||
| 130 | lint: ## Run golangci-lint | 140 | lint: ## Run golangci-lint | ||
| 131 | @echo "Running golangci-lint..." | 141 | @echo "Running golangci-lint..." | ||
| @@ -135,16 +145,31 @@ lint-fix: ## Run golangci-lint with --fix | |||||
| 135 | @echo "Running golangci-lint with --fix..." | 145 | @echo "Running golangci-lint with --fix..." | ||
| 136 | golangci-lint run --fix | 146 | golangci-lint run --fix | ||
| 137 | 147 | ||||
| 148 | + | staticcheck: ## Run Staticcheck | |||
| 149 | + | @echo "Running staticcheck..." | |||
| 150 | + | staticcheck ./... | |||
| 151 | + | ||||
| 138 | # Format code | 152 | # Format code | ||
| 139 | fmt: ## Format Go code with gofmt | 153 | fmt: ## Format Go code with gofmt | ||
| 140 | @echo "Formatting Go code..." | 154 | @echo "Formatting Go code..." | ||
| 141 | go fmt ./... | 155 | go fmt ./... | ||
| 142 | 156 | ||||
| 143 | # Tools installation | 157 | # Tools installation | ||
| 144 | - | install-tools: ## Install development tools | 158 | + | install-golangci-lint: ## Install pinned golangci-lint |
| 159 | + | @echo "Installing golangci-lint $(GOLANGCI_LINT_VERSION) to $(TOOL_BIN_DIR)..." | |||
| 160 | + | @mkdir -p $(TOOL_BIN_DIR) | |||
| 161 | + | # Build the pinned version with the active Go toolchain instead of using the | |||
| 162 | + | # upstream install.sh helper, which currently misidentifies release assets now | |||
| 163 | + | # that golangci-lint publishes sibling .sbom.json files for each tarball. | |||
| 164 | + | GOBIN=$(TOOL_BIN_DIR) go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) | |||
| 165 | + | ||||
| 166 | + | install-staticcheck: ## Install pinned Staticcheck with the active Go toolchain | |||
| 167 | + | @echo "Installing staticcheck $(STATICCHECK_VERSION)..." | |||
| 168 | + | go install honnef.co/go/tools/cmd/staticcheck@$(STATICCHECK_VERSION) | |||
| 169 | + | ||||
| 170 | + | install-tools: install-golangci-lint install-staticcheck ## Install development tools | |||
| 145 | @echo "Installing development tools..." | 171 | @echo "Installing development tools..." | ||
| 146 | go install github.com/sqlc-dev/sqlc/cmd/sqlc@latest | 172 | go install github.com/sqlc-dev/sqlc/cmd/sqlc@latest | ||
| 147 | - | go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest | |||
| 148 | @echo "Tools installed successfully" | 173 | @echo "Tools installed successfully" | ||
| 149 | 174 | ||||
| 150 | # Cleanup | 175 | # Cleanup | ||
M README.md +11 -5
| @@ -208,9 +208,11 @@ Set `ENV=dev` to use `.dev/dumber/` for config and data instead of XDG paths. | |||
| 208 | 208 | ||
| 209 | 209 | **Prerequisites:** | |
| 210 | 210 | ||
| 211 | - | - Go 1.25+ | |
| 212 | - | - Node.js 20+ for frontend assets | |
| 211 | + | - Go 1.26+ | |
| 213 | 212 | - WebKitGTK 6.0 and GTK4 development packages | |
| 213 | + | - Brotli for compressed systemviews assets | |
| 214 | + | ||
| 215 | + | Systemviews assets are generated with `go tool templ` and Go's `js/wasm` toolchain; no root Node toolchain is required. | |
| 214 | 216 | ||
| 215 | 217 | ```bash | |
| 216 | 218 | git clone https://github.com/bnema/dumber | |
| @@ -223,13 +225,17 @@ make build | |||
| 223 | 225 | ||
| 224 | 226 | | Target | Description | | |
| 225 | 227 | |--------|-------------| | |
| 226 | - | | `make build` | Build frontend and binary | | |
| 227 | - | | `make build-quick` | Build binary only, skipping frontend | | |
| 228 | + | | `make build` | Build systemviews assets and binary | | |
| 229 | + | | `make build-quick` | Build binary only, skipping systemviews assets | | |
| 228 | 230 | | `make dev` | Run with `go run` | | |
| 229 | 231 | | `make test` | Run tests | | |
| 230 | - | | `make lint` | Run golangci-lint | | |
| 232 | + | | `make lint` | Run the pinned golangci-lint version | | |
| 233 | + | | `make staticcheck` | Run Staticcheck with the pinned tool version | | |
| 234 | + | | `make verify-generated` | Verify generated systemviews artifacts are committed | | |
| 231 | 235 | | `make flatpak-build` | Build Flatpak bundle | | |
| 232 | 236 | ||
| 237 | + | Development tool versions are pinned in `Makefile` (`GOLANGCI_LINT_VERSION`, `STATICCHECK_VERSION`). Bump those values intentionally when refreshing lint/static analysis tooling. | |
| 238 | + | ||
| 233 | 239 | ## Contributing | |
| 234 | 240 | ||
| 235 | 241 | Dumber is currently focused on stability, performance, browser-engine behavior, and core UI/UX polish. | |
| @@ -208,9 +208,11 @@ Set `ENV=dev` to use `.dev/dumber/` for config and data instead of XDG paths. | |||||
| 208 | 208 | ||||
| 209 | **Prerequisites:** | 209 | **Prerequisites:** | ||
| 210 | 210 | ||||
| 211 | - | - Go 1.25+ | 211 | + | - Go 1.26+ |
| 212 | - | - Node.js 20+ for frontend assets | |||
| 213 | - WebKitGTK 6.0 and GTK4 development packages | 212 | - WebKitGTK 6.0 and GTK4 development packages | ||
| 213 | + | - Brotli for compressed systemviews assets | |||
| 214 | + | ||||
| 215 | + | Systemviews assets are generated with `go tool templ` and Go's `js/wasm` toolchain; no root Node toolchain is required. | |||
| 214 | 216 | ||||
| 215 | ```bash | 217 | ```bash | ||
| 216 | git clone https://github.com/bnema/dumber | 218 | git clone https://github.com/bnema/dumber | ||
| @@ -223,13 +225,17 @@ make build | |||||
| 223 | 225 | ||||
| 224 | | Target | Description | | 226 | | Target | Description | | ||
| 225 | |--------|-------------| | 227 | |--------|-------------| | ||
| 226 | - | | `make build` | Build frontend and binary | | 228 | + | | `make build` | Build systemviews assets and binary | |
| 227 | - | | `make build-quick` | Build binary only, skipping frontend | | 229 | + | | `make build-quick` | Build binary only, skipping systemviews assets | |
| 228 | | `make dev` | Run with `go run` | | 230 | | `make dev` | Run with `go run` | | ||
| 229 | | `make test` | Run tests | | 231 | | `make test` | Run tests | | ||
| 230 | - | | `make lint` | Run golangci-lint | | 232 | + | | `make lint` | Run the pinned golangci-lint version | |
| 233 | + | | `make staticcheck` | Run Staticcheck with the pinned tool version | | |||
| 234 | + | | `make verify-generated` | Verify generated systemviews artifacts are committed | | |||
| 231 | | `make flatpak-build` | Build Flatpak bundle | | 235 | | `make flatpak-build` | Build Flatpak bundle | | ||
| 232 | 236 | ||||
| 237 | + | Development tool versions are pinned in `Makefile` (`GOLANGCI_LINT_VERSION`, `STATICCHECK_VERSION`). Bump those values intentionally when refreshing lint/static analysis tooling. | |||
| 238 | + | ||||
| 233 | ## Contributing | 239 | ## Contributing | ||
| 234 | 240 | ||||
| 235 | Dumber is currently focused on stability, performance, browser-engine behavior, and core UI/UX polish. | 241 | Dumber is currently focused on stability, performance, browser-engine behavior, and core UI/UX polish. | ||
M docs/installation.md +7 -3
| @@ -43,15 +43,19 @@ flatpak install dumber.flatpak | |||
| 43 | 43 | ```bash | |
| 44 | 44 | git clone https://github.com/bnema/dumber | |
| 45 | 45 | cd dumber | |
| 46 | - | go build -o dumber ./cmd/dumber | |
| 46 | + | make build | |
| 47 | + | ./dist/dumber browse | |
| 47 | 48 | ``` | |
| 48 | 49 | ||
| 49 | 50 | ### Build Dependencies | |
| 50 | 51 | ||
| 51 | - | - Go 1.25+ | |
| 52 | + | - Go 1.26+ | |
| 52 | 53 | - GTK4 development libraries | |
| 53 | - | - WebKitGTK 2.42+ development libraries | |
| 54 | + | - WebKitGTK 6.0 development libraries | |
| 54 | 55 | - GStreamer development libraries | |
| 56 | + | - Brotli for compressed systemviews assets | |
| 57 | + | ||
| 58 | + | Systemviews assets are generated with `go tool templ` and Go's `js/wasm` toolchain; no root Node toolchain is required. | |
| 55 | 59 | ||
| 56 | 60 | ## Post-Install | |
| 57 | 61 | ||
| @@ -43,15 +43,19 @@ flatpak install dumber.flatpak | |||||
| 43 | ```bash | 43 | ```bash | ||
| 44 | git clone https://github.com/bnema/dumber | 44 | git clone https://github.com/bnema/dumber | ||
| 45 | cd dumber | 45 | cd dumber | ||
| 46 | - | go build -o dumber ./cmd/dumber | 46 | + | make build |
| 47 | + | ./dist/dumber browse | |||
| 47 | ``` | 48 | ``` | ||
| 48 | 49 | ||||
| 49 | ### Build Dependencies | 50 | ### Build Dependencies | ||
| 50 | 51 | ||||
| 51 | - | - Go 1.25+ | 52 | + | - Go 1.26+ |
| 52 | - GTK4 development libraries | 53 | - GTK4 development libraries | ||
| 53 | - | - WebKitGTK 2.42+ development libraries | 54 | + | - WebKitGTK 6.0 development libraries |
| 54 | - GStreamer development libraries | 55 | - GStreamer development libraries | ||
| 56 | + | - Brotli for compressed systemviews assets | |||
| 57 | + | ||||
| 58 | + | Systemviews assets are generated with `go tool templ` and Go's `js/wasm` toolchain; no root Node toolchain is required. | |||
| 55 | 59 | ||||
| 56 | ## Post-Install | 60 | ## Post-Install | ||
| 57 | 61 | ||||
M internal/application/usecase/search_history.go +47 -16
| @@ -18,7 +18,16 @@ import ( | |||
| 18 | 18 | // Compile-time check: SearchHistoryUseCase must satisfy port.HomepageHistory. | |
| 19 | 19 | var _ port.HomepageHistory = (*SearchHistoryUseCase)(nil) | |
| 20 | 20 | ||
| 21 | - | const historyWindowDuration = 24 * time.Hour | |
| 21 | + | const ( | |
| 22 | + | historyWindowDuration = 24 * time.Hour | |
| 23 | + | ||
| 24 | + | defaultHistoryPageLimit = 50 | |
| 25 | + | maxHistoryPageLimit = 500 | |
| 26 | + | defaultHistorySearchLimit = 20 | |
| 27 | + | maxHistorySearchLimit = 100 | |
| 28 | + | defaultDomainStatsLimit = 20 | |
| 29 | + | maxDomainStatsLimit = 100 | |
| 30 | + | ) | |
| 22 | 31 | ||
| 23 | 32 | // SearchHistoryUseCase handles history search and retrieval operations. | |
| 24 | 33 | type SearchHistoryUseCase struct { | |
| @@ -47,10 +56,7 @@ func (uc *SearchHistoryUseCase) Search(ctx context.Context, input SearchInput) ( | |||
| 47 | 56 | return &SearchOutput{Matches: []entity.HistoryMatch{}}, nil | |
| 48 | 57 | } | |
| 49 | 58 | ||
| 50 | - | limit := input.Limit | |
| 51 | - | if limit <= 0 { | |
| 52 | - | limit = 20 // Default limit | |
| 53 | - | } | |
| 59 | + | limit := clampPositiveLimit(input.Limit, defaultHistorySearchLimit, maxHistorySearchLimit) | |
| 54 | 60 | ||
| 55 | 61 | // Use repository's FTS5 search | |
| 56 | 62 | matches, err := uc.historyRepo.Search(ctx, input.Query, limit) | |
| @@ -67,11 +73,10 @@ func (uc *SearchHistoryUseCase) Search(ctx context.Context, input SearchInput) ( | |||
| 67 | 73 | } | |
| 68 | 74 | ||
| 69 | 75 | // GetRecent retrieves recent history entries. A zero limit means all entries; | |
| 70 | - | // negative limits retain the historical default page size. | |
| 76 | + | // negative limits retain the historical default page size. Positive limits are | |
| 77 | + | // capped to keep WebUI-originated queries bounded. | |
| 71 | 78 | func (uc *SearchHistoryUseCase) GetRecent(ctx context.Context, limit, offset int) ([]*entity.HistoryEntry, error) { | |
| 72 | - | if limit < 0 { | |
| 73 | - | limit = 50 // Default limit for invalid negative values. | |
| 74 | - | } | |
| 79 | + | limit = clampOptionalLimit(limit, defaultHistoryPageLimit, maxHistoryPageLimit) | |
| 75 | 80 | ||
| 76 | 81 | entries, err := uc.historyRepo.GetRecent(ctx, limit, offset) | |
| 77 | 82 | if err != nil { | |
| @@ -82,11 +87,10 @@ func (uc *SearchHistoryUseCase) GetRecent(ctx context.Context, limit, offset int | |||
| 82 | 87 | } | |
| 83 | 88 | ||
| 84 | 89 | // GetRecentByDomain retrieves recent history entries for a canonical domain. A | |
| 85 | - | // zero limit means all matching entries; negative limits use the default page size. | |
| 90 | + | // zero limit means all matching entries; negative limits use the default page | |
| 91 | + | // size. Positive limits are capped to keep WebUI-originated queries bounded. | |
| 86 | 92 | func (uc *SearchHistoryUseCase) GetRecentByDomain(ctx context.Context, domain string, limit, offset int) ([]*entity.HistoryEntry, error) { | |
| 87 | - | if limit < 0 { | |
| 88 | - | limit = 50 | |
| 89 | - | } | |
| 93 | + | limit = clampOptionalLimit(limit, defaultHistoryPageLimit, maxHistoryPageLimit) | |
| 90 | 94 | domain, err := canonicalHistoryDomain(domain) | |
| 91 | 95 | if err != nil { | |
| 92 | 96 | return nil, err | |
| @@ -298,13 +302,40 @@ func (uc *SearchHistoryUseCase) GetDomainStats(ctx context.Context, limit int) ( | |||
| 298 | 302 | log := logging.FromContext(ctx) | |
| 299 | 303 | log.Debug().Int("limit", limit).Msg("getting domain stats") | |
| 300 | 304 | ||
| 301 | - | if limit <= 0 { | |
| 302 | - | limit = 20 | |
| 303 | - | } | |
| 305 | + | limit = clampPositiveLimit(limit, defaultDomainStatsLimit, maxDomainStatsLimit) | |
| 304 | 306 | ||
| 305 | 307 | return uc.historyRepo.GetDomainStats(ctx, limit) | |
| 306 | 308 | } | |
| 307 | 309 | ||
| 310 | + | // clampOptionalLimit normalizes an optional limit where zero remains a valid | |
| 311 | + | // "no limit" value, negatives fall back to defaultLimit, and values above | |
| 312 | + | // maxLimit are capped. Use clampPositiveLimit instead when zero should be | |
| 313 | + | // treated as invalid input. | |
| 314 | + | func clampOptionalLimit(limit, defaultLimit, maxLimit int) int { | |
| 315 | + | if limit < 0 { | |
| 316 | + | return defaultLimit | |
| 317 | + | } | |
| 318 | + | if limit > maxLimit { | |
| 319 | + | return maxLimit | |
| 320 | + | } | |
| 321 | + | return limit | |
| 322 | + | } | |
| 323 | + | ||
| 324 | + | // clampPositiveLimit normalizes a required positive limit. Values less than or | |
| 325 | + | // equal to zero fall back to defaultLimit, values above maxLimit are capped, | |
| 326 | + | // and the result stays within the expected positive range. Use this for | |
| 327 | + | // required limits like domain stats, and clampOptionalLimit when zero has its | |
| 328 | + | // own meaning. | |
| 329 | + | func clampPositiveLimit(limit, defaultLimit, maxLimit int) int { | |
| 330 | + | if limit <= 0 { | |
| 331 | + | return defaultLimit | |
| 332 | + | } | |
| 333 | + | if limit > maxLimit { | |
| 334 | + | return maxLimit | |
| 335 | + | } | |
| 336 | + | return limit | |
| 337 | + | } | |
| 338 | + | ||
| 308 | 339 | // GetStats retrieves lightweight aggregate history statistics. | |
| 309 | 340 | func (uc *SearchHistoryUseCase) GetStats(ctx context.Context) (*entity.HistoryStats, error) { | |
| 310 | 341 | stats, err := uc.historyRepo.GetStats(ctx) | |
| @@ -18,7 +18,16 @@ import ( | |||||
| 18 | // Compile-time check: SearchHistoryUseCase must satisfy port.HomepageHistory. | 18 | // Compile-time check: SearchHistoryUseCase must satisfy port.HomepageHistory. | ||
| 19 | var _ port.HomepageHistory = (*SearchHistoryUseCase)(nil) | 19 | var _ port.HomepageHistory = (*SearchHistoryUseCase)(nil) | ||
| 20 | 20 | ||||
| 21 | - | const historyWindowDuration = 24 * time.Hour | 21 | + | const ( |
| 22 | + | historyWindowDuration = 24 * time.Hour | |||
| 23 | + | ||||
| 24 | + | defaultHistoryPageLimit = 50 | |||
| 25 | + | maxHistoryPageLimit = 500 | |||
| 26 | + | defaultHistorySearchLimit = 20 | |||
| 27 | + | maxHistorySearchLimit = 100 | |||
| 28 | + | defaultDomainStatsLimit = 20 | |||
| 29 | + | maxDomainStatsLimit = 100 | |||
| 30 | + | ) | |||
| 22 | 31 | ||||
| 23 | // SearchHistoryUseCase handles history search and retrieval operations. | 32 | // SearchHistoryUseCase handles history search and retrieval operations. | ||
| 24 | type SearchHistoryUseCase struct { | 33 | type SearchHistoryUseCase struct { | ||
| @@ -47,10 +56,7 @@ func (uc *SearchHistoryUseCase) Search(ctx context.Context, input SearchInput) ( | |||||
| 47 | return &SearchOutput{Matches: []entity.HistoryMatch{}}, nil | 56 | return &SearchOutput{Matches: []entity.HistoryMatch{}}, nil | ||
| 48 | } | 57 | } | ||
| 49 | 58 | ||||
| 50 | - | limit := input.Limit | 59 | + | limit := clampPositiveLimit(input.Limit, defaultHistorySearchLimit, maxHistorySearchLimit) |
| 51 | - | if limit <= 0 { | |||
| 52 | - | limit = 20 // Default limit | |||
| 53 | - | } | |||
| 54 | 60 | ||||
| 55 | // Use repository's FTS5 search | 61 | // Use repository's FTS5 search | ||
| 56 | matches, err := uc.historyRepo.Search(ctx, input.Query, limit) | 62 | matches, err := uc.historyRepo.Search(ctx, input.Query, limit) | ||
| @@ -67,11 +73,10 @@ func (uc *SearchHistoryUseCase) Search(ctx context.Context, input SearchInput) ( | |||||
| 67 | } | 73 | } | ||
| 68 | 74 | ||||
| 69 | // GetRecent retrieves recent history entries. A zero limit means all entries; | 75 | // GetRecent retrieves recent history entries. A zero limit means all entries; | ||
| 70 | - | // negative limits retain the historical default page size. | 76 | + | // negative limits retain the historical default page size. Positive limits are |
| 77 | + | // capped to keep WebUI-originated queries bounded. | |||
| 71 | func (uc *SearchHistoryUseCase) GetRecent(ctx context.Context, limit, offset int) ([]*entity.HistoryEntry, error) { | 78 | func (uc *SearchHistoryUseCase) GetRecent(ctx context.Context, limit, offset int) ([]*entity.HistoryEntry, error) { | ||
| 72 | - | if limit < 0 { | 79 | + | limit = clampOptionalLimit(limit, defaultHistoryPageLimit, maxHistoryPageLimit) |
| 73 | - | limit = 50 // Default limit for invalid negative values. | |||
| 74 | - | } | |||
| 75 | 80 | ||||
| 76 | entries, err := uc.historyRepo.GetRecent(ctx, limit, offset) | 81 | entries, err := uc.historyRepo.GetRecent(ctx, limit, offset) | ||
| 77 | if err != nil { | 82 | if err != nil { | ||
| @@ -82,11 +87,10 @@ func (uc *SearchHistoryUseCase) GetRecent(ctx context.Context, limit, offset int | |||||
| 82 | } | 87 | } | ||
| 83 | 88 | ||||
| 84 | // GetRecentByDomain retrieves recent history entries for a canonical domain. A | 89 | // GetRecentByDomain retrieves recent history entries for a canonical domain. A | ||
| 85 | - | // zero limit means all matching entries; negative limits use the default page size. | 90 | + | // zero limit means all matching entries; negative limits use the default page |
| 91 | + | // size. Positive limits are capped to keep WebUI-originated queries bounded. | |||
| 86 | func (uc *SearchHistoryUseCase) GetRecentByDomain(ctx context.Context, domain string, limit, offset int) ([]*entity.HistoryEntry, error) { | 92 | func (uc *SearchHistoryUseCase) GetRecentByDomain(ctx context.Context, domain string, limit, offset int) ([]*entity.HistoryEntry, error) { | ||
| 87 | - | if limit < 0 { | 93 | + | limit = clampOptionalLimit(limit, defaultHistoryPageLimit, maxHistoryPageLimit) |
| 88 | - | limit = 50 | |||
| 89 | - | } | |||
| 90 | domain, err := canonicalHistoryDomain(domain) | 94 | domain, err := canonicalHistoryDomain(domain) | ||
| 91 | if err != nil { | 95 | if err != nil { | ||
| 92 | return nil, err | 96 | return nil, err | ||
| @@ -298,13 +302,40 @@ func (uc *SearchHistoryUseCase) GetDomainStats(ctx context.Context, limit int) ( | |||||
| 298 | log := logging.FromContext(ctx) | 302 | log := logging.FromContext(ctx) | ||
| 299 | log.Debug().Int("limit", limit).Msg("getting domain stats") | 303 | log.Debug().Int("limit", limit).Msg("getting domain stats") | ||
| 300 | 304 | ||||
| 301 | - | if limit <= 0 { | 305 | + | limit = clampPositiveLimit(limit, defaultDomainStatsLimit, maxDomainStatsLimit) |
| 302 | - | limit = 20 | |||
| 303 | - | } | |||
| 304 | 306 | ||||
| 305 | return uc.historyRepo.GetDomainStats(ctx, limit) | 307 | return uc.historyRepo.GetDomainStats(ctx, limit) | ||
| 306 | } | 308 | } | ||
| 307 | 309 | ||||
| 310 | + | // clampOptionalLimit normalizes an optional limit where zero remains a valid | |||
| 311 | + | // "no limit" value, negatives fall back to defaultLimit, and values above | |||
| 312 | + | // maxLimit are capped. Use clampPositiveLimit instead when zero should be | |||
| 313 | + | // treated as invalid input. | |||
| 314 | + | func clampOptionalLimit(limit, defaultLimit, maxLimit int) int { | |||
| 315 | + | if limit < 0 { | |||
| 316 | + | return defaultLimit | |||
| 317 | + | } | |||
| 318 | + | if limit > maxLimit { | |||
| 319 | + | return maxLimit | |||
| 320 | + | } | |||
| 321 | + | return limit | |||
| 322 | + | } | |||
| 323 | + | ||||
| 324 | + | // clampPositiveLimit normalizes a required positive limit. Values less than or | |||
| 325 | + | // equal to zero fall back to defaultLimit, values above maxLimit are capped, | |||
| 326 | + | // and the result stays within the expected positive range. Use this for | |||
| 327 | + | // required limits like domain stats, and clampOptionalLimit when zero has its | |||
| 328 | + | // own meaning. | |||
| 329 | + | func clampPositiveLimit(limit, defaultLimit, maxLimit int) int { | |||
| 330 | + | if limit <= 0 { | |||
| 331 | + | return defaultLimit | |||
| 332 | + | } | |||
| 333 | + | if limit > maxLimit { | |||
| 334 | + | return maxLimit | |||
| 335 | + | } | |||
| 336 | + | return limit | |||
| 337 | + | } | |||
| 338 | + | ||||
| 308 | // GetStats retrieves lightweight aggregate history statistics. | 339 | // GetStats retrieves lightweight aggregate history statistics. | ||
| 309 | func (uc *SearchHistoryUseCase) GetStats(ctx context.Context) (*entity.HistoryStats, error) { | 340 | func (uc *SearchHistoryUseCase) GetStats(ctx context.Context) (*entity.HistoryStats, error) { | ||
| 310 | stats, err := uc.historyRepo.GetStats(ctx) | 341 | stats, err := uc.historyRepo.GetStats(ctx) | ||
M internal/application/usecase/search_history_test.go +49 -0
| @@ -38,6 +38,31 @@ func TestSearchHistoryUseCase_GetRecent_NegativeLimitDefaultsToPageSize(t *testi | |||
| 38 | 38 | assert.Empty(t, result) | |
| 39 | 39 | } | |
| 40 | 40 | ||
| 41 | + | func TestSearchHistoryUseCase_GetRecent_ClampsOversizedLimit(t *testing.T) { | |
| 42 | + | ctx := testContext() | |
| 43 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 44 | + | historyRepo.EXPECT().GetRecent(mock.Anything, 500, 10).Return([]*entity.HistoryEntry{}, nil).Once() | |
| 45 | + | ||
| 46 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo) | |
| 47 | + | result, err := uc.GetRecent(ctx, 10_000, 10) | |
| 48 | + | ||
| 49 | + | require.NoError(t, err) | |
| 50 | + | assert.Empty(t, result) | |
| 51 | + | } | |
| 52 | + | ||
| 53 | + | func TestSearchHistoryUseCase_Search_ClampsOversizedLimit(t *testing.T) { | |
| 54 | + | ctx := testContext() | |
| 55 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 56 | + | historyRepo.EXPECT().Search(mock.Anything, "dumber", 100).Return([]entity.HistoryMatch{}, nil).Once() | |
| 57 | + | ||
| 58 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo) | |
| 59 | + | result, err := uc.Search(ctx, usecase.SearchInput{Query: "dumber", Limit: 10_000}) | |
| 60 | + | ||
| 61 | + | require.NoError(t, err) | |
| 62 | + | require.NotNil(t, result) | |
| 63 | + | assert.Empty(t, result.Matches) | |
| 64 | + | } | |
| 65 | + | ||
| 41 | 66 | func TestSearchHistoryUseCase_GetRecentWindow_InvalidDomainReturnsValidationError(t *testing.T) { | |
| 42 | 67 | ctx := testContext() | |
| 43 | 68 | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| @@ -189,6 +214,18 @@ func TestSearchHistoryUseCase_GetMostVisited_ReturnsEmptyWhenNoHistory(t *testin | |||
| 189 | 214 | assert.Empty(t, result) | |
| 190 | 215 | } | |
| 191 | 216 | ||
| 217 | + | func TestSearchHistoryUseCase_GetDomainStats_ClampsOversizedLimit(t *testing.T) { | |
| 218 | + | ctx := testContext() | |
| 219 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 220 | + | historyRepo.EXPECT().GetDomainStats(mock.Anything, 100).Return([]*entity.DomainStat{}, nil).Once() | |
| 221 | + | ||
| 222 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo) | |
| 223 | + | result, err := uc.GetDomainStats(ctx, 10_000) | |
| 224 | + | ||
| 225 | + | require.NoError(t, err) | |
| 226 | + | assert.Empty(t, result) | |
| 227 | + | } | |
| 228 | + | ||
| 192 | 229 | func TestSearchHistoryUseCase_ClearRange_RecentRangeDeletesSinceCutoff(t *testing.T) { | |
| 193 | 230 | ctx := testContext() | |
| 194 | 231 | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| @@ -257,6 +294,18 @@ func TestSearchHistoryUseCase_GetRecentByDomainAllowsUnderscoreDomains(t *testin | |||
| 257 | 294 | assert.Equal(t, want, result) | |
| 258 | 295 | } | |
| 259 | 296 | ||
| 297 | + | func TestSearchHistoryUseCase_GetRecentByDomain_ClampsOversizedLimit(t *testing.T) { | |
| 298 | + | ctx := testContext() | |
| 299 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 300 | + | historyRepo.EXPECT().GetRecentByDomain(mock.Anything, "example.com", 500, 25).Return([]*entity.HistoryEntry{}, nil).Once() | |
| 301 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo) | |
| 302 | + | ||
| 303 | + | result, err := uc.GetRecentByDomain(ctx, "example.com", 10_000, 25) | |
| 304 | + | ||
| 305 | + | require.NoError(t, err) | |
| 306 | + | assert.Empty(t, result) | |
| 307 | + | } | |
| 308 | + | ||
| 260 | 309 | func TestSearchHistoryUseCase_DeleteByDomainAllowsUnderscoreDomains(t *testing.T) { | |
| 261 | 310 | ctx := testContext() | |
| 262 | 311 | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| @@ -38,6 +38,31 @@ func TestSearchHistoryUseCase_GetRecent_NegativeLimitDefaultsToPageSize(t *testi | |||||
| 38 | assert.Empty(t, result) | 38 | assert.Empty(t, result) | ||
| 39 | } | 39 | } | ||
| 40 | 40 | ||||
| 41 | + | func TestSearchHistoryUseCase_GetRecent_ClampsOversizedLimit(t *testing.T) { | |||
| 42 | + | ctx := testContext() | |||
| 43 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |||
| 44 | + | historyRepo.EXPECT().GetRecent(mock.Anything, 500, 10).Return([]*entity.HistoryEntry{}, nil).Once() | |||
| 45 | + | ||||
| 46 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo) | |||
| 47 | + | result, err := uc.GetRecent(ctx, 10_000, 10) | |||
| 48 | + | ||||
| 49 | + | require.NoError(t, err) | |||
| 50 | + | assert.Empty(t, result) | |||
| 51 | + | } | |||
| 52 | + | ||||
| 53 | + | func TestSearchHistoryUseCase_Search_ClampsOversizedLimit(t *testing.T) { | |||
| 54 | + | ctx := testContext() | |||
| 55 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |||
| 56 | + | historyRepo.EXPECT().Search(mock.Anything, "dumber", 100).Return([]entity.HistoryMatch{}, nil).Once() | |||
| 57 | + | ||||
| 58 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo) | |||
| 59 | + | result, err := uc.Search(ctx, usecase.SearchInput{Query: "dumber", Limit: 10_000}) | |||
| 60 | + | ||||
| 61 | + | require.NoError(t, err) | |||
| 62 | + | require.NotNil(t, result) | |||
| 63 | + | assert.Empty(t, result.Matches) | |||
| 64 | + | } | |||
| 65 | + | ||||
| 41 | func TestSearchHistoryUseCase_GetRecentWindow_InvalidDomainReturnsValidationError(t *testing.T) { | 66 | func TestSearchHistoryUseCase_GetRecentWindow_InvalidDomainReturnsValidationError(t *testing.T) { | ||
| 42 | ctx := testContext() | 67 | ctx := testContext() | ||
| 43 | historyRepo := repomocks.NewMockHistoryRepository(t) | 68 | historyRepo := repomocks.NewMockHistoryRepository(t) | ||
| @@ -189,6 +214,18 @@ func TestSearchHistoryUseCase_GetMostVisited_ReturnsEmptyWhenNoHistory(t *testin | |||||
| 189 | assert.Empty(t, result) | 214 | assert.Empty(t, result) | ||
| 190 | } | 215 | } | ||
| 191 | 216 | ||||
| 217 | + | func TestSearchHistoryUseCase_GetDomainStats_ClampsOversizedLimit(t *testing.T) { | |||
| 218 | + | ctx := testContext() | |||
| 219 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |||
| 220 | + | historyRepo.EXPECT().GetDomainStats(mock.Anything, 100).Return([]*entity.DomainStat{}, nil).Once() | |||
| 221 | + | ||||
| 222 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo) | |||
| 223 | + | result, err := uc.GetDomainStats(ctx, 10_000) | |||
| 224 | + | ||||
| 225 | + | require.NoError(t, err) | |||
| 226 | + | assert.Empty(t, result) | |||
| 227 | + | } | |||
| 228 | + | ||||
| 192 | func TestSearchHistoryUseCase_ClearRange_RecentRangeDeletesSinceCutoff(t *testing.T) { | 229 | func TestSearchHistoryUseCase_ClearRange_RecentRangeDeletesSinceCutoff(t *testing.T) { | ||
| 193 | ctx := testContext() | 230 | ctx := testContext() | ||
| 194 | historyRepo := repomocks.NewMockHistoryRepository(t) | 231 | historyRepo := repomocks.NewMockHistoryRepository(t) | ||
| @@ -257,6 +294,18 @@ func TestSearchHistoryUseCase_GetRecentByDomainAllowsUnderscoreDomains(t *testin | |||||
| 257 | assert.Equal(t, want, result) | 294 | assert.Equal(t, want, result) | ||
| 258 | } | 295 | } | ||
| 259 | 296 | ||||
| 297 | + | func TestSearchHistoryUseCase_GetRecentByDomain_ClampsOversizedLimit(t *testing.T) { | |||
| 298 | + | ctx := testContext() | |||
| 299 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |||
| 300 | + | historyRepo.EXPECT().GetRecentByDomain(mock.Anything, "example.com", 500, 25).Return([]*entity.HistoryEntry{}, nil).Once() | |||
| 301 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo) | |||
| 302 | + | ||||
| 303 | + | result, err := uc.GetRecentByDomain(ctx, "example.com", 10_000, 25) | |||
| 304 | + | ||||
| 305 | + | require.NoError(t, err) | |||
| 306 | + | assert.Empty(t, result) | |||
| 307 | + | } | |||
| 308 | + | ||||
| 260 | func TestSearchHistoryUseCase_DeleteByDomainAllowsUnderscoreDomains(t *testing.T) { | 309 | func TestSearchHistoryUseCase_DeleteByDomainAllowsUnderscoreDomains(t *testing.T) { | ||
| 261 | ctx := testContext() | 310 | ctx := testContext() | ||
| 262 | historyRepo := repomocks.NewMockHistoryRepository(t) | 311 | historyRepo := repomocks.NewMockHistoryRepository(t) | ||
M internal/infrastructure/externaltheme/noctalia/watcher_test.go +3 -2
| @@ -89,7 +89,8 @@ func TestFileWatcherPathChangeRestartsAndDisabledStops(t *testing.T) { | |||
| 89 | 89 | func TestFileWatcherDebouncesBurstToOneCallback(t *testing.T) { | |
| 90 | 90 | dir := t.TempDir() | |
| 91 | 91 | path := writeThemeFile(t, dir, "theme.json") | |
| 92 | - | watcher := newFastWatcher() | |
| 92 | + | watcher := NewFileWatcher() | |
| 93 | + | watcher.delay = 100 * time.Millisecond | |
| 93 | 94 | changes := make(chan struct{}, 8) | |
| 94 | 95 | ||
| 95 | 96 | if err := watcher.Start(context.Background(), watcherConfig(path), func() { changes <- struct{}{} }); err != nil { | |
| @@ -100,7 +101,7 @@ func TestFileWatcherDebouncesBurstToOneCallback(t *testing.T) { | |||
| 100 | 101 | } | |
| 101 | 102 | ||
| 102 | 103 | waitForChange(t, changes) | |
| 103 | - | assertNoChange(t, changes, 80*time.Millisecond) | |
| 104 | + | assertNoChange(t, changes, 150*time.Millisecond) | |
| 104 | 105 | } | |
| 105 | 106 | ||
| 106 | 107 | func TestFileWatcherAtomicRenameWriteEmitsCallback(t *testing.T) { | |
| @@ -89,7 +89,8 @@ func TestFileWatcherPathChangeRestartsAndDisabledStops(t *testing.T) { | |||||
| 89 | func TestFileWatcherDebouncesBurstToOneCallback(t *testing.T) { | 89 | func TestFileWatcherDebouncesBurstToOneCallback(t *testing.T) { | ||
| 90 | dir := t.TempDir() | 90 | dir := t.TempDir() | ||
| 91 | path := writeThemeFile(t, dir, "theme.json") | 91 | path := writeThemeFile(t, dir, "theme.json") | ||
| 92 | - | watcher := newFastWatcher() | 92 | + | watcher := NewFileWatcher() |
| 93 | + | watcher.delay = 100 * time.Millisecond | |||
| 93 | changes := make(chan struct{}, 8) | 94 | changes := make(chan struct{}, 8) | ||
| 94 | 95 | ||||
| 95 | if err := watcher.Start(context.Background(), watcherConfig(path), func() { changes <- struct{}{} }); err != nil { | 96 | if err := watcher.Start(context.Background(), watcherConfig(path), func() { changes <- struct{}{} }); err != nil { | ||
| @@ -100,7 +101,7 @@ func TestFileWatcherDebouncesBurstToOneCallback(t *testing.T) { | |||||
| 100 | } | 101 | } | ||
| 101 | 102 | ||||
| 102 | waitForChange(t, changes) | 103 | waitForChange(t, changes) | ||
| 103 | - | assertNoChange(t, changes, 80*time.Millisecond) | 104 | + | assertNoChange(t, changes, 150*time.Millisecond) |
| 104 | } | 105 | } | ||
| 105 | 106 | ||||
| 106 | func TestFileWatcherAtomicRenameWriteEmitsCallback(t *testing.T) { | 107 | func TestFileWatcherAtomicRenameWriteEmitsCallback(t *testing.T) { | ||
M internal/infrastructure/favicon/cache.go +17 -1
| @@ -30,6 +30,7 @@ type Cache struct { | |||
| 30 | 30 | diskDir string | |
| 31 | 31 | writeChan chan diskWrite | |
| 32 | 32 | closeOnce sync.Once | |
| 33 | + | closed bool | |
| 33 | 34 | mu sync.RWMutex | |
| 34 | 35 | } | |
| 35 | 36 | ||
| @@ -100,8 +101,12 @@ func (c *Cache) Set(ctx context.Context, domain string, data []byte) { | |||
| 100 | 101 | Int("bytes", len(data)). | |
| 101 | 102 | Msg("favicon: Cache.Set") | |
| 102 | 103 | ||
| 103 | - | // Write to memory cache | |
| 104 | + | // Write to memory cache unless the cache is already shutting down. | |
| 104 | 105 | c.mu.Lock() | |
| 106 | + | if c.closed { | |
| 107 | + | c.mu.Unlock() | |
| 108 | + | return | |
| 109 | + | } | |
| 105 | 110 | c.memCache[domain] = data | |
| 106 | 111 | c.mu.Unlock() | |
| 107 | 112 | ||
| @@ -228,6 +233,10 @@ func (c *Cache) HasOnDisk(domain string) bool { | |||
| 228 | 233 | // Close shuts down the background writer goroutine. | |
| 229 | 234 | func (c *Cache) Close() { | |
| 230 | 235 | c.closeOnce.Do(func() { | |
| 236 | + | c.mu.Lock() | |
| 237 | + | defer c.mu.Unlock() | |
| 238 | + | ||
| 239 | + | c.closed = true | |
| 231 | 240 | if c.writeChan != nil { | |
| 232 | 241 | close(c.writeChan) | |
| 233 | 242 | } | |
| @@ -308,6 +317,13 @@ func (c *Cache) queueDiskWrite(domain string, data []byte) { | |||
| 308 | 317 | if c.diskDir == "" { | |
| 309 | 318 | return | |
| 310 | 319 | } | |
| 320 | + | ||
| 321 | + | c.mu.RLock() | |
| 322 | + | defer c.mu.RUnlock() | |
| 323 | + | if c.closed { | |
| 324 | + | return | |
| 325 | + | } | |
| 326 | + | ||
| 311 | 327 | select { | |
| 312 | 328 | case c.writeChan <- diskWrite{domain: domain, data: data}: | |
| 313 | 329 | // queued successfully | |
| @@ -30,6 +30,7 @@ type Cache struct { | |||||
| 30 | diskDir string | 30 | diskDir string | ||
| 31 | writeChan chan diskWrite | 31 | writeChan chan diskWrite | ||
| 32 | closeOnce sync.Once | 32 | closeOnce sync.Once | ||
| 33 | + | closed bool | |||
| 33 | mu sync.RWMutex | 34 | mu sync.RWMutex | ||
| 34 | } | 35 | } | ||
| 35 | 36 | ||||
| @@ -100,8 +101,12 @@ func (c *Cache) Set(ctx context.Context, domain string, data []byte) { | |||||
| 100 | Int("bytes", len(data)). | 101 | Int("bytes", len(data)). | ||
| 101 | Msg("favicon: Cache.Set") | 102 | Msg("favicon: Cache.Set") | ||
| 102 | 103 | ||||
| 103 | - | // Write to memory cache | 104 | + | // Write to memory cache unless the cache is already shutting down. |
| 104 | c.mu.Lock() | 105 | c.mu.Lock() | ||
| 106 | + | if c.closed { | |||
| 107 | + | c.mu.Unlock() | |||
| 108 | + | return | |||
| 109 | + | } | |||
| 105 | c.memCache[domain] = data | 110 | c.memCache[domain] = data | ||
| 106 | c.mu.Unlock() | 111 | c.mu.Unlock() | ||
| 107 | 112 | ||||
| @@ -228,6 +233,10 @@ func (c *Cache) HasOnDisk(domain string) bool { | |||||
| 228 | // Close shuts down the background writer goroutine. | 233 | // Close shuts down the background writer goroutine. | ||
| 229 | func (c *Cache) Close() { | 234 | func (c *Cache) Close() { | ||
| 230 | c.closeOnce.Do(func() { | 235 | c.closeOnce.Do(func() { | ||
| 236 | + | c.mu.Lock() | |||
| 237 | + | defer c.mu.Unlock() | |||
| 238 | + | ||||
| 239 | + | c.closed = true | |||
| 231 | if c.writeChan != nil { | 240 | if c.writeChan != nil { | ||
| 232 | close(c.writeChan) | 241 | close(c.writeChan) | ||
| 233 | } | 242 | } | ||
| @@ -308,6 +317,13 @@ func (c *Cache) queueDiskWrite(domain string, data []byte) { | |||||
| 308 | if c.diskDir == "" { | 317 | if c.diskDir == "" { | ||
| 309 | return | 318 | return | ||
| 310 | } | 319 | } | ||
| 320 | + | ||||
| 321 | + | c.mu.RLock() | |||
| 322 | + | defer c.mu.RUnlock() | |||
| 323 | + | if c.closed { | |||
| 324 | + | return | |||
| 325 | + | } | |||
| 326 | + | ||||
| 311 | select { | 327 | select { | ||
| 312 | case c.writeChan <- diskWrite{domain: domain, data: data}: | 328 | case c.writeChan <- diskWrite{domain: domain, data: data}: | ||
| 313 | // queued successfully | 329 | // queued successfully | ||
A internal/infrastructure/favicon/cache_test.go +21 -0
| @@ -0,0 +1,21 @@ | |||
| 1 | + | package favicon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/stretchr/testify/require" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | func TestCache_SetAfterCloseIsNoOp(t *testing.T) { | |
| 11 | + | ctx := context.Background() | |
| 12 | + | cache := NewCache(t.TempDir()) | |
| 13 | + | cache.Close() | |
| 14 | + | ||
| 15 | + | require.NotPanics(t, func() { | |
| 16 | + | cache.Set(ctx, "example.com", []byte("favicon")) | |
| 17 | + | }) | |
| 18 | + | ||
| 19 | + | _, ok := cache.Get(ctx, "example.com") | |
| 20 | + | require.False(t, ok) | |
| 21 | + | } | |
| @@ -0,0 +1,21 @@ | |||||
| 1 | + | package favicon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/stretchr/testify/require" | |||
| 8 | + | ) | |||
| 9 | + | ||||
| 10 | + | func TestCache_SetAfterCloseIsNoOp(t *testing.T) { | |||
| 11 | + | ctx := context.Background() | |||
| 12 | + | cache := NewCache(t.TempDir()) | |||
| 13 | + | cache.Close() | |||
| 14 | + | ||||
| 15 | + | require.NotPanics(t, func() { | |||
| 16 | + | cache.Set(ctx, "example.com", []byte("favicon")) | |||
| 17 | + | }) | |||
| 18 | + | ||||
| 19 | + | _, ok := cache.Get(ctx, "example.com") | |||
| 20 | + | require.False(t, ok) | |||
| 21 | + | } | |||
M internal/infrastructure/filtering/downloader.go +230 -18
| @@ -7,7 +7,9 @@ import ( | |||
| 7 | 7 | "io" | |
| 8 | 8 | "net/http" | |
| 9 | 9 | "os" | |
| 10 | + | pathpkg "path" | |
| 10 | 11 | "path/filepath" | |
| 12 | + | "strings" | |
| 11 | 13 | "time" | |
| 12 | 14 | ||
| 13 | 15 | "github.com/bnema/dumber/internal/logging" | |
| @@ -16,13 +18,34 @@ import ( | |||
| 16 | 18 | const ( | |
| 17 | 19 | cacheDirPerm = 0o755 | |
| 18 | 20 | manifestFilePerm = 0o644 | |
| 21 | + | ||
| 22 | + | defaultMaxManifestBytes int64 = 1 << 20 // 1 MiB | |
| 23 | + | defaultMaxFilterFiles = 128 | |
| 24 | + | defaultMaxFilterFileBytes int64 = 64 << 20 // 64 MiB | |
| 25 | + | defaultMaxTotalFilterBytes int64 = 512 << 20 // 512 MiB | |
| 19 | 26 | ) | |
| 20 | 27 | ||
| 28 | + | // downloadLimits bounds remote filter metadata and payloads. | |
| 29 | + | type downloadLimits struct { | |
| 30 | + | maxManifestBytes int64 | |
| 31 | + | maxFilterFiles int | |
| 32 | + | maxFilterFileBytes int64 | |
| 33 | + | maxTotalFilterBytes int64 | |
| 34 | + | } | |
| 35 | + | ||
| 36 | + | var defaultDownloadLimits = downloadLimits{ | |
| 37 | + | maxManifestBytes: defaultMaxManifestBytes, | |
| 38 | + | maxFilterFiles: defaultMaxFilterFiles, | |
| 39 | + | maxFilterFileBytes: defaultMaxFilterFileBytes, | |
| 40 | + | maxTotalFilterBytes: defaultMaxTotalFilterBytes, | |
| 41 | + | } | |
| 42 | + | ||
| 21 | 43 | // Downloader handles downloading filter files from GitHub releases. | |
| 22 | 44 | type Downloader struct { | |
| 23 | 45 | baseURL string | |
| 24 | 46 | cacheDir string | |
| 25 | 47 | httpClient *http.Client | |
| 48 | + | limits downloadLimits | |
| 26 | 49 | } | |
| 27 | 50 | ||
| 28 | 51 | // NewDownloader creates a new Downloader. | |
| @@ -34,14 +57,15 @@ func NewDownloader(cacheDir string) *Downloader { | |||
| 34 | 57 | httpClient: &http.Client{ | |
| 35 | 58 | Timeout: 60 * time.Second, | |
| 36 | 59 | }, | |
| 60 | + | limits: defaultDownloadLimits, | |
| 37 | 61 | } | |
| 38 | 62 | } | |
| 39 | 63 | ||
| 40 | 64 | // GetCachedManifest reads the locally cached manifest.json. | |
| 41 | 65 | // Returns nil if no cached manifest exists. | |
| 42 | 66 | func (d *Downloader) GetCachedManifest() (*Manifest, error) { | |
| 43 | - | path := filepath.Join(d.cacheDir, FilterFiles.Manifest) | |
| 44 | - | data, err := os.ReadFile(path) | |
| 67 | + | manifestPath := filepath.Join(d.cacheDir, FilterFiles.Manifest) | |
| 68 | + | data, err := readLimitedFile(manifestPath, d.limits.maxManifestBytes, "cached manifest") | |
| 45 | 69 | if err != nil { | |
| 46 | 70 | if os.IsNotExist(err) { | |
| 47 | 71 | return nil, nil | |
| @@ -53,6 +77,9 @@ func (d *Downloader) GetCachedManifest() (*Manifest, error) { | |||
| 53 | 77 | if err := json.Unmarshal(data, &manifest); err != nil { | |
| 54 | 78 | return nil, fmt.Errorf("failed to parse cached manifest: %w", err) | |
| 55 | 79 | } | |
| 80 | + | if err := d.validateManifest(&manifest); err != nil { | |
| 81 | + | return nil, fmt.Errorf("invalid cached manifest: %w", err) | |
| 82 | + | } | |
| 56 | 83 | return &manifest, nil | |
| 57 | 84 | } | |
| 58 | 85 | ||
| @@ -84,7 +111,7 @@ func (d *Downloader) FetchManifest(ctx context.Context) (*Manifest, error) { | |||
| 84 | 111 | return nil, fmt.Errorf("manifest fetch failed with status %d", resp.StatusCode) | |
| 85 | 112 | } | |
| 86 | 113 | ||
| 87 | - | data, err := io.ReadAll(resp.Body) | |
| 114 | + | data, err := readLimitedResponseBody(resp, d.limits.maxManifestBytes, "manifest") | |
| 88 | 115 | if err != nil { | |
| 89 | 116 | return nil, fmt.Errorf("failed to read manifest body: %w", err) | |
| 90 | 117 | } | |
| @@ -93,6 +120,9 @@ func (d *Downloader) FetchManifest(ctx context.Context) (*Manifest, error) { | |||
| 93 | 120 | if err := json.Unmarshal(data, &manifest); err != nil { | |
| 94 | 121 | return nil, fmt.Errorf("failed to parse manifest: %w", err) | |
| 95 | 122 | } | |
| 123 | + | if err := d.validateManifest(&manifest); err != nil { | |
| 124 | + | return nil, fmt.Errorf("invalid manifest: %w", err) | |
| 125 | + | } | |
| 96 | 126 | ||
| 97 | 127 | log.Debug().Str("version", manifest.Version).Msg("fetched manifest") | |
| 98 | 128 | return &manifest, nil | |
| @@ -153,7 +183,12 @@ func (d *Downloader) DownloadFilters(ctx context.Context, onProgress func(Downlo | |||
| 153 | 183 | }) | |
| 154 | 184 | } | |
| 155 | 185 | ||
| 156 | - | path, bytesDownloaded, err := d.downloadFile(ctx, filename) | |
| 186 | + | remainingBytes := d.limits.maxTotalFilterBytes - totalBytes | |
| 187 | + | if remainingBytes <= 0 { | |
| 188 | + | return nil, fmt.Errorf("filter downloads exceed total size limit of %d bytes", d.limits.maxTotalFilterBytes) | |
| 189 | + | } | |
| 190 | + | ||
| 191 | + | path, bytesDownloaded, err := d.downloadFile(ctx, filename, minInt64(d.limits.maxFilterFileBytes, remainingBytes)) | |
| 157 | 192 | if err != nil { | |
| 158 | 193 | log.Error().Err(err).Str("file", filename).Msg("failed to download filter file") | |
| 159 | 194 | return nil, err | |
| @@ -174,9 +209,13 @@ func (d *Downloader) DownloadFilters(ctx context.Context, onProgress func(Downlo | |||
| 174 | 209 | } | |
| 175 | 210 | ||
| 176 | 211 | // downloadFile downloads a single file and returns its local path. | |
| 177 | - | func (d *Downloader) downloadFile(ctx context.Context, filename string) (string, int64, error) { | |
| 178 | - | url := fmt.Sprintf("%s/%s", d.baseURL, filename) | |
| 179 | - | localPath := filepath.Join(d.cacheDir, filename) | |
| 212 | + | func (d *Downloader) downloadFile(ctx context.Context, filename string, maxBytes int64) (string, int64, error) { | |
| 213 | + | localPath, safeFilename, err := d.cachePathForFilterFile(filename) | |
| 214 | + | if err != nil { | |
| 215 | + | return "", 0, err | |
| 216 | + | } | |
| 217 | + | ||
| 218 | + | url := fmt.Sprintf("%s/%s", d.baseURL, safeFilename) | |
| 180 | 219 | ||
| 181 | 220 | req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) | |
| 182 | 221 | if err != nil { | |
| @@ -197,25 +236,29 @@ func (d *Downloader) downloadFile(ctx context.Context, filename string) (string, | |||
| 197 | 236 | return "", 0, fmt.Errorf("download %s failed with status %d", filename, resp.StatusCode) | |
| 198 | 237 | } | |
| 199 | 238 | ||
| 239 | + | if resp.ContentLength > maxBytes { | |
| 240 | + | return "", 0, fmt.Errorf("filter file %q too large: content length %d exceeds limit %d", safeFilename, resp.ContentLength, maxBytes) | |
| 241 | + | } | |
| 242 | + | ||
| 200 | 243 | // Write to temp file in cacheDir, then atomic rename into destination. | |
| 201 | - | tmpFile, err := os.CreateTemp(d.cacheDir, filepath.Base(filename)+".*.tmp") | |
| 244 | + | tmpFile, err := os.CreateTemp(d.cacheDir, filepath.Base(safeFilename)+".*.tmp") | |
| 202 | 245 | if err != nil { | |
| 203 | 246 | return "", 0, fmt.Errorf("failed to create temp file: %w", err) | |
| 204 | 247 | } | |
| 205 | 248 | tmpPath := tmpFile.Name() | |
| 206 | 249 | ||
| 207 | - | bytesWritten, err := io.Copy(tmpFile, resp.Body) | |
| 250 | + | bytesWritten, err := copyLimited(tmpFile, resp.Body, maxBytes, fmt.Sprintf("filter file %q", safeFilename)) | |
| 208 | 251 | if err != nil { | |
| 209 | 252 | _ = tmpFile.Close() | |
| 210 | 253 | _ = os.Remove(tmpPath) | |
| 211 | - | return "", 0, fmt.Errorf("failed to write %s: %w", filename, err) | |
| 254 | + | return "", 0, fmt.Errorf("failed to write %s: %w", safeFilename, err) | |
| 212 | 255 | } | |
| 213 | 256 | if err := tmpFile.Close(); err != nil { | |
| 214 | 257 | _ = os.Remove(tmpPath) | |
| 215 | 258 | return "", 0, fmt.Errorf("failed to close temp file: %w", err) | |
| 216 | 259 | } | |
| 217 | 260 | ||
| 218 | - | if err := d.renameTempFile(tmpPath, localPath); err != nil { | |
| 261 | + | if err := d.renameTempFile(tmpPath, safeFilename); err != nil { | |
| 219 | 262 | _ = os.Remove(tmpPath) | |
| 220 | 263 | return "", 0, err | |
| 221 | 264 | } | |
| @@ -223,14 +266,88 @@ func (d *Downloader) downloadFile(ctx context.Context, filename string) (string, | |||
| 223 | 266 | return localPath, bytesWritten, nil | |
| 224 | 267 | } | |
| 225 | 268 | ||
| 226 | - | func (*Downloader) renameTempFile(tmpPath, localPath string) error { | |
| 227 | - | parentDir := filepath.Dir(localPath) | |
| 269 | + | func (d *Downloader) cachePathForFilterFile(filename string) (string, string, error) { | |
| 270 | + | safeFilename, err := validateFilterFilename(filename) | |
| 271 | + | if err != nil { | |
| 272 | + | return "", "", err | |
| 273 | + | } | |
| 274 | + | ||
| 275 | + | cacheDirAbs, err := filepath.Abs(d.cacheDir) | |
| 276 | + | if err != nil { | |
| 277 | + | return "", "", fmt.Errorf("failed to resolve cache dir: %w", err) | |
| 278 | + | } | |
| 279 | + | localPath := filepath.Join(d.cacheDir, filepath.FromSlash(safeFilename)) | |
| 280 | + | targetAbs, err := filepath.Abs(localPath) | |
| 281 | + | if err != nil { | |
| 282 | + | return "", "", fmt.Errorf("failed to resolve cache path: %w", err) | |
| 283 | + | } | |
| 284 | + | if err := ensurePathInside(cacheDirAbs, targetAbs); err != nil { | |
| 285 | + | return "", "", fmt.Errorf("invalid filter filename %q: %w", filename, err) | |
| 286 | + | } | |
| 287 | + | ||
| 288 | + | return localPath, safeFilename, nil | |
| 289 | + | } | |
| 290 | + | ||
| 291 | + | func validateFilterFilename(filename string) (string, error) { | |
| 292 | + | if filename == "" { | |
| 293 | + | return "", fmt.Errorf("invalid filter filename: empty") | |
| 294 | + | } | |
| 295 | + | if strings.TrimSpace(filename) != filename { | |
| 296 | + | return "", fmt.Errorf("invalid filter filename %q: surrounding whitespace is not allowed", filename) | |
| 297 | + | } | |
| 298 | + | if filepath.IsAbs(filename) || strings.HasPrefix(filename, "/") { | |
| 299 | + | return "", fmt.Errorf("invalid filter filename %q: absolute paths are not allowed", filename) | |
| 300 | + | } | |
| 301 | + | if strings.Contains(filename, "\\") { | |
| 302 | + | return "", fmt.Errorf("invalid filter filename %q: backslashes are not allowed", filename) | |
| 303 | + | } | |
| 304 | + | if strings.ContainsAny(filename, "?#") { | |
| 305 | + | return "", fmt.Errorf("invalid filter filename %q: URL control characters are not allowed", filename) | |
| 306 | + | } | |
| 228 | 307 | ||
| 229 | - | if err := os.MkdirAll(parentDir, cacheDirPerm); err != nil { | |
| 230 | - | return fmt.Errorf("failed to create target dir: %w", err) | |
| 308 | + | parts := strings.Split(filename, "/") | |
| 309 | + | for _, part := range parts { | |
| 310 | + | if part == "" || part == "." || part == ".." { | |
| 311 | + | return "", fmt.Errorf("invalid filter filename %q: path traversal components are not allowed", filename) | |
| 312 | + | } | |
| 313 | + | } | |
| 314 | + | ||
| 315 | + | clean := pathpkg.Clean(filename) | |
| 316 | + | if pathpkg.Ext(clean) != ".json" { | |
| 317 | + | return "", fmt.Errorf("invalid filter filename %q: expected .json extension", filename) | |
| 318 | + | } | |
| 319 | + | ||
| 320 | + | return clean, nil | |
| 321 | + | } | |
| 322 | + | ||
| 323 | + | func ensurePathInside(baseAbs, targetAbs string) error { | |
| 324 | + | rel, err := filepath.Rel(baseAbs, targetAbs) | |
| 325 | + | if err != nil { | |
| 326 | + | return fmt.Errorf("failed to compare with cache dir: %w", err) | |
| 327 | + | } | |
| 328 | + | if rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) { | |
| 329 | + | return fmt.Errorf("resolved path escapes cache dir") | |
| 330 | + | } | |
| 331 | + | return nil | |
| 332 | + | } | |
| 333 | + | ||
| 334 | + | func (d *Downloader) renameTempFile(tmpPath, safeFilename string) error { | |
| 335 | + | relTarget := filepath.FromSlash(safeFilename) | |
| 336 | + | parentDir := filepath.Dir(relTarget) | |
| 337 | + | ||
| 338 | + | root, err := os.OpenRoot(d.cacheDir) | |
| 339 | + | if err != nil { | |
| 340 | + | return fmt.Errorf("failed to open cache root: %w", err) | |
| 341 | + | } | |
| 342 | + | defer func() { _ = root.Close() }() | |
| 343 | + | ||
| 344 | + | if parentDir != "." { | |
| 345 | + | if err := root.MkdirAll(parentDir, cacheDirPerm); err != nil { | |
| 346 | + | return fmt.Errorf("failed to create target dir: %w", err) | |
| 347 | + | } | |
| 231 | 348 | } | |
| 232 | 349 | ||
| 233 | - | if err := os.Rename(tmpPath, localPath); err != nil { | |
| 350 | + | if err := root.Rename(filepath.Base(tmpPath), relTarget); err != nil { | |
| 234 | 351 | return fmt.Errorf("failed to rename temp file: %w", err) | |
| 235 | 352 | } | |
| 236 | 353 | ||
| @@ -239,6 +356,10 @@ func (*Downloader) renameTempFile(tmpPath, localPath string) error { | |||
| 239 | 356 | ||
| 240 | 357 | // cacheManifest writes an already-fetched manifest to disk. | |
| 241 | 358 | func (d *Downloader) cacheManifest(manifest *Manifest) error { | |
| 359 | + | if err := d.validateManifest(manifest); err != nil { | |
| 360 | + | return fmt.Errorf("invalid manifest: %w", err) | |
| 361 | + | } | |
| 362 | + | ||
| 242 | 363 | data, err := json.MarshalIndent(manifest, "", " ") | |
| 243 | 364 | if err != nil { | |
| 244 | 365 | return fmt.Errorf("failed to marshal manifest: %w", err) | |
| @@ -262,8 +383,11 @@ func (d *Downloader) GetCachedFilterPaths() []string { | |||
| 262 | 383 | } | |
| 263 | 384 | paths := make([]string, 0, len(manifest.Combined.Files)) | |
| 264 | 385 | for _, filename := range manifest.Combined.Files { | |
| 265 | - | path := filepath.Join(d.cacheDir, filename) | |
| 266 | - | if _, err := os.Stat(path); err != nil { | |
| 386 | + | path, safeFilename, err := d.cachePathForFilterFile(filename) | |
| 387 | + | if err != nil { | |
| 388 | + | return nil | |
| 389 | + | } | |
| 390 | + | if !d.hasCachedFilterFile(safeFilename) { | |
| 267 | 391 | return nil | |
| 268 | 392 | } | |
| 269 | 393 | paths = append(paths, path) | |
| @@ -314,6 +438,94 @@ func (d *Downloader) ClearCache() error { | |||
| 314 | 438 | return os.RemoveAll(d.cacheDir) | |
| 315 | 439 | } | |
| 316 | 440 | ||
| 441 | + | func (d *Downloader) hasCachedFilterFile(safeFilename string) bool { | |
| 442 | + | root, err := os.OpenRoot(d.cacheDir) | |
| 443 | + | if err != nil { | |
| 444 | + | return false | |
| 445 | + | } | |
| 446 | + | defer func() { _ = root.Close() }() | |
| 447 | + | ||
| 448 | + | info, err := root.Stat(filepath.FromSlash(safeFilename)) | |
| 449 | + | return err == nil && !info.IsDir() | |
| 450 | + | } | |
| 451 | + | ||
| 452 | + | func (d *Downloader) validateManifest(manifest *Manifest) error { | |
| 453 | + | if manifest == nil { | |
| 454 | + | return fmt.Errorf("manifest is nil") | |
| 455 | + | } | |
| 456 | + | if len(manifest.Combined.Files) > d.limits.maxFilterFiles { | |
| 457 | + | return fmt.Errorf("manifest lists %d combined filter files, limit is %d", len(manifest.Combined.Files), d.limits.maxFilterFiles) | |
| 458 | + | } | |
| 459 | + | for _, filename := range manifest.Combined.Files { | |
| 460 | + | if _, _, err := d.cachePathForFilterFile(filename); err != nil { | |
| 461 | + | return err | |
| 462 | + | } | |
| 463 | + | } | |
| 464 | + | return nil | |
| 465 | + | } | |
| 466 | + | ||
| 467 | + | func readLimitedFile(filename string, limit int64, label string) ([]byte, error) { | |
| 468 | + | file, err := os.Open(filename) | |
| 469 | + | if err != nil { | |
| 470 | + | return nil, err | |
| 471 | + | } | |
| 472 | + | defer func() { _ = file.Close() }() | |
| 473 | + | ||
| 474 | + | info, err := file.Stat() | |
| 475 | + | if err != nil { | |
| 476 | + | return nil, err | |
| 477 | + | } | |
| 478 | + | if info.Size() > limit { | |
| 479 | + | return nil, fmt.Errorf("%s too large: size %d exceeds limit %d", label, info.Size(), limit) | |
| 480 | + | } | |
| 481 | + | ||
| 482 | + | return readLimitedReader(file, limit, label) | |
| 483 | + | } | |
| 484 | + | ||
| 485 | + | func readLimitedResponseBody(resp *http.Response, limit int64, label string) ([]byte, error) { | |
| 486 | + | if resp.ContentLength > limit { | |
| 487 | + | return nil, fmt.Errorf("%s too large: content length %d exceeds limit %d", label, resp.ContentLength, limit) | |
| 488 | + | } | |
| 489 | + | return readLimitedReader(resp.Body, limit, label) | |
| 490 | + | } | |
| 491 | + | ||
| 492 | + | func readLimitedReader(r io.Reader, limit int64, label string) ([]byte, error) { | |
| 493 | + | if limit <= 0 { | |
| 494 | + | return nil, fmt.Errorf("%s size limit must be positive", label) | |
| 495 | + | } | |
| 496 | + | ||
| 497 | + | data, err := io.ReadAll(io.LimitReader(r, limit+1)) | |
| 498 | + | if err != nil { | |
| 499 | + | return nil, err | |
| 500 | + | } | |
| 501 | + | if int64(len(data)) > limit { | |
| 502 | + | return nil, fmt.Errorf("%s too large: exceeds limit %d bytes", label, limit) | |
| 503 | + | } | |
| 504 | + | return data, nil | |
| 505 | + | } | |
| 506 | + | ||
| 507 | + | func copyLimited(dst io.Writer, src io.Reader, limit int64, label string) (int64, error) { | |
| 508 | + | if limit <= 0 { | |
| 509 | + | return 0, fmt.Errorf("%s size limit must be positive", label) | |
| 510 | + | } | |
| 511 | + | ||
| 512 | + | written, err := io.Copy(dst, io.LimitReader(src, limit+1)) | |
| 513 | + | if err != nil { | |
| 514 | + | return written, err | |
| 515 | + | } | |
| 516 | + | if written > limit { | |
| 517 | + | return written, fmt.Errorf("%s too large: exceeds limit %d bytes", label, limit) | |
| 518 | + | } | |
| 519 | + | return written, nil | |
| 520 | + | } | |
| 521 | + | ||
| 522 | + | func minInt64(a, b int64) int64 { | |
| 523 | + | if a < b { | |
| 524 | + | return a | |
| 525 | + | } | |
| 526 | + | return b | |
| 527 | + | } | |
| 528 | + | ||
| 317 | 529 | // IsCacheStale checks if the cached manifest is older than maxAge. | |
| 318 | 530 | // Returns true if cache is stale or doesn't exist. | |
| 319 | 531 | func (d *Downloader) IsCacheStale(maxAge time.Duration) bool { | |
| @@ -7,7 +7,9 @@ import ( | |||||
| 7 | "io" | 7 | "io" | ||
| 8 | "net/http" | 8 | "net/http" | ||
| 9 | "os" | 9 | "os" | ||
| 10 | + | pathpkg "path" | |||
| 10 | "path/filepath" | 11 | "path/filepath" | ||
| 12 | + | "strings" | |||
| 11 | "time" | 13 | "time" | ||
| 12 | 14 | ||||
| 13 | "github.com/bnema/dumber/internal/logging" | 15 | "github.com/bnema/dumber/internal/logging" | ||
| @@ -16,13 +18,34 @@ import ( | |||||
| 16 | const ( | 18 | const ( | ||
| 17 | cacheDirPerm = 0o755 | 19 | cacheDirPerm = 0o755 | ||
| 18 | manifestFilePerm = 0o644 | 20 | manifestFilePerm = 0o644 | ||
| 21 | + | ||||
| 22 | + | defaultMaxManifestBytes int64 = 1 << 20 // 1 MiB | |||
| 23 | + | defaultMaxFilterFiles = 128 | |||
| 24 | + | defaultMaxFilterFileBytes int64 = 64 << 20 // 64 MiB | |||
| 25 | + | defaultMaxTotalFilterBytes int64 = 512 << 20 // 512 MiB | |||
| 19 | ) | 26 | ) | ||
| 20 | 27 | ||||
| 28 | + | // downloadLimits bounds remote filter metadata and payloads. | |||
| 29 | + | type downloadLimits struct { | |||
| 30 | + | maxManifestBytes int64 | |||
| 31 | + | maxFilterFiles int | |||
| 32 | + | maxFilterFileBytes int64 | |||
| 33 | + | maxTotalFilterBytes int64 | |||
| 34 | + | } | |||
| 35 | + | ||||
| 36 | + | var defaultDownloadLimits = downloadLimits{ | |||
| 37 | + | maxManifestBytes: defaultMaxManifestBytes, | |||
| 38 | + | maxFilterFiles: defaultMaxFilterFiles, | |||
| 39 | + | maxFilterFileBytes: defaultMaxFilterFileBytes, | |||
| 40 | + | maxTotalFilterBytes: defaultMaxTotalFilterBytes, | |||
| 41 | + | } | |||
| 42 | + | ||||
| 21 | // Downloader handles downloading filter files from GitHub releases. | 43 | // Downloader handles downloading filter files from GitHub releases. | ||
| 22 | type Downloader struct { | 44 | type Downloader struct { | ||
| 23 | baseURL string | 45 | baseURL string | ||
| 24 | cacheDir string | 46 | cacheDir string | ||
| 25 | httpClient *http.Client | 47 | httpClient *http.Client | ||
| 48 | + | limits downloadLimits | |||
| 26 | } | 49 | } | ||
| 27 | 50 | ||||
| 28 | // NewDownloader creates a new Downloader. | 51 | // NewDownloader creates a new Downloader. | ||
| @@ -34,14 +57,15 @@ func NewDownloader(cacheDir string) *Downloader { | |||||
| 34 | httpClient: &http.Client{ | 57 | httpClient: &http.Client{ | ||
| 35 | Timeout: 60 * time.Second, | 58 | Timeout: 60 * time.Second, | ||
| 36 | }, | 59 | }, | ||
| 60 | + | limits: defaultDownloadLimits, | |||
| 37 | } | 61 | } | ||
| 38 | } | 62 | } | ||
| 39 | 63 | ||||
| 40 | // GetCachedManifest reads the locally cached manifest.json. | 64 | // GetCachedManifest reads the locally cached manifest.json. | ||
| 41 | // Returns nil if no cached manifest exists. | 65 | // Returns nil if no cached manifest exists. | ||
| 42 | func (d *Downloader) GetCachedManifest() (*Manifest, error) { | 66 | func (d *Downloader) GetCachedManifest() (*Manifest, error) { | ||
| 43 | - | path := filepath.Join(d.cacheDir, FilterFiles.Manifest) | 67 | + | manifestPath := filepath.Join(d.cacheDir, FilterFiles.Manifest) |
| 44 | - | data, err := os.ReadFile(path) | 68 | + | data, err := readLimitedFile(manifestPath, d.limits.maxManifestBytes, "cached manifest") |
| 45 | if err != nil { | 69 | if err != nil { | ||
| 46 | if os.IsNotExist(err) { | 70 | if os.IsNotExist(err) { | ||
| 47 | return nil, nil | 71 | return nil, nil | ||
| @@ -53,6 +77,9 @@ func (d *Downloader) GetCachedManifest() (*Manifest, error) { | |||||
| 53 | if err := json.Unmarshal(data, &manifest); err != nil { | 77 | if err := json.Unmarshal(data, &manifest); err != nil { | ||
| 54 | return nil, fmt.Errorf("failed to parse cached manifest: %w", err) | 78 | return nil, fmt.Errorf("failed to parse cached manifest: %w", err) | ||
| 55 | } | 79 | } | ||
| 80 | + | if err := d.validateManifest(&manifest); err != nil { | |||
| 81 | + | return nil, fmt.Errorf("invalid cached manifest: %w", err) | |||
| 82 | + | } | |||
| 56 | return &manifest, nil | 83 | return &manifest, nil | ||
| 57 | } | 84 | } | ||
| 58 | 85 | ||||
| @@ -84,7 +111,7 @@ func (d *Downloader) FetchManifest(ctx context.Context) (*Manifest, error) { | |||||
| 84 | return nil, fmt.Errorf("manifest fetch failed with status %d", resp.StatusCode) | 111 | return nil, fmt.Errorf("manifest fetch failed with status %d", resp.StatusCode) | ||
| 85 | } | 112 | } | ||
| 86 | 113 | ||||
| 87 | - | data, err := io.ReadAll(resp.Body) | 114 | + | data, err := readLimitedResponseBody(resp, d.limits.maxManifestBytes, "manifest") |
| 88 | if err != nil { | 115 | if err != nil { | ||
| 89 | return nil, fmt.Errorf("failed to read manifest body: %w", err) | 116 | return nil, fmt.Errorf("failed to read manifest body: %w", err) | ||
| 90 | } | 117 | } | ||
| @@ -93,6 +120,9 @@ func (d *Downloader) FetchManifest(ctx context.Context) (*Manifest, error) { | |||||
| 93 | if err := json.Unmarshal(data, &manifest); err != nil { | 120 | if err := json.Unmarshal(data, &manifest); err != nil { | ||
| 94 | return nil, fmt.Errorf("failed to parse manifest: %w", err) | 121 | return nil, fmt.Errorf("failed to parse manifest: %w", err) | ||
| 95 | } | 122 | } | ||
| 123 | + | if err := d.validateManifest(&manifest); err != nil { | |||
| 124 | + | return nil, fmt.Errorf("invalid manifest: %w", err) | |||
| 125 | + | } | |||
| 96 | 126 | ||||
| 97 | log.Debug().Str("version", manifest.Version).Msg("fetched manifest") | 127 | log.Debug().Str("version", manifest.Version).Msg("fetched manifest") | ||
| 98 | return &manifest, nil | 128 | return &manifest, nil | ||
| @@ -153,7 +183,12 @@ func (d *Downloader) DownloadFilters(ctx context.Context, onProgress func(Downlo | |||||
| 153 | }) | 183 | }) | ||
| 154 | } | 184 | } | ||
| 155 | 185 | ||||
| 156 | - | path, bytesDownloaded, err := d.downloadFile(ctx, filename) | 186 | + | remainingBytes := d.limits.maxTotalFilterBytes - totalBytes |
| 187 | + | if remainingBytes <= 0 { | |||
| 188 | + | return nil, fmt.Errorf("filter downloads exceed total size limit of %d bytes", d.limits.maxTotalFilterBytes) | |||
| 189 | + | } | |||
| 190 | + | ||||
| 191 | + | path, bytesDownloaded, err := d.downloadFile(ctx, filename, minInt64(d.limits.maxFilterFileBytes, remainingBytes)) | |||
| 157 | if err != nil { | 192 | if err != nil { | ||
| 158 | log.Error().Err(err).Str("file", filename).Msg("failed to download filter file") | 193 | log.Error().Err(err).Str("file", filename).Msg("failed to download filter file") | ||
| 159 | return nil, err | 194 | return nil, err | ||
| @@ -174,9 +209,13 @@ func (d *Downloader) DownloadFilters(ctx context.Context, onProgress func(Downlo | |||||
| 174 | } | 209 | } | ||
| 175 | 210 | ||||
| 176 | // downloadFile downloads a single file and returns its local path. | 211 | // downloadFile downloads a single file and returns its local path. | ||
| 177 | - | func (d *Downloader) downloadFile(ctx context.Context, filename string) (string, int64, error) { | 212 | + | func (d *Downloader) downloadFile(ctx context.Context, filename string, maxBytes int64) (string, int64, error) { |
| 178 | - | url := fmt.Sprintf("%s/%s", d.baseURL, filename) | 213 | + | localPath, safeFilename, err := d.cachePathForFilterFile(filename) |
| 179 | - | localPath := filepath.Join(d.cacheDir, filename) | 214 | + | if err != nil { |
| 215 | + | return "", 0, err | |||
| 216 | + | } | |||
| 217 | + | ||||
| 218 | + | url := fmt.Sprintf("%s/%s", d.baseURL, safeFilename) | |||
| 180 | 219 | ||||
| 181 | req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) | 220 | req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) | ||
| 182 | if err != nil { | 221 | if err != nil { | ||
| @@ -197,25 +236,29 @@ func (d *Downloader) downloadFile(ctx context.Context, filename string) (string, | |||||
| 197 | return "", 0, fmt.Errorf("download %s failed with status %d", filename, resp.StatusCode) | 236 | return "", 0, fmt.Errorf("download %s failed with status %d", filename, resp.StatusCode) | ||
| 198 | } | 237 | } | ||
| 199 | 238 | ||||
| 239 | + | if resp.ContentLength > maxBytes { | |||
| 240 | + | return "", 0, fmt.Errorf("filter file %q too large: content length %d exceeds limit %d", safeFilename, resp.ContentLength, maxBytes) | |||
| 241 | + | } | |||
| 242 | + | ||||
| 200 | // Write to temp file in cacheDir, then atomic rename into destination. | 243 | // Write to temp file in cacheDir, then atomic rename into destination. | ||
| 201 | - | tmpFile, err := os.CreateTemp(d.cacheDir, filepath.Base(filename)+".*.tmp") | 244 | + | tmpFile, err := os.CreateTemp(d.cacheDir, filepath.Base(safeFilename)+".*.tmp") |
| 202 | if err != nil { | 245 | if err != nil { | ||
| 203 | return "", 0, fmt.Errorf("failed to create temp file: %w", err) | 246 | return "", 0, fmt.Errorf("failed to create temp file: %w", err) | ||
| 204 | } | 247 | } | ||
| 205 | tmpPath := tmpFile.Name() | 248 | tmpPath := tmpFile.Name() | ||
| 206 | 249 | ||||
| 207 | - | bytesWritten, err := io.Copy(tmpFile, resp.Body) | 250 | + | bytesWritten, err := copyLimited(tmpFile, resp.Body, maxBytes, fmt.Sprintf("filter file %q", safeFilename)) |
| 208 | if err != nil { | 251 | if err != nil { | ||
| 209 | _ = tmpFile.Close() | 252 | _ = tmpFile.Close() | ||
| 210 | _ = os.Remove(tmpPath) | 253 | _ = os.Remove(tmpPath) | ||
| 211 | - | return "", 0, fmt.Errorf("failed to write %s: %w", filename, err) | 254 | + | return "", 0, fmt.Errorf("failed to write %s: %w", safeFilename, err) |
| 212 | } | 255 | } | ||
| 213 | if err := tmpFile.Close(); err != nil { | 256 | if err := tmpFile.Close(); err != nil { | ||
| 214 | _ = os.Remove(tmpPath) | 257 | _ = os.Remove(tmpPath) | ||
| 215 | return "", 0, fmt.Errorf("failed to close temp file: %w", err) | 258 | return "", 0, fmt.Errorf("failed to close temp file: %w", err) | ||
| 216 | } | 259 | } | ||
| 217 | 260 | ||||
| 218 | - | if err := d.renameTempFile(tmpPath, localPath); err != nil { | 261 | + | if err := d.renameTempFile(tmpPath, safeFilename); err != nil { |
| 219 | _ = os.Remove(tmpPath) | 262 | _ = os.Remove(tmpPath) | ||
| 220 | return "", 0, err | 263 | return "", 0, err | ||
| 221 | } | 264 | } | ||
| @@ -223,14 +266,88 @@ func (d *Downloader) downloadFile(ctx context.Context, filename string) (string, | |||||
| 223 | return localPath, bytesWritten, nil | 266 | return localPath, bytesWritten, nil | ||
| 224 | } | 267 | } | ||
| 225 | 268 | ||||
| 226 | - | func (*Downloader) renameTempFile(tmpPath, localPath string) error { | 269 | + | func (d *Downloader) cachePathForFilterFile(filename string) (string, string, error) { |
| 227 | - | parentDir := filepath.Dir(localPath) | 270 | + | safeFilename, err := validateFilterFilename(filename) |
| 271 | + | if err != nil { | |||
| 272 | + | return "", "", err | |||
| 273 | + | } | |||
| 274 | + | ||||
| 275 | + | cacheDirAbs, err := filepath.Abs(d.cacheDir) | |||
| 276 | + | if err != nil { | |||
| 277 | + | return "", "", fmt.Errorf("failed to resolve cache dir: %w", err) | |||
| 278 | + | } | |||
| 279 | + | localPath := filepath.Join(d.cacheDir, filepath.FromSlash(safeFilename)) | |||
| 280 | + | targetAbs, err := filepath.Abs(localPath) | |||
| 281 | + | if err != nil { | |||
| 282 | + | return "", "", fmt.Errorf("failed to resolve cache path: %w", err) | |||
| 283 | + | } | |||
| 284 | + | if err := ensurePathInside(cacheDirAbs, targetAbs); err != nil { | |||
| 285 | + | return "", "", fmt.Errorf("invalid filter filename %q: %w", filename, err) | |||
| 286 | + | } | |||
| 287 | + | ||||
| 288 | + | return localPath, safeFilename, nil | |||
| 289 | + | } | |||
| 290 | + | ||||
| 291 | + | func validateFilterFilename(filename string) (string, error) { | |||
| 292 | + | if filename == "" { | |||
| 293 | + | return "", fmt.Errorf("invalid filter filename: empty") | |||
| 294 | + | } | |||
| 295 | + | if strings.TrimSpace(filename) != filename { | |||
| 296 | + | return "", fmt.Errorf("invalid filter filename %q: surrounding whitespace is not allowed", filename) | |||
| 297 | + | } | |||
| 298 | + | if filepath.IsAbs(filename) || strings.HasPrefix(filename, "/") { | |||
| 299 | + | return "", fmt.Errorf("invalid filter filename %q: absolute paths are not allowed", filename) | |||
| 300 | + | } | |||
| 301 | + | if strings.Contains(filename, "\\") { | |||
| 302 | + | return "", fmt.Errorf("invalid filter filename %q: backslashes are not allowed", filename) | |||
| 303 | + | } | |||
| 304 | + | if strings.ContainsAny(filename, "?#") { | |||
| 305 | + | return "", fmt.Errorf("invalid filter filename %q: URL control characters are not allowed", filename) | |||
| 306 | + | } | |||
| 228 | 307 | ||||
| 229 | - | if err := os.MkdirAll(parentDir, cacheDirPerm); err != nil { | 308 | + | parts := strings.Split(filename, "/") |
| 230 | - | return fmt.Errorf("failed to create target dir: %w", err) | 309 | + | for _, part := range parts { |
| 310 | + | if part == "" || part == "." || part == ".." { | |||
| 311 | + | return "", fmt.Errorf("invalid filter filename %q: path traversal components are not allowed", filename) | |||
| 312 | + | } | |||
| 313 | + | } | |||
| 314 | + | ||||
| 315 | + | clean := pathpkg.Clean(filename) | |||
| 316 | + | if pathpkg.Ext(clean) != ".json" { | |||
| 317 | + | return "", fmt.Errorf("invalid filter filename %q: expected .json extension", filename) | |||
| 318 | + | } | |||
| 319 | + | ||||
| 320 | + | return clean, nil | |||
| 321 | + | } | |||
| 322 | + | ||||
| 323 | + | func ensurePathInside(baseAbs, targetAbs string) error { | |||
| 324 | + | rel, err := filepath.Rel(baseAbs, targetAbs) | |||
| 325 | + | if err != nil { | |||
| 326 | + | return fmt.Errorf("failed to compare with cache dir: %w", err) | |||
| 327 | + | } | |||
| 328 | + | if rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) { | |||
| 329 | + | return fmt.Errorf("resolved path escapes cache dir") | |||
| 330 | + | } | |||
| 331 | + | return nil | |||
| 332 | + | } | |||
| 333 | + | ||||
| 334 | + | func (d *Downloader) renameTempFile(tmpPath, safeFilename string) error { | |||
| 335 | + | relTarget := filepath.FromSlash(safeFilename) | |||
| 336 | + | parentDir := filepath.Dir(relTarget) | |||
| 337 | + | ||||
| 338 | + | root, err := os.OpenRoot(d.cacheDir) | |||
| 339 | + | if err != nil { | |||
| 340 | + | return fmt.Errorf("failed to open cache root: %w", err) | |||
| 341 | + | } | |||
| 342 | + | defer func() { _ = root.Close() }() | |||
| 343 | + | ||||
| 344 | + | if parentDir != "." { | |||
| 345 | + | if err := root.MkdirAll(parentDir, cacheDirPerm); err != nil { | |||
| 346 | + | return fmt.Errorf("failed to create target dir: %w", err) | |||
| 347 | + | } | |||
| 231 | } | 348 | } | ||
| 232 | 349 | ||||
| 233 | - | if err := os.Rename(tmpPath, localPath); err != nil { | 350 | + | if err := root.Rename(filepath.Base(tmpPath), relTarget); err != nil { |
| 234 | return fmt.Errorf("failed to rename temp file: %w", err) | 351 | return fmt.Errorf("failed to rename temp file: %w", err) | ||
| 235 | } | 352 | } | ||
| 236 | 353 | ||||
| @@ -239,6 +356,10 @@ func (*Downloader) renameTempFile(tmpPath, localPath string) error { | |||||
| 239 | 356 | ||||
| 240 | // cacheManifest writes an already-fetched manifest to disk. | 357 | // cacheManifest writes an already-fetched manifest to disk. | ||
| 241 | func (d *Downloader) cacheManifest(manifest *Manifest) error { | 358 | func (d *Downloader) cacheManifest(manifest *Manifest) error { | ||
| 359 | + | if err := d.validateManifest(manifest); err != nil { | |||
| 360 | + | return fmt.Errorf("invalid manifest: %w", err) | |||
| 361 | + | } | |||
| 362 | + | ||||
| 242 | data, err := json.MarshalIndent(manifest, "", " ") | 363 | data, err := json.MarshalIndent(manifest, "", " ") | ||
| 243 | if err != nil { | 364 | if err != nil { | ||
| 244 | return fmt.Errorf("failed to marshal manifest: %w", err) | 365 | return fmt.Errorf("failed to marshal manifest: %w", err) | ||
| @@ -262,8 +383,11 @@ func (d *Downloader) GetCachedFilterPaths() []string { | |||||
| 262 | } | 383 | } | ||
| 263 | paths := make([]string, 0, len(manifest.Combined.Files)) | 384 | paths := make([]string, 0, len(manifest.Combined.Files)) | ||
| 264 | for _, filename := range manifest.Combined.Files { | 385 | for _, filename := range manifest.Combined.Files { | ||
| 265 | - | path := filepath.Join(d.cacheDir, filename) | 386 | + | path, safeFilename, err := d.cachePathForFilterFile(filename) |
| 266 | - | if _, err := os.Stat(path); err != nil { | 387 | + | if err != nil { |
| 388 | + | return nil | |||
| 389 | + | } | |||
| 390 | + | if !d.hasCachedFilterFile(safeFilename) { | |||
| 267 | return nil | 391 | return nil | ||
| 268 | } | 392 | } | ||
| 269 | paths = append(paths, path) | 393 | paths = append(paths, path) | ||
| @@ -314,6 +438,94 @@ func (d *Downloader) ClearCache() error { | |||||
| 314 | return os.RemoveAll(d.cacheDir) | 438 | return os.RemoveAll(d.cacheDir) | ||
| 315 | } | 439 | } | ||
| 316 | 440 | ||||
| 441 | + | func (d *Downloader) hasCachedFilterFile(safeFilename string) bool { | |||
| 442 | + | root, err := os.OpenRoot(d.cacheDir) | |||
| 443 | + | if err != nil { | |||
| 444 | + | return false | |||
| 445 | + | } | |||
| 446 | + | defer func() { _ = root.Close() }() | |||
| 447 | + | ||||
| 448 | + | info, err := root.Stat(filepath.FromSlash(safeFilename)) | |||
| 449 | + | return err == nil && !info.IsDir() | |||
| 450 | + | } | |||
| 451 | + | ||||
| 452 | + | func (d *Downloader) validateManifest(manifest *Manifest) error { | |||
| 453 | + | if manifest == nil { | |||
| 454 | + | return fmt.Errorf("manifest is nil") | |||
| 455 | + | } | |||
| 456 | + | if len(manifest.Combined.Files) > d.limits.maxFilterFiles { | |||
| 457 | + | return fmt.Errorf("manifest lists %d combined filter files, limit is %d", len(manifest.Combined.Files), d.limits.maxFilterFiles) | |||
| 458 | + | } | |||
| 459 | + | for _, filename := range manifest.Combined.Files { | |||
| 460 | + | if _, _, err := d.cachePathForFilterFile(filename); err != nil { | |||
| 461 | + | return err | |||
| 462 | + | } | |||
| 463 | + | } | |||
| 464 | + | return nil | |||
| 465 | + | } | |||
| 466 | + | ||||
| 467 | + | func readLimitedFile(filename string, limit int64, label string) ([]byte, error) { | |||
| 468 | + | file, err := os.Open(filename) | |||
| 469 | + | if err != nil { | |||
| 470 | + | return nil, err | |||
| 471 | + | } | |||
| 472 | + | defer func() { _ = file.Close() }() | |||
| 473 | + | ||||
| 474 | + | info, err := file.Stat() | |||
| 475 | + | if err != nil { | |||
| 476 | + | return nil, err | |||
| 477 | + | } | |||
| 478 | + | if info.Size() > limit { | |||
| 479 | + | return nil, fmt.Errorf("%s too large: size %d exceeds limit %d", label, info.Size(), limit) | |||
| 480 | + | } | |||
| 481 | + | ||||
| 482 | + | return readLimitedReader(file, limit, label) | |||
| 483 | + | } | |||
| 484 | + | ||||
| 485 | + | func readLimitedResponseBody(resp *http.Response, limit int64, label string) ([]byte, error) { | |||
| 486 | + | if resp.ContentLength > limit { | |||
| 487 | + | return nil, fmt.Errorf("%s too large: content length %d exceeds limit %d", label, resp.ContentLength, limit) | |||
| 488 | + | } | |||
| 489 | + | return readLimitedReader(resp.Body, limit, label) | |||
| 490 | + | } | |||
| 491 | + | ||||
| 492 | + | func readLimitedReader(r io.Reader, limit int64, label string) ([]byte, error) { | |||
| 493 | + | if limit <= 0 { | |||
| 494 | + | return nil, fmt.Errorf("%s size limit must be positive", label) | |||
| 495 | + | } | |||
| 496 | + | ||||
| 497 | + | data, err := io.ReadAll(io.LimitReader(r, limit+1)) | |||
| 498 | + | if err != nil { | |||
| 499 | + | return nil, err | |||
| 500 | + | } | |||
| 501 | + | if int64(len(data)) > limit { | |||
| 502 | + | return nil, fmt.Errorf("%s too large: exceeds limit %d bytes", label, limit) | |||
| 503 | + | } | |||
| 504 | + | return data, nil | |||
| 505 | + | } | |||
| 506 | + | ||||
| 507 | + | func copyLimited(dst io.Writer, src io.Reader, limit int64, label string) (int64, error) { | |||
| 508 | + | if limit <= 0 { | |||
| 509 | + | return 0, fmt.Errorf("%s size limit must be positive", label) | |||
| 510 | + | } | |||
| 511 | + | ||||
| 512 | + | written, err := io.Copy(dst, io.LimitReader(src, limit+1)) | |||
| 513 | + | if err != nil { | |||
| 514 | + | return written, err | |||
| 515 | + | } | |||
| 516 | + | if written > limit { | |||
| 517 | + | return written, fmt.Errorf("%s too large: exceeds limit %d bytes", label, limit) | |||
| 518 | + | } | |||
| 519 | + | return written, nil | |||
| 520 | + | } | |||
| 521 | + | ||||
| 522 | + | func minInt64(a, b int64) int64 { | |||
| 523 | + | if a < b { | |||
| 524 | + | return a | |||
| 525 | + | } | |||
| 526 | + | return b | |||
| 527 | + | } | |||
| 528 | + | ||||
| 317 | // IsCacheStale checks if the cached manifest is older than maxAge. | 529 | // IsCacheStale checks if the cached manifest is older than maxAge. | ||
| 318 | // Returns true if cache is stale or doesn't exist. | 530 | // Returns true if cache is stale or doesn't exist. | ||
| 319 | func (d *Downloader) IsCacheStale(maxAge time.Duration) bool { | 531 | func (d *Downloader) IsCacheStale(maxAge time.Duration) bool { | ||
M internal/infrastructure/filtering/downloader_test.go +208 -3
| @@ -2,11 +2,13 @@ package filtering | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "encoding/json" | |
| 5 | 6 | "io" | |
| 6 | 7 | "net/http" | |
| 7 | 8 | "net/http/httptest" | |
| 8 | 9 | "os" | |
| 9 | 10 | "path/filepath" | |
| 11 | + | "strings" | |
| 10 | 12 | "testing" | |
| 11 | 13 | ||
| 12 | 14 | "github.com/stretchr/testify/require" | |
| @@ -46,7 +48,7 @@ func TestDownloader_DownloadFile_RecoversWhenTargetDirDeletedConcurrently(t *tes | |||
| 46 | 48 | } | |
| 47 | 49 | resultCh := make(chan result, 1) | |
| 48 | 50 | go func() { | |
| 49 | - | path, n, err := d.downloadFile(context.Background(), filename) | |
| 51 | + | path, n, err := d.downloadFile(context.Background(), filename, d.limits.maxFilterFileBytes) | |
| 50 | 52 | resultCh <- result{path: path, n: n, err: err} | |
| 51 | 53 | }() | |
| 52 | 54 | ||
| @@ -64,6 +66,209 @@ func TestDownloader_DownloadFile_RecoversWhenTargetDirDeletedConcurrently(t *tes | |||
| 64 | 66 | require.Contains(t, string(content), "url-filter") | |
| 65 | 67 | } | |
| 66 | 68 | ||
| 69 | + | func TestDownloader_DownloadFilters_RejectsUnsafeManifestFilenames(t *testing.T) { | |
| 70 | + | cacheDir := filepath.Join(t.TempDir(), "cache") | |
| 71 | + | requested := make([]string, 0, 1) | |
| 72 | + | ||
| 73 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 74 | + | requested = append(requested, r.URL.Path) | |
| 75 | + | if r.URL.Path != "/"+FilterFiles.Manifest { | |
| 76 | + | http.NotFound(w, r) | |
| 77 | + | return | |
| 78 | + | } | |
| 79 | + | _, _ = io.WriteString(w, `{"version":"test","combined":{"total_rules":1,"files":["../escape.json"]}}`) | |
| 80 | + | })) | |
| 81 | + | defer server.Close() | |
| 82 | + | ||
| 83 | + | d := NewDownloader(cacheDir) | |
| 84 | + | d.baseURL = server.URL | |
| 85 | + | ||
| 86 | + | paths, err := d.DownloadFilters(context.Background(), nil) | |
| 87 | + | require.Error(t, err) | |
| 88 | + | require.Nil(t, paths) | |
| 89 | + | require.Contains(t, err.Error(), "invalid manifest") | |
| 90 | + | require.Equal(t, []string{"/" + FilterFiles.Manifest}, requested) | |
| 91 | + | } | |
| 92 | + | ||
| 93 | + | func TestDownloader_DownloadFilters_AllowsSafeNestedManifestFilenames(t *testing.T) { | |
| 94 | + | cacheDir := filepath.Join(t.TempDir(), "cache") | |
| 95 | + | filename := "nested/combined-part1.json" | |
| 96 | + | body := `[{"trigger":{"url-filter":"test"},"action":{"type":"block"}}]` | |
| 97 | + | ||
| 98 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 99 | + | switch r.URL.Path { | |
| 100 | + | case "/" + FilterFiles.Manifest: | |
| 101 | + | _, _ = io.WriteString(w, `{"version":"test","combined":{"total_rules":1,"files":["`+filename+`"]}}`) | |
| 102 | + | case "/" + filename: | |
| 103 | + | _, _ = io.WriteString(w, body) | |
| 104 | + | default: | |
| 105 | + | http.NotFound(w, r) | |
| 106 | + | } | |
| 107 | + | })) | |
| 108 | + | defer server.Close() | |
| 109 | + | ||
| 110 | + | d := NewDownloader(cacheDir) | |
| 111 | + | d.baseURL = server.URL | |
| 112 | + | ||
| 113 | + | paths, err := d.DownloadFilters(context.Background(), nil) | |
| 114 | + | require.NoError(t, err) | |
| 115 | + | ||
| 116 | + | wantPath := filepath.Join(cacheDir, "nested", "combined-part1.json") | |
| 117 | + | require.Equal(t, []string{wantPath}, paths) | |
| 118 | + | content, err := os.ReadFile(wantPath) | |
| 119 | + | require.NoError(t, err) | |
| 120 | + | require.Equal(t, body, string(content)) | |
| 121 | + | } | |
| 122 | + | ||
| 123 | + | func TestDownloader_FetchManifest_RejectsOversizedManifest(t *testing.T) { | |
| 124 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 125 | + | if r.URL.Path != "/"+FilterFiles.Manifest { | |
| 126 | + | http.NotFound(w, r) | |
| 127 | + | return | |
| 128 | + | } | |
| 129 | + | _, _ = io.WriteString(w, strings.Repeat("x", 17)) | |
| 130 | + | })) | |
| 131 | + | defer server.Close() | |
| 132 | + | ||
| 133 | + | d := NewDownloader(t.TempDir()) | |
| 134 | + | d.baseURL = server.URL | |
| 135 | + | d.limits.maxManifestBytes = 16 | |
| 136 | + | ||
| 137 | + | manifest, err := d.FetchManifest(context.Background()) | |
| 138 | + | require.Error(t, err) | |
| 139 | + | require.Nil(t, manifest) | |
| 140 | + | require.Contains(t, err.Error(), "manifest too large") | |
| 141 | + | } | |
| 142 | + | ||
| 143 | + | func TestDownloader_FetchManifest_RejectsTooManyFiles(t *testing.T) { | |
| 144 | + | payload, err := json.Marshal(Manifest{ | |
| 145 | + | Version: "test", | |
| 146 | + | Combined: CombinedInfo{ | |
| 147 | + | TotalRules: 3, | |
| 148 | + | Files: []string{"part1.json", "part2.json", "part3.json"}, | |
| 149 | + | }, | |
| 150 | + | }) | |
| 151 | + | require.NoError(t, err) | |
| 152 | + | ||
| 153 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 154 | + | if r.URL.Path != "/"+FilterFiles.Manifest { | |
| 155 | + | http.NotFound(w, r) | |
| 156 | + | return | |
| 157 | + | } | |
| 158 | + | _, _ = w.Write(payload) | |
| 159 | + | })) | |
| 160 | + | defer server.Close() | |
| 161 | + | ||
| 162 | + | d := NewDownloader(t.TempDir()) | |
| 163 | + | d.baseURL = server.URL | |
| 164 | + | d.limits.maxFilterFiles = 2 | |
| 165 | + | ||
| 166 | + | manifest, err := d.FetchManifest(context.Background()) | |
| 167 | + | require.Error(t, err) | |
| 168 | + | require.Nil(t, manifest) | |
| 169 | + | require.Contains(t, err.Error(), "limit is 2") | |
| 170 | + | } | |
| 171 | + | ||
| 172 | + | func TestDownloader_DownloadFilters_RejectsOversizedFilterFile(t *testing.T) { | |
| 173 | + | cacheDir := filepath.Join(t.TempDir(), "cache") | |
| 174 | + | filename := "combined.json" | |
| 175 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 176 | + | switch r.URL.Path { | |
| 177 | + | case "/" + FilterFiles.Manifest: | |
| 178 | + | _, _ = io.WriteString(w, `{"version":"test","combined":{"total_rules":1,"files":["combined.json"]}}`) | |
| 179 | + | case "/" + filename: | |
| 180 | + | _, _ = io.WriteString(w, "12345") | |
| 181 | + | default: | |
| 182 | + | http.NotFound(w, r) | |
| 183 | + | } | |
| 184 | + | })) | |
| 185 | + | defer server.Close() | |
| 186 | + | ||
| 187 | + | d := NewDownloader(cacheDir) | |
| 188 | + | d.baseURL = server.URL | |
| 189 | + | d.limits.maxFilterFileBytes = 4 | |
| 190 | + | d.limits.maxTotalFilterBytes = 32 | |
| 191 | + | ||
| 192 | + | paths, err := d.DownloadFilters(context.Background(), nil) | |
| 193 | + | require.Error(t, err) | |
| 194 | + | require.Nil(t, paths) | |
| 195 | + | require.Contains(t, err.Error(), "too large") | |
| 196 | + | require.NoFileExists(t, filepath.Join(cacheDir, filename)) | |
| 197 | + | } | |
| 198 | + | ||
| 199 | + | func TestDownloader_DownloadFilters_EnforcesTotalDownloadLimit(t *testing.T) { | |
| 200 | + | cacheDir := filepath.Join(t.TempDir(), "cache") | |
| 201 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 202 | + | switch r.URL.Path { | |
| 203 | + | case "/" + FilterFiles.Manifest: | |
| 204 | + | _, _ = io.WriteString(w, `{"version":"test","combined":{"total_rules":2,"files":["part1.json","part2.json"]}}`) | |
| 205 | + | case "/part1.json", "/part2.json": | |
| 206 | + | _, _ = io.WriteString(w, "12345") | |
| 207 | + | default: | |
| 208 | + | http.NotFound(w, r) | |
| 209 | + | } | |
| 210 | + | })) | |
| 211 | + | defer server.Close() | |
| 212 | + | ||
| 213 | + | d := NewDownloader(cacheDir) | |
| 214 | + | d.baseURL = server.URL | |
| 215 | + | d.limits.maxFilterFileBytes = 8 | |
| 216 | + | d.limits.maxTotalFilterBytes = 8 | |
| 217 | + | ||
| 218 | + | paths, err := d.DownloadFilters(context.Background(), nil) | |
| 219 | + | require.Error(t, err) | |
| 220 | + | require.Nil(t, paths) | |
| 221 | + | require.Contains(t, err.Error(), "too large") | |
| 222 | + | require.FileExists(t, filepath.Join(cacheDir, "part1.json")) | |
| 223 | + | require.NoFileExists(t, filepath.Join(cacheDir, "part2.json")) | |
| 224 | + | } | |
| 225 | + | ||
| 226 | + | func TestDownloader_DownloadFilters_RejectsSymlinkEscape(t *testing.T) { | |
| 227 | + | cacheDir := filepath.Join(t.TempDir(), "cache") | |
| 228 | + | outsideDir := filepath.Join(t.TempDir(), "outside") | |
| 229 | + | require.NoError(t, os.MkdirAll(cacheDir, cacheDirPerm)) | |
| 230 | + | require.NoError(t, os.MkdirAll(outsideDir, cacheDirPerm)) | |
| 231 | + | require.NoError(t, os.Symlink(outsideDir, filepath.Join(cacheDir, "nested"))) | |
| 232 | + | ||
| 233 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 234 | + | switch r.URL.Path { | |
| 235 | + | case "/" + FilterFiles.Manifest: | |
| 236 | + | _, _ = io.WriteString(w, `{"version":"test","combined":{"total_rules":1,"files":["nested/combined.json"]}}`) | |
| 237 | + | case "/nested/combined.json": | |
| 238 | + | _, _ = io.WriteString(w, `[{"trigger":{"url-filter":"test"},"action":{"type":"block"}}]`) | |
| 239 | + | default: | |
| 240 | + | http.NotFound(w, r) | |
| 241 | + | } | |
| 242 | + | })) | |
| 243 | + | defer server.Close() | |
| 244 | + | ||
| 245 | + | d := NewDownloader(cacheDir) | |
| 246 | + | d.baseURL = server.URL | |
| 247 | + | ||
| 248 | + | paths, err := d.DownloadFilters(context.Background(), nil) | |
| 249 | + | require.Error(t, err) | |
| 250 | + | require.Nil(t, paths) | |
| 251 | + | require.Contains(t, err.Error(), "failed to create target dir") | |
| 252 | + | require.NoFileExists(t, filepath.Join(outsideDir, "combined.json")) | |
| 253 | + | } | |
| 254 | + | ||
| 255 | + | func TestDownloader_GetCachedFilterPaths_RejectsSymlinkEscape(t *testing.T) { | |
| 256 | + | cacheDir := filepath.Join(t.TempDir(), "cache") | |
| 257 | + | outsideDir := filepath.Join(t.TempDir(), "outside") | |
| 258 | + | require.NoError(t, os.MkdirAll(cacheDir, cacheDirPerm)) | |
| 259 | + | require.NoError(t, os.MkdirAll(outsideDir, cacheDirPerm)) | |
| 260 | + | require.NoError(t, os.Symlink(outsideDir, filepath.Join(cacheDir, "nested"))) | |
| 261 | + | require.NoError(t, os.WriteFile(filepath.Join(outsideDir, "combined.json"), []byte("[]"), manifestFilePerm)) | |
| 262 | + | require.NoError(t, os.WriteFile( | |
| 263 | + | filepath.Join(cacheDir, FilterFiles.Manifest), | |
| 264 | + | []byte(`{"version":"test","combined":{"total_rules":1,"files":["nested/combined.json"]}}`), | |
| 265 | + | manifestFilePerm, | |
| 266 | + | )) | |
| 267 | + | ||
| 268 | + | d := NewDownloader(cacheDir) | |
| 269 | + | require.Nil(t, d.GetCachedFilterPaths()) | |
| 270 | + | } | |
| 271 | + | ||
| 67 | 272 | func TestDownloader_RenameTempFile_RequiresDirectoryParent(t *testing.T) { | |
| 68 | 273 | t.Parallel() | |
| 69 | 274 | ||
| @@ -78,7 +283,7 @@ func TestDownloader_RenameTempFile_RequiresDirectoryParent(t *testing.T) { | |||
| 78 | 283 | ||
| 79 | 284 | d := NewDownloader(cacheDir) | |
| 80 | 285 | ||
| 81 | - | err := d.renameTempFile(tmpFile, filepath.Join(parentAsFile, "filter.json")) | |
| 286 | + | err := d.renameTempFile(tmpFile, "target/filter.json") | |
| 82 | 287 | require.Error(t, err) | |
| 83 | - | require.Contains(t, err.Error(), "not a directory") | |
| 288 | + | require.Contains(t, err.Error(), "failed to create target dir") | |
| 84 | 289 | } | |
| @@ -2,11 +2,13 @@ package filtering | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "encoding/json" | |||
| 5 | "io" | 6 | "io" | ||
| 6 | "net/http" | 7 | "net/http" | ||
| 7 | "net/http/httptest" | 8 | "net/http/httptest" | ||
| 8 | "os" | 9 | "os" | ||
| 9 | "path/filepath" | 10 | "path/filepath" | ||
| 11 | + | "strings" | |||
| 10 | "testing" | 12 | "testing" | ||
| 11 | 13 | ||||
| 12 | "github.com/stretchr/testify/require" | 14 | "github.com/stretchr/testify/require" | ||
| @@ -46,7 +48,7 @@ func TestDownloader_DownloadFile_RecoversWhenTargetDirDeletedConcurrently(t *tes | |||||
| 46 | } | 48 | } | ||
| 47 | resultCh := make(chan result, 1) | 49 | resultCh := make(chan result, 1) | ||
| 48 | go func() { | 50 | go func() { | ||
| 49 | - | path, n, err := d.downloadFile(context.Background(), filename) | 51 | + | path, n, err := d.downloadFile(context.Background(), filename, d.limits.maxFilterFileBytes) |
| 50 | resultCh <- result{path: path, n: n, err: err} | 52 | resultCh <- result{path: path, n: n, err: err} | ||
| 51 | }() | 53 | }() | ||
| 52 | 54 | ||||
| @@ -64,6 +66,209 @@ func TestDownloader_DownloadFile_RecoversWhenTargetDirDeletedConcurrently(t *tes | |||||
| 64 | require.Contains(t, string(content), "url-filter") | 66 | require.Contains(t, string(content), "url-filter") | ||
| 65 | } | 67 | } | ||
| 66 | 68 | ||||
| 69 | + | func TestDownloader_DownloadFilters_RejectsUnsafeManifestFilenames(t *testing.T) { | |||
| 70 | + | cacheDir := filepath.Join(t.TempDir(), "cache") | |||
| 71 | + | requested := make([]string, 0, 1) | |||
| 72 | + | ||||
| 73 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 74 | + | requested = append(requested, r.URL.Path) | |||
| 75 | + | if r.URL.Path != "/"+FilterFiles.Manifest { | |||
| 76 | + | http.NotFound(w, r) | |||
| 77 | + | return | |||
| 78 | + | } | |||
| 79 | + | _, _ = io.WriteString(w, `{"version":"test","combined":{"total_rules":1,"files":["../escape.json"]}}`) | |||
| 80 | + | })) | |||
| 81 | + | defer server.Close() | |||
| 82 | + | ||||
| 83 | + | d := NewDownloader(cacheDir) | |||
| 84 | + | d.baseURL = server.URL | |||
| 85 | + | ||||
| 86 | + | paths, err := d.DownloadFilters(context.Background(), nil) | |||
| 87 | + | require.Error(t, err) | |||
| 88 | + | require.Nil(t, paths) | |||
| 89 | + | require.Contains(t, err.Error(), "invalid manifest") | |||
| 90 | + | require.Equal(t, []string{"/" + FilterFiles.Manifest}, requested) | |||
| 91 | + | } | |||
| 92 | + | ||||
| 93 | + | func TestDownloader_DownloadFilters_AllowsSafeNestedManifestFilenames(t *testing.T) { | |||
| 94 | + | cacheDir := filepath.Join(t.TempDir(), "cache") | |||
| 95 | + | filename := "nested/combined-part1.json" | |||
| 96 | + | body := `[{"trigger":{"url-filter":"test"},"action":{"type":"block"}}]` | |||
| 97 | + | ||||
| 98 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 99 | + | switch r.URL.Path { | |||
| 100 | + | case "/" + FilterFiles.Manifest: | |||
| 101 | + | _, _ = io.WriteString(w, `{"version":"test","combined":{"total_rules":1,"files":["`+filename+`"]}}`) | |||
| 102 | + | case "/" + filename: | |||
| 103 | + | _, _ = io.WriteString(w, body) | |||
| 104 | + | default: | |||
| 105 | + | http.NotFound(w, r) | |||
| 106 | + | } | |||
| 107 | + | })) | |||
| 108 | + | defer server.Close() | |||
| 109 | + | ||||
| 110 | + | d := NewDownloader(cacheDir) | |||
| 111 | + | d.baseURL = server.URL | |||
| 112 | + | ||||
| 113 | + | paths, err := d.DownloadFilters(context.Background(), nil) | |||
| 114 | + | require.NoError(t, err) | |||
| 115 | + | ||||
| 116 | + | wantPath := filepath.Join(cacheDir, "nested", "combined-part1.json") | |||
| 117 | + | require.Equal(t, []string{wantPath}, paths) | |||
| 118 | + | content, err := os.ReadFile(wantPath) | |||
| 119 | + | require.NoError(t, err) | |||
| 120 | + | require.Equal(t, body, string(content)) | |||
| 121 | + | } | |||
| 122 | + | ||||
| 123 | + | func TestDownloader_FetchManifest_RejectsOversizedManifest(t *testing.T) { | |||
| 124 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 125 | + | if r.URL.Path != "/"+FilterFiles.Manifest { | |||
| 126 | + | http.NotFound(w, r) | |||
| 127 | + | return | |||
| 128 | + | } | |||
| 129 | + | _, _ = io.WriteString(w, strings.Repeat("x", 17)) | |||
| 130 | + | })) | |||
| 131 | + | defer server.Close() | |||
| 132 | + | ||||
| 133 | + | d := NewDownloader(t.TempDir()) | |||
| 134 | + | d.baseURL = server.URL | |||
| 135 | + | d.limits.maxManifestBytes = 16 | |||
| 136 | + | ||||
| 137 | + | manifest, err := d.FetchManifest(context.Background()) | |||
| 138 | + | require.Error(t, err) | |||
| 139 | + | require.Nil(t, manifest) | |||
| 140 | + | require.Contains(t, err.Error(), "manifest too large") | |||
| 141 | + | } | |||
| 142 | + | ||||
| 143 | + | func TestDownloader_FetchManifest_RejectsTooManyFiles(t *testing.T) { | |||
| 144 | + | payload, err := json.Marshal(Manifest{ | |||
| 145 | + | Version: "test", | |||
| 146 | + | Combined: CombinedInfo{ | |||
| 147 | + | TotalRules: 3, | |||
| 148 | + | Files: []string{"part1.json", "part2.json", "part3.json"}, | |||
| 149 | + | }, | |||
| 150 | + | }) | |||
| 151 | + | require.NoError(t, err) | |||
| 152 | + | ||||
| 153 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 154 | + | if r.URL.Path != "/"+FilterFiles.Manifest { | |||
| 155 | + | http.NotFound(w, r) | |||
| 156 | + | return | |||
| 157 | + | } | |||
| 158 | + | _, _ = w.Write(payload) | |||
| 159 | + | })) | |||
| 160 | + | defer server.Close() | |||
| 161 | + | ||||
| 162 | + | d := NewDownloader(t.TempDir()) | |||
| 163 | + | d.baseURL = server.URL | |||
| 164 | + | d.limits.maxFilterFiles = 2 | |||
| 165 | + | ||||
| 166 | + | manifest, err := d.FetchManifest(context.Background()) | |||
| 167 | + | require.Error(t, err) | |||
| 168 | + | require.Nil(t, manifest) | |||
| 169 | + | require.Contains(t, err.Error(), "limit is 2") | |||
| 170 | + | } | |||
| 171 | + | ||||
| 172 | + | func TestDownloader_DownloadFilters_RejectsOversizedFilterFile(t *testing.T) { | |||
| 173 | + | cacheDir := filepath.Join(t.TempDir(), "cache") | |||
| 174 | + | filename := "combined.json" | |||
| 175 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 176 | + | switch r.URL.Path { | |||
| 177 | + | case "/" + FilterFiles.Manifest: | |||
| 178 | + | _, _ = io.WriteString(w, `{"version":"test","combined":{"total_rules":1,"files":["combined.json"]}}`) | |||
| 179 | + | case "/" + filename: | |||
| 180 | + | _, _ = io.WriteString(w, "12345") | |||
| 181 | + | default: | |||
| 182 | + | http.NotFound(w, r) | |||
| 183 | + | } | |||
| 184 | + | })) | |||
| 185 | + | defer server.Close() | |||
| 186 | + | ||||
| 187 | + | d := NewDownloader(cacheDir) | |||
| 188 | + | d.baseURL = server.URL | |||
| 189 | + | d.limits.maxFilterFileBytes = 4 | |||
| 190 | + | d.limits.maxTotalFilterBytes = 32 | |||
| 191 | + | ||||
| 192 | + | paths, err := d.DownloadFilters(context.Background(), nil) | |||
| 193 | + | require.Error(t, err) | |||
| 194 | + | require.Nil(t, paths) | |||
| 195 | + | require.Contains(t, err.Error(), "too large") | |||
| 196 | + | require.NoFileExists(t, filepath.Join(cacheDir, filename)) | |||
| 197 | + | } | |||
| 198 | + | ||||
| 199 | + | func TestDownloader_DownloadFilters_EnforcesTotalDownloadLimit(t *testing.T) { | |||
| 200 | + | cacheDir := filepath.Join(t.TempDir(), "cache") | |||
| 201 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 202 | + | switch r.URL.Path { | |||
| 203 | + | case "/" + FilterFiles.Manifest: | |||
| 204 | + | _, _ = io.WriteString(w, `{"version":"test","combined":{"total_rules":2,"files":["part1.json","part2.json"]}}`) | |||
| 205 | + | case "/part1.json", "/part2.json": | |||
| 206 | + | _, _ = io.WriteString(w, "12345") | |||
| 207 | + | default: | |||
| 208 | + | http.NotFound(w, r) | |||
| 209 | + | } | |||
| 210 | + | })) | |||
| 211 | + | defer server.Close() | |||
| 212 | + | ||||
| 213 | + | d := NewDownloader(cacheDir) | |||
| 214 | + | d.baseURL = server.URL | |||
| 215 | + | d.limits.maxFilterFileBytes = 8 | |||
| 216 | + | d.limits.maxTotalFilterBytes = 8 | |||
| 217 | + | ||||
| 218 | + | paths, err := d.DownloadFilters(context.Background(), nil) | |||
| 219 | + | require.Error(t, err) | |||
| 220 | + | require.Nil(t, paths) | |||
| 221 | + | require.Contains(t, err.Error(), "too large") | |||
| 222 | + | require.FileExists(t, filepath.Join(cacheDir, "part1.json")) | |||
| 223 | + | require.NoFileExists(t, filepath.Join(cacheDir, "part2.json")) | |||
| 224 | + | } | |||
| 225 | + | ||||
| 226 | + | func TestDownloader_DownloadFilters_RejectsSymlinkEscape(t *testing.T) { | |||
| 227 | + | cacheDir := filepath.Join(t.TempDir(), "cache") | |||
| 228 | + | outsideDir := filepath.Join(t.TempDir(), "outside") | |||
| 229 | + | require.NoError(t, os.MkdirAll(cacheDir, cacheDirPerm)) | |||
| 230 | + | require.NoError(t, os.MkdirAll(outsideDir, cacheDirPerm)) | |||
| 231 | + | require.NoError(t, os.Symlink(outsideDir, filepath.Join(cacheDir, "nested"))) | |||
| 232 | + | ||||
| 233 | + | server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 234 | + | switch r.URL.Path { | |||
| 235 | + | case "/" + FilterFiles.Manifest: | |||
| 236 | + | _, _ = io.WriteString(w, `{"version":"test","combined":{"total_rules":1,"files":["nested/combined.json"]}}`) | |||
| 237 | + | case "/nested/combined.json": | |||
| 238 | + | _, _ = io.WriteString(w, `[{"trigger":{"url-filter":"test"},"action":{"type":"block"}}]`) | |||
| 239 | + | default: | |||
| 240 | + | http.NotFound(w, r) | |||
| 241 | + | } | |||
| 242 | + | })) | |||
| 243 | + | defer server.Close() | |||
| 244 | + | ||||
| 245 | + | d := NewDownloader(cacheDir) | |||
| 246 | + | d.baseURL = server.URL | |||
| 247 | + | ||||
| 248 | + | paths, err := d.DownloadFilters(context.Background(), nil) | |||
| 249 | + | require.Error(t, err) | |||
| 250 | + | require.Nil(t, paths) | |||
| 251 | + | require.Contains(t, err.Error(), "failed to create target dir") | |||
| 252 | + | require.NoFileExists(t, filepath.Join(outsideDir, "combined.json")) | |||
| 253 | + | } | |||
| 254 | + | ||||
| 255 | + | func TestDownloader_GetCachedFilterPaths_RejectsSymlinkEscape(t *testing.T) { | |||
| 256 | + | cacheDir := filepath.Join(t.TempDir(), "cache") | |||
| 257 | + | outsideDir := filepath.Join(t.TempDir(), "outside") | |||
| 258 | + | require.NoError(t, os.MkdirAll(cacheDir, cacheDirPerm)) | |||
| 259 | + | require.NoError(t, os.MkdirAll(outsideDir, cacheDirPerm)) | |||
| 260 | + | require.NoError(t, os.Symlink(outsideDir, filepath.Join(cacheDir, "nested"))) | |||
| 261 | + | require.NoError(t, os.WriteFile(filepath.Join(outsideDir, "combined.json"), []byte("[]"), manifestFilePerm)) | |||
| 262 | + | require.NoError(t, os.WriteFile( | |||
| 263 | + | filepath.Join(cacheDir, FilterFiles.Manifest), | |||
| 264 | + | []byte(`{"version":"test","combined":{"total_rules":1,"files":["nested/combined.json"]}}`), | |||
| 265 | + | manifestFilePerm, | |||
| 266 | + | )) | |||
| 267 | + | ||||
| 268 | + | d := NewDownloader(cacheDir) | |||
| 269 | + | require.Nil(t, d.GetCachedFilterPaths()) | |||
| 270 | + | } | |||
| 271 | + | ||||
| 67 | func TestDownloader_RenameTempFile_RequiresDirectoryParent(t *testing.T) { | 272 | func TestDownloader_RenameTempFile_RequiresDirectoryParent(t *testing.T) { | ||
| 68 | t.Parallel() | 273 | t.Parallel() | ||
| 69 | 274 | ||||
| @@ -78,7 +283,7 @@ func TestDownloader_RenameTempFile_RequiresDirectoryParent(t *testing.T) { | |||||
| 78 | 283 | ||||
| 79 | d := NewDownloader(cacheDir) | 284 | d := NewDownloader(cacheDir) | ||
| 80 | 285 | ||||
| 81 | - | err := d.renameTempFile(tmpFile, filepath.Join(parentAsFile, "filter.json")) | 286 | + | err := d.renameTempFile(tmpFile, "target/filter.json") |
| 82 | require.Error(t, err) | 287 | require.Error(t, err) | ||
| 83 | - | require.Contains(t, err.Error(), "not a directory") | 288 | + | require.Contains(t, err.Error(), "failed to create target dir") |
| 84 | } | 289 | } | ||
M internal/infrastructure/webkit/messaging.go +28 -7
| @@ -198,16 +198,26 @@ func (r *MessageRouter) handleScriptMessage(senderUCM webkit.UserContentManager, | |||
| 198 | 198 | log.Warn().Err(err).Str("json", rawJSON).Msg("failed to unmarshal script message") | |
| 199 | 199 | return | |
| 200 | 200 | } | |
| 201 | - | if msg.WebViewID == 0 && msg.WebViewIDAlt != 0 { | |
| 202 | - | msg.WebViewID = msg.WebViewIDAlt | |
| 201 | + | ||
| 202 | + | senderWV := lookupSenderWebView(senderUCM) | |
| 203 | + | if senderWV == nil { | |
| 204 | + | log.Warn().Msg("rejecting script message from unknown sender") | |
| 205 | + | return | |
| 203 | 206 | } | |
| 204 | - | if msg.WebViewID == 0 { | |
| 205 | - | if senderWV := lookupSenderWebView(senderUCM); senderWV != nil { | |
| 206 | - | msg.WebViewID = uint64(senderWV.ID()) | |
| 207 | - | r.syncWebViewID(senderWV) | |
| 208 | - | } | |
| 207 | + | if !isTrustedBridgeWebView(senderWV) { | |
| 208 | + | log.Warn(). | |
| 209 | + | Str("type", msg.Type). | |
| 210 | + | Uint64("sender_webview_id", uint64(senderWV.ID())). | |
| 211 | + | Str("uri", senderWV.URI()). | |
| 212 | + | Msg("rejecting script message from untrusted page") | |
| 213 | + | return | |
| 209 | 214 | } | |
| 210 | 215 | ||
| 216 | + | // Trust the sender WebView resolved from WebKit's UserContentManager, not a | |
| 217 | + | // caller-provided webViewId field in page-controlled JSON. | |
| 218 | + | msg.WebViewID = uint64(senderWV.ID()) | |
| 219 | + | r.syncWebViewID(senderWV) | |
| 220 | + | ||
| 211 | 221 | if msg.Type == "" { | |
| 212 | 222 | log.Warn().Msg("script message missing type") | |
| 213 | 223 | return | |
| @@ -262,6 +272,17 @@ func lookupSenderWebView(senderUCM webkit.UserContentManager) *WebView { | |||
| 262 | 272 | return LookupWebViewByUCMPointer(ptr) | |
| 263 | 273 | } | |
| 264 | 274 | ||
| 275 | + | func isTrustedBridgeWebView(wv *WebView) bool { | |
| 276 | + | if wv == nil { | |
| 277 | + | return false | |
| 278 | + | } | |
| 279 | + | return isTrustedBridgeURI(wv.URI()) | |
| 280 | + | } | |
| 281 | + | ||
| 282 | + | func isTrustedBridgeURI(raw string) bool { | |
| 283 | + | return isTrustedSystemviewURL(raw) | |
| 284 | + | } | |
| 285 | + | ||
| 265 | 286 | func (r *MessageRouter) syncWebViewID(wv *WebView) { | |
| 266 | 287 | if wv == nil { | |
| 267 | 288 | return | |
| @@ -198,16 +198,26 @@ func (r *MessageRouter) handleScriptMessage(senderUCM webkit.UserContentManager, | |||||
| 198 | log.Warn().Err(err).Str("json", rawJSON).Msg("failed to unmarshal script message") | 198 | log.Warn().Err(err).Str("json", rawJSON).Msg("failed to unmarshal script message") | ||
| 199 | return | 199 | return | ||
| 200 | } | 200 | } | ||
| 201 | - | if msg.WebViewID == 0 && msg.WebViewIDAlt != 0 { | 201 | + | |
| 202 | - | msg.WebViewID = msg.WebViewIDAlt | 202 | + | senderWV := lookupSenderWebView(senderUCM) |
| 203 | + | if senderWV == nil { | |||
| 204 | + | log.Warn().Msg("rejecting script message from unknown sender") | |||
| 205 | + | return | |||
| 203 | } | 206 | } | ||
| 204 | - | if msg.WebViewID == 0 { | 207 | + | if !isTrustedBridgeWebView(senderWV) { |
| 205 | - | if senderWV := lookupSenderWebView(senderUCM); senderWV != nil { | 208 | + | log.Warn(). |
| 206 | - | msg.WebViewID = uint64(senderWV.ID()) | 209 | + | Str("type", msg.Type). |
| 207 | - | r.syncWebViewID(senderWV) | 210 | + | Uint64("sender_webview_id", uint64(senderWV.ID())). |
| 208 | - | } | 211 | + | Str("uri", senderWV.URI()). |
| 212 | + | Msg("rejecting script message from untrusted page") | |||
| 213 | + | return | |||
| 209 | } | 214 | } | ||
| 210 | 215 | ||||
| 216 | + | // Trust the sender WebView resolved from WebKit's UserContentManager, not a | |||
| 217 | + | // caller-provided webViewId field in page-controlled JSON. | |||
| 218 | + | msg.WebViewID = uint64(senderWV.ID()) | |||
| 219 | + | r.syncWebViewID(senderWV) | |||
| 220 | + | ||||
| 211 | if msg.Type == "" { | 221 | if msg.Type == "" { | ||
| 212 | log.Warn().Msg("script message missing type") | 222 | log.Warn().Msg("script message missing type") | ||
| 213 | return | 223 | return | ||
| @@ -262,6 +272,17 @@ func lookupSenderWebView(senderUCM webkit.UserContentManager) *WebView { | |||||
| 262 | return LookupWebViewByUCMPointer(ptr) | 272 | return LookupWebViewByUCMPointer(ptr) | ||
| 263 | } | 273 | } | ||
| 264 | 274 | ||||
| 275 | + | func isTrustedBridgeWebView(wv *WebView) bool { | |||
| 276 | + | if wv == nil { | |||
| 277 | + | return false | |||
| 278 | + | } | |||
| 279 | + | return isTrustedBridgeURI(wv.URI()) | |||
| 280 | + | } | |||
| 281 | + | ||||
| 282 | + | func isTrustedBridgeURI(raw string) bool { | |||
| 283 | + | return isTrustedSystemviewURL(raw) | |||
| 284 | + | } | |||
| 285 | + | ||||
| 265 | func (r *MessageRouter) syncWebViewID(wv *WebView) { | 286 | func (r *MessageRouter) syncWebViewID(wv *WebView) { | ||
| 266 | if wv == nil { | 287 | if wv == nil { | ||
| 267 | return | 288 | return | ||
A internal/infrastructure/webkit/messaging_test.go +42 -0
| @@ -0,0 +1,42 @@ | |||
| 1 | + | package webkit | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/stretchr/testify/require" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | func TestIsTrustedBridgeURI(t *testing.T) { | |
| 10 | + | t.Parallel() | |
| 11 | + | ||
| 12 | + | trusted := []string{ | |
| 13 | + | "dumb://history", | |
| 14 | + | "dumb://favorites", | |
| 15 | + | "dumb://config", | |
| 16 | + | "dumb://error", | |
| 17 | + | "dumb://crash", | |
| 18 | + | "dumb://history/path?cursor=1", | |
| 19 | + | } | |
| 20 | + | for _, raw := range trusted { | |
| 21 | + | t.Run(raw, func(t *testing.T) { | |
| 22 | + | t.Parallel() | |
| 23 | + | require.True(t, isTrustedBridgeURI(raw)) | |
| 24 | + | }) | |
| 25 | + | } | |
| 26 | + | ||
| 27 | + | untrusted := []string{ | |
| 28 | + | "", | |
| 29 | + | "https://example.com", | |
| 30 | + | "http://localhost", | |
| 31 | + | "file:///tmp/history.html", | |
| 32 | + | "dumb://evil/history", | |
| 33 | + | "dumb://example.com", | |
| 34 | + | "javascript:alert(1)", | |
| 35 | + | } | |
| 36 | + | for _, raw := range untrusted { | |
| 37 | + | t.Run(raw, func(t *testing.T) { | |
| 38 | + | t.Parallel() | |
| 39 | + | require.False(t, isTrustedBridgeURI(raw)) | |
| 40 | + | }) | |
| 41 | + | } | |
| 42 | + | } | |
| @@ -0,0 +1,42 @@ | |||||
| 1 | + | package webkit | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/stretchr/testify/require" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | func TestIsTrustedBridgeURI(t *testing.T) { | |||
| 10 | + | t.Parallel() | |||
| 11 | + | ||||
| 12 | + | trusted := []string{ | |||
| 13 | + | "dumb://history", | |||
| 14 | + | "dumb://favorites", | |||
| 15 | + | "dumb://config", | |||
| 16 | + | "dumb://error", | |||
| 17 | + | "dumb://crash", | |||
| 18 | + | "dumb://history/path?cursor=1", | |||
| 19 | + | } | |||
| 20 | + | for _, raw := range trusted { | |||
| 21 | + | t.Run(raw, func(t *testing.T) { | |||
| 22 | + | t.Parallel() | |||
| 23 | + | require.True(t, isTrustedBridgeURI(raw)) | |||
| 24 | + | }) | |||
| 25 | + | } | |||
| 26 | + | ||||
| 27 | + | untrusted := []string{ | |||
| 28 | + | "", | |||
| 29 | + | "https://example.com", | |||
| 30 | + | "http://localhost", | |||
| 31 | + | "file:///tmp/history.html", | |||
| 32 | + | "dumb://evil/history", | |||
| 33 | + | "dumb://example.com", | |||
| 34 | + | "javascript:alert(1)", | |||
| 35 | + | } | |||
| 36 | + | for _, raw := range untrusted { | |||
| 37 | + | t.Run(raw, func(t *testing.T) { | |||
| 38 | + | t.Parallel() | |||
| 39 | + | require.False(t, isTrustedBridgeURI(raw)) | |||
| 40 | + | }) | |||
| 41 | + | } | |||
| 42 | + | } | |||
M internal/ui/app.go +11 -3
| @@ -2808,7 +2808,11 @@ func (a *App) cleanupCreatedBrowserWindows(createdWindows []*browserWindow) { | |||
| 2808 | 2808 | continue | |
| 2809 | 2809 | } | |
| 2810 | 2810 | mainWindow := bw.mainWindow | |
| 2811 | - | a.removeBrowserWindow(bw.id) | |
| 2811 | + | if _, tracked := a.browserWindows[bw.id]; tracked { | |
| 2812 | + | a.removeBrowserWindow(bw.id) | |
| 2813 | + | } else { | |
| 2814 | + | a.cleanupTransientBrowserWindowForDestroy(bw) | |
| 2815 | + | } | |
| 2812 | 2816 | if mainWindow != nil { | |
| 2813 | 2817 | if a.mainWindow == mainWindow { | |
| 2814 | 2818 | a.mainWindow = nil | |
| @@ -2838,10 +2842,14 @@ func (a *App) pruneStaleBrowserWindows(runtimeWindows []*browserWindow) { | |||
| 2838 | 2842 | } | |
| 2839 | 2843 | for id, bw := range a.browserWindows { | |
| 2840 | 2844 | if !runtimeSet[id] { | |
| 2841 | - | if bw != nil && bw.mainWindow != nil { | |
| 2842 | - | bw.mainWindow.Destroy() | |
| 2845 | + | var mainWindowToDestroy *window.MainWindow | |
| 2846 | + | if bw != nil { | |
| 2847 | + | mainWindowToDestroy = bw.mainWindow | |
| 2843 | 2848 | } | |
| 2844 | 2849 | a.removeBrowserWindow(id) | |
| 2850 | + | if mainWindowToDestroy != nil { | |
| 2851 | + | mainWindowToDestroy.Destroy() | |
| 2852 | + | } | |
| 2845 | 2853 | } | |
| 2846 | 2854 | } | |
| 2847 | 2855 | ||
| @@ -2808,7 +2808,11 @@ func (a *App) cleanupCreatedBrowserWindows(createdWindows []*browserWindow) { | |||||
| 2808 | continue | 2808 | continue | ||
| 2809 | } | 2809 | } | ||
| 2810 | mainWindow := bw.mainWindow | 2810 | mainWindow := bw.mainWindow | ||
| 2811 | - | a.removeBrowserWindow(bw.id) | 2811 | + | if _, tracked := a.browserWindows[bw.id]; tracked { |
| 2812 | + | a.removeBrowserWindow(bw.id) | |||
| 2813 | + | } else { | |||
| 2814 | + | a.cleanupTransientBrowserWindowForDestroy(bw) | |||
| 2815 | + | } | |||
| 2812 | if mainWindow != nil { | 2816 | if mainWindow != nil { | ||
| 2813 | if a.mainWindow == mainWindow { | 2817 | if a.mainWindow == mainWindow { | ||
| 2814 | a.mainWindow = nil | 2818 | a.mainWindow = nil | ||
| @@ -2838,10 +2842,14 @@ func (a *App) pruneStaleBrowserWindows(runtimeWindows []*browserWindow) { | |||||
| 2838 | } | 2842 | } | ||
| 2839 | for id, bw := range a.browserWindows { | 2843 | for id, bw := range a.browserWindows { | ||
| 2840 | if !runtimeSet[id] { | 2844 | if !runtimeSet[id] { | ||
| 2841 | - | if bw != nil && bw.mainWindow != nil { | 2845 | + | var mainWindowToDestroy *window.MainWindow |
| 2842 | - | bw.mainWindow.Destroy() | 2846 | + | if bw != nil { |
| 2847 | + | mainWindowToDestroy = bw.mainWindow | |||
| 2843 | } | 2848 | } | ||
| 2844 | a.removeBrowserWindow(id) | 2849 | a.removeBrowserWindow(id) | ||
| 2850 | + | if mainWindowToDestroy != nil { | |||
| 2851 | + | mainWindowToDestroy.Destroy() | |||
| 2852 | + | } | |||
| 2845 | } | 2853 | } | ||
| 2846 | } | 2854 | } | ||
| 2847 | 2855 | ||||
M internal/ui/browser_window.go +44 -5
| @@ -39,18 +39,26 @@ type browserWindow struct { | |||
| 39 | 39 | webrtcIndicator *component.WebRTCPermissionIndicator | |
| 40 | 40 | } | |
| 41 | 41 | ||
| 42 | - | func (bw *browserWindow) detachInput() { | |
| 42 | + | func (bw *browserWindow) detachInputForDestroy() { | |
| 43 | 43 | if bw == nil { | |
| 44 | 44 | return | |
| 45 | 45 | } | |
| 46 | 46 | if bw.keyboardHandler != nil { | |
| 47 | - | bw.keyboardHandler.Detach() | |
| 47 | + | bw.keyboardHandler.DetachForDestroy() | |
| 48 | 48 | } | |
| 49 | 49 | if bw.globalShortcutHandler != nil { | |
| 50 | - | bw.globalShortcutHandler.Detach() | |
| 50 | + | bw.globalShortcutHandler.DetachForDestroy() | |
| 51 | 51 | } | |
| 52 | 52 | } | |
| 53 | 53 | ||
| 54 | + | func (bw *browserWindow) teardownForDestroy() { | |
| 55 | + | if bw == nil { | |
| 56 | + | return | |
| 57 | + | } | |
| 58 | + | bw.detachInputForDestroy() | |
| 59 | + | bw.clearShellState() | |
| 60 | + | } | |
| 61 | + | ||
| 54 | 62 | func (bw *browserWindow) clearShellState() { | |
| 55 | 63 | if bw == nil { | |
| 56 | 64 | return | |
| @@ -299,8 +307,7 @@ func (a *App) removeBrowserWindow(id string) { | |||
| 299 | 307 | } | |
| 300 | 308 | } | |
| 301 | 309 | if removed != nil { | |
| 302 | - | removed.detachInput() | |
| 303 | - | removed.clearShellState() | |
| 310 | + | removed.teardownForDestroy() | |
| 304 | 311 | } | |
| 305 | 312 | if a.contentCoord != nil { | |
| 306 | 313 | a.contentCoord.ClearPopupNamedContextsForWindow(id) | |
| @@ -330,6 +337,38 @@ func (a *App) removeBrowserWindow(id string) { | |||
| 330 | 337 | } | |
| 331 | 338 | } | |
| 332 | 339 | ||
| 340 | + | func (a *App) cleanupTransientBrowserWindowForDestroy(bw *browserWindow) { | |
| 341 | + | if bw == nil { | |
| 342 | + | return | |
| 343 | + | } | |
| 344 | + | bw.teardownForDestroy() | |
| 345 | + | if a.contentCoord != nil { | |
| 346 | + | a.contentCoord.ClearPopupNamedContextsForWindow(bw.id) | |
| 347 | + | } | |
| 348 | + | fallback := a.deterministicBrowserWindowFallback() | |
| 349 | + | if a.lastFocusedWindowID == bw.id { | |
| 350 | + | if fallback != nil { | |
| 351 | + | a.lastFocusedWindowID = fallback.id | |
| 352 | + | } else { | |
| 353 | + | a.lastFocusedWindowID = "" | |
| 354 | + | } | |
| 355 | + | } | |
| 356 | + | if bw.mainWindow == a.mainWindow { | |
| 357 | + | a.clearResizeModeBorder() | |
| 358 | + | if fallback != nil { | |
| 359 | + | a.activateBrowserWindow(fallback) | |
| 360 | + | return | |
| 361 | + | } | |
| 362 | + | a.lastFocusedWindowID = "" | |
| 363 | + | a.mainWindow = nil | |
| 364 | + | a.keyboardHandler = nil | |
| 365 | + | a.globalShortcutHandler = nil | |
| 366 | + | if a.tabCoord != nil { | |
| 367 | + | a.tabCoord.SetCurrentTarget(coordinator.TabTarget{}) | |
| 368 | + | } | |
| 369 | + | } | |
| 370 | + | } | |
| 371 | + | ||
| 333 | 372 | func (a *App) deterministicBrowserWindowFallback() *browserWindow { | |
| 334 | 373 | if a.browserWindows == nil { | |
| 335 | 374 | return nil | |
| @@ -39,18 +39,26 @@ type browserWindow struct { | |||||
| 39 | webrtcIndicator *component.WebRTCPermissionIndicator | 39 | webrtcIndicator *component.WebRTCPermissionIndicator | ||
| 40 | } | 40 | } | ||
| 41 | 41 | ||||
| 42 | - | func (bw *browserWindow) detachInput() { | 42 | + | func (bw *browserWindow) detachInputForDestroy() { |
| 43 | if bw == nil { | 43 | if bw == nil { | ||
| 44 | return | 44 | return | ||
| 45 | } | 45 | } | ||
| 46 | if bw.keyboardHandler != nil { | 46 | if bw.keyboardHandler != nil { | ||
| 47 | - | bw.keyboardHandler.Detach() | 47 | + | bw.keyboardHandler.DetachForDestroy() |
| 48 | } | 48 | } | ||
| 49 | if bw.globalShortcutHandler != nil { | 49 | if bw.globalShortcutHandler != nil { | ||
| 50 | - | bw.globalShortcutHandler.Detach() | 50 | + | bw.globalShortcutHandler.DetachForDestroy() |
| 51 | } | 51 | } | ||
| 52 | } | 52 | } | ||
| 53 | 53 | ||||
| 54 | + | func (bw *browserWindow) teardownForDestroy() { | |||
| 55 | + | if bw == nil { | |||
| 56 | + | return | |||
| 57 | + | } | |||
| 58 | + | bw.detachInputForDestroy() | |||
| 59 | + | bw.clearShellState() | |||
| 60 | + | } | |||
| 61 | + | ||||
| 54 | func (bw *browserWindow) clearShellState() { | 62 | func (bw *browserWindow) clearShellState() { | ||
| 55 | if bw == nil { | 63 | if bw == nil { | ||
| 56 | return | 64 | return | ||
| @@ -299,8 +307,7 @@ func (a *App) removeBrowserWindow(id string) { | |||||
| 299 | } | 307 | } | ||
| 300 | } | 308 | } | ||
| 301 | if removed != nil { | 309 | if removed != nil { | ||
| 302 | - | removed.detachInput() | 310 | + | removed.teardownForDestroy() |
| 303 | - | removed.clearShellState() | |||
| 304 | } | 311 | } | ||
| 305 | if a.contentCoord != nil { | 312 | if a.contentCoord != nil { | ||
| 306 | a.contentCoord.ClearPopupNamedContextsForWindow(id) | 313 | a.contentCoord.ClearPopupNamedContextsForWindow(id) | ||
| @@ -330,6 +337,38 @@ func (a *App) removeBrowserWindow(id string) { | |||||
| 330 | } | 337 | } | ||
| 331 | } | 338 | } | ||
| 332 | 339 | ||||
| 340 | + | func (a *App) cleanupTransientBrowserWindowForDestroy(bw *browserWindow) { | |||
| 341 | + | if bw == nil { | |||
| 342 | + | return | |||
| 343 | + | } | |||
| 344 | + | bw.teardownForDestroy() | |||
| 345 | + | if a.contentCoord != nil { | |||
| 346 | + | a.contentCoord.ClearPopupNamedContextsForWindow(bw.id) | |||
| 347 | + | } | |||
| 348 | + | fallback := a.deterministicBrowserWindowFallback() | |||
| 349 | + | if a.lastFocusedWindowID == bw.id { | |||
| 350 | + | if fallback != nil { | |||
| 351 | + | a.lastFocusedWindowID = fallback.id | |||
| 352 | + | } else { | |||
| 353 | + | a.lastFocusedWindowID = "" | |||
| 354 | + | } | |||
| 355 | + | } | |||
| 356 | + | if bw.mainWindow == a.mainWindow { | |||
| 357 | + | a.clearResizeModeBorder() | |||
| 358 | + | if fallback != nil { | |||
| 359 | + | a.activateBrowserWindow(fallback) | |||
| 360 | + | return | |||
| 361 | + | } | |||
| 362 | + | a.lastFocusedWindowID = "" | |||
| 363 | + | a.mainWindow = nil | |||
| 364 | + | a.keyboardHandler = nil | |||
| 365 | + | a.globalShortcutHandler = nil | |||
| 366 | + | if a.tabCoord != nil { | |||
| 367 | + | a.tabCoord.SetCurrentTarget(coordinator.TabTarget{}) | |||
| 368 | + | } | |||
| 369 | + | } | |||
| 370 | + | } | |||
| 371 | + | ||||
| 333 | func (a *App) deterministicBrowserWindowFallback() *browserWindow { | 372 | func (a *App) deterministicBrowserWindowFallback() *browserWindow { | ||
| 334 | if a.browserWindows == nil { | 373 | if a.browserWindows == nil { | ||
| 335 | return nil | 374 | return nil | ||
M internal/ui/browser_window_test.go +34 -0
| @@ -67,6 +67,40 @@ func TestBrowserWindow_RemoveBrowserWindowClearsShellState(t *testing.T) { | |||
| 67 | 67 | } | |
| 68 | 68 | } | |
| 69 | 69 | ||
| 70 | + | func TestApp_CleanupCreatedBrowserWindowsDetachesUnregisteredWindowBeforeDestroy(t *testing.T) { | |
| 71 | + | transientMainWindow := &window.MainWindow{} | |
| 72 | + | fallbackMainWindow := &window.MainWindow{} | |
| 73 | + | transient := &browserWindow{id: "window-transient", mainWindow: transientMainWindow} | |
| 74 | + | fallback := &browserWindow{id: "window-fallback", mainWindow: fallbackMainWindow} | |
| 75 | + | app := &App{ | |
| 76 | + | mainWindow: transientMainWindow, | |
| 77 | + | browserWindows: map[string]*browserWindow{fallback.id: fallback}, | |
| 78 | + | browserWindowOrder: []string{fallback.id}, | |
| 79 | + | lastFocusedWindowID: transient.id, | |
| 80 | + | } | |
| 81 | + | ||
| 82 | + | setShellField(t, transient, "keyboardHandler", &input.KeyboardHandler{}) | |
| 83 | + | setShellField(t, transient, "globalShortcutHandler", &input.GlobalShortcutHandler{}) | |
| 84 | + | setShellField(t, transient, "accentPicker", &component.AccentPicker{}) | |
| 85 | + | ||
| 86 | + | app.cleanupCreatedBrowserWindows([]*browserWindow{transient}) | |
| 87 | + | ||
| 88 | + | for _, name := range []string{"keyboardHandler", "globalShortcutHandler", "accentPicker"} { | |
| 89 | + | if !fieldIsZero(t, transient, name) { | |
| 90 | + | t.Fatalf("transient browserWindow.%s was not cleared", name) | |
| 91 | + | } | |
| 92 | + | } | |
| 93 | + | if app.mainWindow != fallbackMainWindow { | |
| 94 | + | t.Fatalf("mainWindow = %p, want fallback main window %p", app.mainWindow, fallbackMainWindow) | |
| 95 | + | } | |
| 96 | + | if app.lastFocusedWindowID != fallback.id { | |
| 97 | + | t.Fatalf("lastFocusedWindowID = %q, want %q", app.lastFocusedWindowID, fallback.id) | |
| 98 | + | } | |
| 99 | + | if got := app.browserWindows[fallback.id]; got != fallback { | |
| 100 | + | t.Fatalf("fallback browser window changed during transient cleanup: got %p want %p", got, fallback) | |
| 101 | + | } | |
| 102 | + | } | |
| 103 | + | ||
| 70 | 104 | func TestBrowserWindow_RemoveBrowserWindowCleansOwnedTabState(t *testing.T) { | |
| 71 | 105 | ownedTab := entity.NewTab(entity.TabID("tab-owned"), entity.WorkspaceID("workspace-owned"), entity.NewPane(entity.PaneID("pane-owned"))) | |
| 72 | 106 | otherTab := entity.NewTab(entity.TabID("tab-other"), entity.WorkspaceID("workspace-other"), entity.NewPane(entity.PaneID("pane-other"))) | |
| @@ -67,6 +67,40 @@ func TestBrowserWindow_RemoveBrowserWindowClearsShellState(t *testing.T) { | |||||
| 67 | } | 67 | } | ||
| 68 | } | 68 | } | ||
| 69 | 69 | ||||
| 70 | + | func TestApp_CleanupCreatedBrowserWindowsDetachesUnregisteredWindowBeforeDestroy(t *testing.T) { | |||
| 71 | + | transientMainWindow := &window.MainWindow{} | |||
| 72 | + | fallbackMainWindow := &window.MainWindow{} | |||
| 73 | + | transient := &browserWindow{id: "window-transient", mainWindow: transientMainWindow} | |||
| 74 | + | fallback := &browserWindow{id: "window-fallback", mainWindow: fallbackMainWindow} | |||
| 75 | + | app := &App{ | |||
| 76 | + | mainWindow: transientMainWindow, | |||
| 77 | + | browserWindows: map[string]*browserWindow{fallback.id: fallback}, | |||
| 78 | + | browserWindowOrder: []string{fallback.id}, | |||
| 79 | + | lastFocusedWindowID: transient.id, | |||
| 80 | + | } | |||
| 81 | + | ||||
| 82 | + | setShellField(t, transient, "keyboardHandler", &input.KeyboardHandler{}) | |||
| 83 | + | setShellField(t, transient, "globalShortcutHandler", &input.GlobalShortcutHandler{}) | |||
| 84 | + | setShellField(t, transient, "accentPicker", &component.AccentPicker{}) | |||
| 85 | + | ||||
| 86 | + | app.cleanupCreatedBrowserWindows([]*browserWindow{transient}) | |||
| 87 | + | ||||
| 88 | + | for _, name := range []string{"keyboardHandler", "globalShortcutHandler", "accentPicker"} { | |||
| 89 | + | if !fieldIsZero(t, transient, name) { | |||
| 90 | + | t.Fatalf("transient browserWindow.%s was not cleared", name) | |||
| 91 | + | } | |||
| 92 | + | } | |||
| 93 | + | if app.mainWindow != fallbackMainWindow { | |||
| 94 | + | t.Fatalf("mainWindow = %p, want fallback main window %p", app.mainWindow, fallbackMainWindow) | |||
| 95 | + | } | |||
| 96 | + | if app.lastFocusedWindowID != fallback.id { | |||
| 97 | + | t.Fatalf("lastFocusedWindowID = %q, want %q", app.lastFocusedWindowID, fallback.id) | |||
| 98 | + | } | |||
| 99 | + | if got := app.browserWindows[fallback.id]; got != fallback { | |||
| 100 | + | t.Fatalf("fallback browser window changed during transient cleanup: got %p want %p", got, fallback) | |||
| 101 | + | } | |||
| 102 | + | } | |||
| 103 | + | ||||
| 70 | func TestBrowserWindow_RemoveBrowserWindowCleansOwnedTabState(t *testing.T) { | 104 | func TestBrowserWindow_RemoveBrowserWindowCleansOwnedTabState(t *testing.T) { | ||
| 71 | ownedTab := entity.NewTab(entity.TabID("tab-owned"), entity.WorkspaceID("workspace-owned"), entity.NewPane(entity.PaneID("pane-owned"))) | 105 | ownedTab := entity.NewTab(entity.TabID("tab-owned"), entity.WorkspaceID("workspace-owned"), entity.NewPane(entity.PaneID("pane-owned"))) | ||
| 72 | otherTab := entity.NewTab(entity.TabID("tab-other"), entity.WorkspaceID("workspace-other"), entity.NewPane(entity.PaneID("pane-other"))) | 106 | otherTab := entity.NewTab(entity.TabID("tab-other"), entity.WorkspaceID("workspace-other"), entity.NewPane(entity.PaneID("pane-other"))) | ||
M internal/ui/input/gesture.go +14 -3
| @@ -104,7 +104,7 @@ func (h *GestureHandler) AttachTo(widget *gtk.Widget) { | |||
| 104 | 104 | h.pressedHandlerID = h.clickGesture.ConnectPressed(&h.pressedCb) | |
| 105 | 105 | h.widget = widget | |
| 106 | 106 | h.destroyCb = func(_ gtk.Widget) { | |
| 107 | - | h.Detach() | |
| 107 | + | h.DetachForDestroy() | |
| 108 | 108 | } | |
| 109 | 109 | h.destroyHandlerID = widget.ConnectDestroy(&h.destroyCb) | |
| 110 | 110 | ||
| @@ -182,8 +182,19 @@ func (h *GestureHandler) detachExistingAttachment() { | |||
| 182 | 182 | } | |
| 183 | 183 | } | |
| 184 | 184 | ||
| 185 | - | // Detach removes the gesture handler and disconnects its GTK signals. | |
| 185 | + | // Detach removes the gesture handler from a live widget and disconnects its | |
| 186 | + | // GTK signals. | |
| 186 | 187 | func (h *GestureHandler) Detach() { | |
| 188 | + | h.detach(true) | |
| 189 | + | } | |
| 190 | + | ||
| 191 | + | // DetachForDestroy disconnects retained GTK callbacks during widget teardown | |
| 192 | + | // without mutating the widget's controller list while GTK is destroying it. | |
| 193 | + | func (h *GestureHandler) DetachForDestroy() { | |
| 194 | + | h.detach(false) | |
| 195 | + | } | |
| 196 | + | ||
| 197 | + | func (h *GestureHandler) detach(removeController bool) { | |
| 187 | 198 | if h == nil { | |
| 188 | 199 | return | |
| 189 | 200 | } | |
| @@ -209,7 +220,7 @@ func (h *GestureHandler) Detach() { | |||
| 209 | 220 | if widget != nil && destroyHandlerID != 0 { | |
| 210 | 221 | widget.DisconnectSignal(destroyHandlerID) | |
| 211 | 222 | } | |
| 212 | - | if widget != nil && clickGesture != nil { | |
| 223 | + | if removeController && widget != nil && clickGesture != nil { | |
| 213 | 224 | widget.RemoveController(&clickGesture.EventController) | |
| 214 | 225 | } | |
| 215 | 226 | } | |
| @@ -104,7 +104,7 @@ func (h *GestureHandler) AttachTo(widget *gtk.Widget) { | |||||
| 104 | h.pressedHandlerID = h.clickGesture.ConnectPressed(&h.pressedCb) | 104 | h.pressedHandlerID = h.clickGesture.ConnectPressed(&h.pressedCb) | ||
| 105 | h.widget = widget | 105 | h.widget = widget | ||
| 106 | h.destroyCb = func(_ gtk.Widget) { | 106 | h.destroyCb = func(_ gtk.Widget) { | ||
| 107 | - | h.Detach() | 107 | + | h.DetachForDestroy() |
| 108 | } | 108 | } | ||
| 109 | h.destroyHandlerID = widget.ConnectDestroy(&h.destroyCb) | 109 | h.destroyHandlerID = widget.ConnectDestroy(&h.destroyCb) | ||
| 110 | 110 | ||||
| @@ -182,8 +182,19 @@ func (h *GestureHandler) detachExistingAttachment() { | |||||
| 182 | } | 182 | } | ||
| 183 | } | 183 | } | ||
| 184 | 184 | ||||
| 185 | - | // Detach removes the gesture handler and disconnects its GTK signals. | 185 | + | // Detach removes the gesture handler from a live widget and disconnects its |
| 186 | + | // GTK signals. | |||
| 186 | func (h *GestureHandler) Detach() { | 187 | func (h *GestureHandler) Detach() { | ||
| 188 | + | h.detach(true) | |||
| 189 | + | } | |||
| 190 | + | ||||
| 191 | + | // DetachForDestroy disconnects retained GTK callbacks during widget teardown | |||
| 192 | + | // without mutating the widget's controller list while GTK is destroying it. | |||
| 193 | + | func (h *GestureHandler) DetachForDestroy() { | |||
| 194 | + | h.detach(false) | |||
| 195 | + | } | |||
| 196 | + | ||||
| 197 | + | func (h *GestureHandler) detach(removeController bool) { | |||
| 187 | if h == nil { | 198 | if h == nil { | ||
| 188 | return | 199 | return | ||
| 189 | } | 200 | } | ||
| @@ -209,7 +220,7 @@ func (h *GestureHandler) Detach() { | |||||
| 209 | if widget != nil && destroyHandlerID != 0 { | 220 | if widget != nil && destroyHandlerID != 0 { | ||
| 210 | widget.DisconnectSignal(destroyHandlerID) | 221 | widget.DisconnectSignal(destroyHandlerID) | ||
| 211 | } | 222 | } | ||
| 212 | - | if widget != nil && clickGesture != nil { | 223 | + | if removeController && widget != nil && clickGesture != nil { |
| 213 | widget.RemoveController(&clickGesture.EventController) | 224 | widget.RemoveController(&clickGesture.EventController) | ||
| 214 | } | 225 | } | ||
| 215 | } | 226 | } | ||
M internal/ui/input/gesture_test.go +19 -0
| @@ -50,3 +50,22 @@ func TestGestureHandlerDetachClearsRetainedCallbacksAndState(t *testing.T) { | |||
| 50 | 50 | assert.Nil(t, h.onAction) | |
| 51 | 51 | assert.Nil(t, h.navigator) | |
| 52 | 52 | } | |
| 53 | + | ||
| 54 | + | func TestGestureHandlerDetachForDestroyClearsRetainedCallbacksAndState(t *testing.T) { | |
| 55 | + | h := NewGestureHandler(context.Background()) | |
| 56 | + | h.pressedCb = func(gtk.GestureClick, int, float64, float64) {} | |
| 57 | + | h.destroyCb = func(gtk.Widget) {} | |
| 58 | + | h.pressedHandlerID = 1 | |
| 59 | + | h.destroyHandlerID = 2 | |
| 60 | + | h.onAction = func(context.Context, Action) error { return nil } | |
| 61 | + | h.navigator = testGestureNavigator{} | |
| 62 | + | ||
| 63 | + | h.DetachForDestroy() | |
| 64 | + | ||
| 65 | + | assert.Nil(t, h.pressedCb) | |
| 66 | + | assert.Nil(t, h.destroyCb) | |
| 67 | + | assert.Zero(t, h.pressedHandlerID) | |
| 68 | + | assert.Zero(t, h.destroyHandlerID) | |
| 69 | + | assert.Nil(t, h.onAction) | |
| 70 | + | assert.Nil(t, h.navigator) | |
| 71 | + | } | |
| @@ -50,3 +50,22 @@ func TestGestureHandlerDetachClearsRetainedCallbacksAndState(t *testing.T) { | |||||
| 50 | assert.Nil(t, h.onAction) | 50 | assert.Nil(t, h.onAction) | ||
| 51 | assert.Nil(t, h.navigator) | 51 | assert.Nil(t, h.navigator) | ||
| 52 | } | 52 | } | ||
| 53 | + | ||||
| 54 | + | func TestGestureHandlerDetachForDestroyClearsRetainedCallbacksAndState(t *testing.T) { | |||
| 55 | + | h := NewGestureHandler(context.Background()) | |||
| 56 | + | h.pressedCb = func(gtk.GestureClick, int, float64, float64) {} | |||
| 57 | + | h.destroyCb = func(gtk.Widget) {} | |||
| 58 | + | h.pressedHandlerID = 1 | |||
| 59 | + | h.destroyHandlerID = 2 | |||
| 60 | + | h.onAction = func(context.Context, Action) error { return nil } | |||
| 61 | + | h.navigator = testGestureNavigator{} | |||
| 62 | + | ||||
| 63 | + | h.DetachForDestroy() | |||
| 64 | + | ||||
| 65 | + | assert.Nil(t, h.pressedCb) | |||
| 66 | + | assert.Nil(t, h.destroyCb) | |||
| 67 | + | assert.Zero(t, h.pressedHandlerID) | |||
| 68 | + | assert.Zero(t, h.destroyHandlerID) | |||
| 69 | + | assert.Nil(t, h.onAction) | |||
| 70 | + | assert.Nil(t, h.navigator) | |||
| 71 | + | } | |||
M internal/ui/input/global_shortcuts.go +16 -6
| @@ -579,24 +579,34 @@ func (h *GlobalShortcutHandler) ReloadShortcuts(ctx context.Context, workspace * | |||
| 579 | 579 | Msg("global shortcuts reloaded") | |
| 580 | 580 | } | |
| 581 | 581 | ||
| 582 | - | // Detach removes the global shortcut handler from the window. | |
| 583 | - | // Note: GTK handles cleanup when the widget is destroyed, | |
| 584 | - | // but we clear our references here. | |
| 582 | + | // Detach removes the global shortcut handler from a live GTK window. | |
| 583 | + | // Use DetachForDestroy when the window is already closing or being destroyed. | |
| 585 | 584 | func (h *GlobalShortcutHandler) Detach() { | |
| 585 | + | h.detach(true) | |
| 586 | + | } | |
| 587 | + | ||
| 588 | + | // DetachForDestroy releases retained GTK callbacks during window teardown | |
| 589 | + | // without mutating the window's controller or action lists while GTK is | |
| 590 | + | // destroying them. | |
| 591 | + | func (h *GlobalShortcutHandler) DetachForDestroy() { | |
| 592 | + | h.detach(false) | |
| 593 | + | } | |
| 594 | + | ||
| 595 | + | func (h *GlobalShortcutHandler) detach(removeFromWindow bool) { | |
| 586 | 596 | h.generation++ | |
| 587 | - | if h.window != nil && h.controller != nil { | |
| 597 | + | if removeFromWindow && h.window != nil && h.controller != nil { | |
| 588 | 598 | h.window.RemoveController(&h.controller.EventController) | |
| 589 | 599 | } | |
| 590 | 600 | if h.releaseController != nil && h.keyReleasedHandlerID != 0 { | |
| 591 | 601 | h.releaseController.DisconnectSignal(h.keyReleasedHandlerID) | |
| 592 | 602 | } | |
| 593 | - | if h.window != nil && h.releaseController != nil { | |
| 603 | + | if removeFromWindow && h.window != nil && h.releaseController != nil { | |
| 594 | 604 | h.window.RemoveController(&h.releaseController.EventController) | |
| 595 | 605 | } | |
| 596 | 606 | if h.globalAction != nil && h.globalActionHandlerID != 0 { | |
| 597 | 607 | h.globalAction.DisconnectSignal(h.globalActionHandlerID) | |
| 598 | 608 | } | |
| 599 | - | if h.window != nil && h.globalAction != nil { | |
| 609 | + | if removeFromWindow && h.window != nil && h.globalAction != nil { | |
| 600 | 610 | h.window.RemoveAction(globalShortcutActionName) | |
| 601 | 611 | } | |
| 602 | 612 | h.controller = nil | |
| @@ -579,24 +579,34 @@ func (h *GlobalShortcutHandler) ReloadShortcuts(ctx context.Context, workspace * | |||||
| 579 | Msg("global shortcuts reloaded") | 579 | Msg("global shortcuts reloaded") | ||
| 580 | } | 580 | } | ||
| 581 | 581 | ||||
| 582 | - | // Detach removes the global shortcut handler from the window. | 582 | + | // Detach removes the global shortcut handler from a live GTK window. |
| 583 | - | // Note: GTK handles cleanup when the widget is destroyed, | 583 | + | // Use DetachForDestroy when the window is already closing or being destroyed. |
| 584 | - | // but we clear our references here. | |||
| 585 | func (h *GlobalShortcutHandler) Detach() { | 584 | func (h *GlobalShortcutHandler) Detach() { | ||
| 585 | + | h.detach(true) | |||
| 586 | + | } | |||
| 587 | + | ||||
| 588 | + | // DetachForDestroy releases retained GTK callbacks during window teardown | |||
| 589 | + | // without mutating the window's controller or action lists while GTK is | |||
| 590 | + | // destroying them. | |||
| 591 | + | func (h *GlobalShortcutHandler) DetachForDestroy() { | |||
| 592 | + | h.detach(false) | |||
| 593 | + | } | |||
| 594 | + | ||||
| 595 | + | func (h *GlobalShortcutHandler) detach(removeFromWindow bool) { | |||
| 586 | h.generation++ | 596 | h.generation++ | ||
| 587 | - | if h.window != nil && h.controller != nil { | 597 | + | if removeFromWindow && h.window != nil && h.controller != nil { |
| 588 | h.window.RemoveController(&h.controller.EventController) | 598 | h.window.RemoveController(&h.controller.EventController) | ||
| 589 | } | 599 | } | ||
| 590 | if h.releaseController != nil && h.keyReleasedHandlerID != 0 { | 600 | if h.releaseController != nil && h.keyReleasedHandlerID != 0 { | ||
| 591 | h.releaseController.DisconnectSignal(h.keyReleasedHandlerID) | 601 | h.releaseController.DisconnectSignal(h.keyReleasedHandlerID) | ||
| 592 | } | 602 | } | ||
| 593 | - | if h.window != nil && h.releaseController != nil { | 603 | + | if removeFromWindow && h.window != nil && h.releaseController != nil { |
| 594 | h.window.RemoveController(&h.releaseController.EventController) | 604 | h.window.RemoveController(&h.releaseController.EventController) | ||
| 595 | } | 605 | } | ||
| 596 | if h.globalAction != nil && h.globalActionHandlerID != 0 { | 606 | if h.globalAction != nil && h.globalActionHandlerID != 0 { | ||
| 597 | h.globalAction.DisconnectSignal(h.globalActionHandlerID) | 607 | h.globalAction.DisconnectSignal(h.globalActionHandlerID) | ||
| 598 | } | 608 | } | ||
| 599 | - | if h.window != nil && h.globalAction != nil { | 609 | + | if removeFromWindow && h.window != nil && h.globalAction != nil { |
| 600 | h.window.RemoveAction(globalShortcutActionName) | 610 | h.window.RemoveAction(globalShortcutActionName) | ||
| 601 | } | 611 | } | ||
| 602 | h.controller = nil | 612 | h.controller = nil | ||
M internal/ui/input/global_shortcuts_test.go +52 -0
| @@ -133,6 +133,58 @@ func TestGlobalShortcutHandlerSuppressesOneShotActionsAfterDetach(t *testing.T) | |||
| 133 | 133 | } | |
| 134 | 134 | } | |
| 135 | 135 | ||
| 136 | + | func TestGlobalShortcutHandlerDetachForDestroyClearsRetainedState(t *testing.T) { | |
| 137 | + | h := &GlobalShortcutHandler{ | |
| 138 | + | registered: make(map[KeyBinding]Action), | |
| 139 | + | shortcutRefs: make(map[string]globalShortcutRegistration), | |
| 140 | + | lastDispatchAt: make(map[Action]time.Time), | |
| 141 | + | heldShortcuts: make(map[globalShortcutHoldKey]struct{}), | |
| 142 | + | globalActionCb: func(_ gio.SimpleAction, _ uintptr) {}, | |
| 143 | + | globalActionHandlerID: 1, | |
| 144 | + | keyReleasedCb: func(_ gtk.EventControllerKey, _ uint, _ uint, _ gdk.ModifierType) {}, | |
| 145 | + | keyReleasedHandlerID: 2, | |
| 146 | + | } | |
| 147 | + | ||
| 148 | + | h.DetachForDestroy() | |
| 149 | + | ||
| 150 | + | if h.controller != nil { | |
| 151 | + | t.Fatal("controller should be cleared during destroy detach") | |
| 152 | + | } | |
| 153 | + | if h.releaseController != nil { | |
| 154 | + | t.Fatal("releaseController should be cleared during destroy detach") | |
| 155 | + | } | |
| 156 | + | if h.shortcutAction != nil { | |
| 157 | + | t.Fatal("shortcutAction should be cleared during destroy detach") | |
| 158 | + | } | |
| 159 | + | if h.globalAction != nil { | |
| 160 | + | t.Fatal("globalAction should be cleared during destroy detach") | |
| 161 | + | } | |
| 162 | + | if h.globalActionCb != nil { | |
| 163 | + | t.Fatal("globalActionCb should be cleared during destroy detach") | |
| 164 | + | } | |
| 165 | + | if h.keyReleasedCb != nil { | |
| 166 | + | t.Fatal("keyReleasedCb should be cleared during destroy detach") | |
| 167 | + | } | |
| 168 | + | if h.globalActionHandlerID != 0 { | |
| 169 | + | t.Fatal("globalActionHandlerID should be reset during destroy detach") | |
| 170 | + | } | |
| 171 | + | if h.keyReleasedHandlerID != 0 { | |
| 172 | + | t.Fatal("keyReleasedHandlerID should be reset during destroy detach") | |
| 173 | + | } | |
| 174 | + | if h.registered != nil { | |
| 175 | + | t.Fatal("registered shortcuts should be cleared during destroy detach") | |
| 176 | + | } | |
| 177 | + | if h.shortcutRefs != nil { | |
| 178 | + | t.Fatal("shortcut refs should be cleared during destroy detach") | |
| 179 | + | } | |
| 180 | + | if h.lastDispatchAt != nil { | |
| 181 | + | t.Fatal("lastDispatchAt should be cleared during destroy detach") | |
| 182 | + | } | |
| 183 | + | if h.heldShortcuts != nil { | |
| 184 | + | t.Fatal("heldShortcuts should be cleared during destroy detach") | |
| 185 | + | } | |
| 186 | + | } | |
| 187 | + | ||
| 136 | 188 | func TestGlobalShortcutHandlerGenerationMarksOldCallbacksStale(t *testing.T) { | |
| 137 | 189 | h := &GlobalShortcutHandler{} | |
| 138 | 190 | generation := h.generation | |
| @@ -133,6 +133,58 @@ func TestGlobalShortcutHandlerSuppressesOneShotActionsAfterDetach(t *testing.T) | |||||
| 133 | } | 133 | } | ||
| 134 | } | 134 | } | ||
| 135 | 135 | ||||
| 136 | + | func TestGlobalShortcutHandlerDetachForDestroyClearsRetainedState(t *testing.T) { | |||
| 137 | + | h := &GlobalShortcutHandler{ | |||
| 138 | + | registered: make(map[KeyBinding]Action), | |||
| 139 | + | shortcutRefs: make(map[string]globalShortcutRegistration), | |||
| 140 | + | lastDispatchAt: make(map[Action]time.Time), | |||
| 141 | + | heldShortcuts: make(map[globalShortcutHoldKey]struct{}), | |||
| 142 | + | globalActionCb: func(_ gio.SimpleAction, _ uintptr) {}, | |||
| 143 | + | globalActionHandlerID: 1, | |||
| 144 | + | keyReleasedCb: func(_ gtk.EventControllerKey, _ uint, _ uint, _ gdk.ModifierType) {}, | |||
| 145 | + | keyReleasedHandlerID: 2, | |||
| 146 | + | } | |||
| 147 | + | ||||
| 148 | + | h.DetachForDestroy() | |||
| 149 | + | ||||
| 150 | + | if h.controller != nil { | |||
| 151 | + | t.Fatal("controller should be cleared during destroy detach") | |||
| 152 | + | } | |||
| 153 | + | if h.releaseController != nil { | |||
| 154 | + | t.Fatal("releaseController should be cleared during destroy detach") | |||
| 155 | + | } | |||
| 156 | + | if h.shortcutAction != nil { | |||
| 157 | + | t.Fatal("shortcutAction should be cleared during destroy detach") | |||
| 158 | + | } | |||
| 159 | + | if h.globalAction != nil { | |||
| 160 | + | t.Fatal("globalAction should be cleared during destroy detach") | |||
| 161 | + | } | |||
| 162 | + | if h.globalActionCb != nil { | |||
| 163 | + | t.Fatal("globalActionCb should be cleared during destroy detach") | |||
| 164 | + | } | |||
| 165 | + | if h.keyReleasedCb != nil { | |||
| 166 | + | t.Fatal("keyReleasedCb should be cleared during destroy detach") | |||
| 167 | + | } | |||
| 168 | + | if h.globalActionHandlerID != 0 { | |||
| 169 | + | t.Fatal("globalActionHandlerID should be reset during destroy detach") | |||
| 170 | + | } | |||
| 171 | + | if h.keyReleasedHandlerID != 0 { | |||
| 172 | + | t.Fatal("keyReleasedHandlerID should be reset during destroy detach") | |||
| 173 | + | } | |||
| 174 | + | if h.registered != nil { | |||
| 175 | + | t.Fatal("registered shortcuts should be cleared during destroy detach") | |||
| 176 | + | } | |||
| 177 | + | if h.shortcutRefs != nil { | |||
| 178 | + | t.Fatal("shortcut refs should be cleared during destroy detach") | |||
| 179 | + | } | |||
| 180 | + | if h.lastDispatchAt != nil { | |||
| 181 | + | t.Fatal("lastDispatchAt should be cleared during destroy detach") | |||
| 182 | + | } | |||
| 183 | + | if h.heldShortcuts != nil { | |||
| 184 | + | t.Fatal("heldShortcuts should be cleared during destroy detach") | |||
| 185 | + | } | |||
| 186 | + | } | |||
| 187 | + | ||||
| 136 | func TestGlobalShortcutHandlerGenerationMarksOldCallbacksStale(t *testing.T) { | 188 | func TestGlobalShortcutHandlerGenerationMarksOldCallbacksStale(t *testing.T) { | ||
| 137 | h := &GlobalShortcutHandler{} | 189 | h := &GlobalShortcutHandler{} | ||
| 138 | generation := h.generation | 190 | generation := h.generation | ||
M internal/ui/input/keyboard.go +14 -2
| @@ -243,8 +243,20 @@ func (h *KeyboardHandler) AttachTo(window *gtk.ApplicationWindow) { | |||
| 243 | 243 | log.Debug().Msg("keyboard handler attached to window") | |
| 244 | 244 | } | |
| 245 | 245 | ||
| 246 | - | // Detach removes the keyboard handler and releases its GTK signal callbacks. | |
| 246 | + | // Detach removes the keyboard handler from a live GTK window and releases its | |
| 247 | + | // retained GTK signal callbacks. | |
| 247 | 248 | func (h *KeyboardHandler) Detach() { | |
| 249 | + | h.detach(true) | |
| 250 | + | } | |
| 251 | + | ||
| 252 | + | // DetachForDestroy releases retained GTK signal callbacks during window | |
| 253 | + | // teardown without mutating the window's controller list while GTK is | |
| 254 | + | // destroying it. | |
| 255 | + | func (h *KeyboardHandler) DetachForDestroy() { | |
| 256 | + | h.detach(false) | |
| 257 | + | } | |
| 258 | + | ||
| 259 | + | func (h *KeyboardHandler) detach(removeController bool) { | |
| 248 | 260 | h.mu.Lock() | |
| 249 | 261 | defer h.mu.Unlock() | |
| 250 | 262 | if h.controller != nil && h.keyPressedHandlerID != 0 { | |
| @@ -253,7 +265,7 @@ func (h *KeyboardHandler) Detach() { | |||
| 253 | 265 | if h.controller != nil && h.keyReleasedHandlerID != 0 { | |
| 254 | 266 | h.controller.DisconnectSignal(h.keyReleasedHandlerID) | |
| 255 | 267 | } | |
| 256 | - | if h.window != nil && h.controller != nil { | |
| 268 | + | if removeController && h.window != nil && h.controller != nil { | |
| 257 | 269 | h.window.RemoveController(&h.controller.EventController) | |
| 258 | 270 | } | |
| 259 | 271 | h.controller = nil | |
| @@ -243,8 +243,20 @@ func (h *KeyboardHandler) AttachTo(window *gtk.ApplicationWindow) { | |||||
| 243 | log.Debug().Msg("keyboard handler attached to window") | 243 | log.Debug().Msg("keyboard handler attached to window") | ||
| 244 | } | 244 | } | ||
| 245 | 245 | ||||
| 246 | - | // Detach removes the keyboard handler and releases its GTK signal callbacks. | 246 | + | // Detach removes the keyboard handler from a live GTK window and releases its |
| 247 | + | // retained GTK signal callbacks. | |||
| 247 | func (h *KeyboardHandler) Detach() { | 248 | func (h *KeyboardHandler) Detach() { | ||
| 249 | + | h.detach(true) | |||
| 250 | + | } | |||
| 251 | + | ||||
| 252 | + | // DetachForDestroy releases retained GTK signal callbacks during window | |||
| 253 | + | // teardown without mutating the window's controller list while GTK is | |||
| 254 | + | // destroying it. | |||
| 255 | + | func (h *KeyboardHandler) DetachForDestroy() { | |||
| 256 | + | h.detach(false) | |||
| 257 | + | } | |||
| 258 | + | ||||
| 259 | + | func (h *KeyboardHandler) detach(removeController bool) { | |||
| 248 | h.mu.Lock() | 260 | h.mu.Lock() | ||
| 249 | defer h.mu.Unlock() | 261 | defer h.mu.Unlock() | ||
| 250 | if h.controller != nil && h.keyPressedHandlerID != 0 { | 262 | if h.controller != nil && h.keyPressedHandlerID != 0 { | ||
| @@ -253,7 +265,7 @@ func (h *KeyboardHandler) Detach() { | |||||
| 253 | if h.controller != nil && h.keyReleasedHandlerID != 0 { | 265 | if h.controller != nil && h.keyReleasedHandlerID != 0 { | ||
| 254 | h.controller.DisconnectSignal(h.keyReleasedHandlerID) | 266 | h.controller.DisconnectSignal(h.keyReleasedHandlerID) | ||
| 255 | } | 267 | } | ||
| 256 | - | if h.window != nil && h.controller != nil { | 268 | + | if removeController && h.window != nil && h.controller != nil { |
| 257 | h.window.RemoveController(&h.controller.EventController) | 269 | h.window.RemoveController(&h.controller.EventController) | ||
| 258 | } | 270 | } | ||
| 259 | h.controller = nil | 271 | h.controller = nil | ||
M internal/ui/input/keyboard_test.go +20 -0
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | ||
| 7 | 7 | "github.com/bnema/dumber/internal/domain/entity" | |
| 8 | 8 | "github.com/bnema/puregotk/v4/gdk" | |
| 9 | + | "github.com/bnema/puregotk/v4/gtk" | |
| 9 | 10 | "github.com/stretchr/testify/assert" | |
| 10 | 11 | ) | |
| 11 | 12 | ||
| @@ -231,6 +232,25 @@ func TestHandleKeyPress_NoRouteCallback_DefaultsToShortcuts(t *testing.T) { | |||
| 231 | 232 | assert.False(t, result, "unregistered key in ModeNormal returns false") | |
| 232 | 233 | } | |
| 233 | 234 | ||
| 235 | + | func TestKeyboardHandlerDetachForDestroyClearsRetainedCallbacksAndState(t *testing.T) { | |
| 236 | + | h := NewKeyboardHandler(context.Background(), newTestWorkspace(), newTestSession()) | |
| 237 | + | h.keyPressedCb = func(gtk.EventControllerKey, uint, uint, gdk.ModifierType) bool { return true } | |
| 238 | + | h.keyReleasedCb = func(gtk.EventControllerKey, uint, uint, gdk.ModifierType) {} | |
| 239 | + | h.keyPressedHandlerID = 1 | |
| 240 | + | h.keyReleasedHandlerID = 2 | |
| 241 | + | h.activePressedActions = map[Action]uint{ActionReload: 1} | |
| 242 | + | ||
| 243 | + | h.DetachForDestroy() | |
| 244 | + | ||
| 245 | + | assert.Nil(t, h.controller) | |
| 246 | + | assert.Nil(t, h.window) | |
| 247 | + | assert.Nil(t, h.keyPressedCb) | |
| 248 | + | assert.Nil(t, h.keyReleasedCb) | |
| 249 | + | assert.Zero(t, h.keyPressedHandlerID) | |
| 250 | + | assert.Zero(t, h.keyReleasedHandlerID) | |
| 251 | + | assert.Nil(t, h.activePressedActions) | |
| 252 | + | } | |
| 253 | + | ||
| 234 | 254 | func TestHandleKeyPress_SuppressesHeldHardwareFallbackTabSwitchUntilRelease(t *testing.T) { | |
| 235 | 255 | h := NewKeyboardHandler(context.Background(), newTestWorkspace(), newTestSession()) | |
| 236 | 256 | calls := 0 | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | 6 | ||||
| 7 | "github.com/bnema/dumber/internal/domain/entity" | 7 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 8 | "github.com/bnema/puregotk/v4/gdk" | 8 | "github.com/bnema/puregotk/v4/gdk" | ||
| 9 | + | "github.com/bnema/puregotk/v4/gtk" | |||
| 9 | "github.com/stretchr/testify/assert" | 10 | "github.com/stretchr/testify/assert" | ||
| 10 | ) | 11 | ) | ||
| 11 | 12 | ||||
| @@ -231,6 +232,25 @@ func TestHandleKeyPress_NoRouteCallback_DefaultsToShortcuts(t *testing.T) { | |||||
| 231 | assert.False(t, result, "unregistered key in ModeNormal returns false") | 232 | assert.False(t, result, "unregistered key in ModeNormal returns false") | ||
| 232 | } | 233 | } | ||
| 233 | 234 | ||||
| 235 | + | func TestKeyboardHandlerDetachForDestroyClearsRetainedCallbacksAndState(t *testing.T) { | |||
| 236 | + | h := NewKeyboardHandler(context.Background(), newTestWorkspace(), newTestSession()) | |||
| 237 | + | h.keyPressedCb = func(gtk.EventControllerKey, uint, uint, gdk.ModifierType) bool { return true } | |||
| 238 | + | h.keyReleasedCb = func(gtk.EventControllerKey, uint, uint, gdk.ModifierType) {} | |||
| 239 | + | h.keyPressedHandlerID = 1 | |||
| 240 | + | h.keyReleasedHandlerID = 2 | |||
| 241 | + | h.activePressedActions = map[Action]uint{ActionReload: 1} | |||
| 242 | + | ||||
| 243 | + | h.DetachForDestroy() | |||
| 244 | + | ||||
| 245 | + | assert.Nil(t, h.controller) | |||
| 246 | + | assert.Nil(t, h.window) | |||
| 247 | + | assert.Nil(t, h.keyPressedCb) | |||
| 248 | + | assert.Nil(t, h.keyReleasedCb) | |||
| 249 | + | assert.Zero(t, h.keyPressedHandlerID) | |||
| 250 | + | assert.Zero(t, h.keyReleasedHandlerID) | |||
| 251 | + | assert.Nil(t, h.activePressedActions) | |||
| 252 | + | } | |||
| 253 | + | ||||
| 234 | func TestHandleKeyPress_SuppressesHeldHardwareFallbackTabSwitchUntilRelease(t *testing.T) { | 254 | func TestHandleKeyPress_SuppressesHeldHardwareFallbackTabSwitchUntilRelease(t *testing.T) { | ||
| 235 | h := NewKeyboardHandler(context.Background(), newTestWorkspace(), newTestSession()) | 255 | h := NewKeyboardHandler(context.Background(), newTestWorkspace(), newTestSession()) | ||
| 236 | calls := 0 | 256 | calls := 0 | ||
M internal/ui/systemviews/components_templ.go +16 -16
| @@ -38,11 +38,11 @@ func appFrameComponent(page renderedPage, theme shellTheme, body templ.Component | |||
| 38 | 38 | return templ_7745c5c3_Err | |
| 39 | 39 | } | |
| 40 | 40 | var templ_7745c5c3_Var3 string | |
| 41 | - | templ_7745c5c3_Var3, templ_7745c5c3_Err = templ.JoinStringErrs(templ.CSSClasses(templ_7745c5c3_Var2).String()) | |
| 41 | + | templ_7745c5c3_Var3, templ_7745c5c3_Err = templ.ResolveAttributeValue(templ.CSSClasses(templ_7745c5c3_Var2).String()) | |
| 42 | 42 | if templ_7745c5c3_Err != nil { | |
| 43 | 43 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 1, Col: 0} | |
| 44 | 44 | } | |
| 45 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var3)) | |
| 45 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var3) | |
| 46 | 46 | if templ_7745c5c3_Err != nil { | |
| 47 | 47 | return templ_7745c5c3_Err | |
| 48 | 48 | } | |
| @@ -51,11 +51,11 @@ func appFrameComponent(page renderedPage, theme shellTheme, body templ.Component | |||
| 51 | 51 | return templ_7745c5c3_Err | |
| 52 | 52 | } | |
| 53 | 53 | var templ_7745c5c3_Var4 string | |
| 54 | - | templ_7745c5c3_Var4, templ_7745c5c3_Err = templ.JoinStringErrs(string(page.route)) | |
| 54 | + | templ_7745c5c3_Var4, templ_7745c5c3_Err = templ.ResolveAttributeValue(string(page.route)) | |
| 55 | 55 | if templ_7745c5c3_Err != nil { | |
| 56 | 56 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 4, Col: 67} | |
| 57 | 57 | } | |
| 58 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var4)) | |
| 58 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var4) | |
| 59 | 59 | if templ_7745c5c3_Err != nil { | |
| 60 | 60 | return templ_7745c5c3_Err | |
| 61 | 61 | } | |
| @@ -64,11 +64,11 @@ func appFrameComponent(page renderedPage, theme shellTheme, body templ.Component | |||
| 64 | 64 | return templ_7745c5c3_Err | |
| 65 | 65 | } | |
| 66 | 66 | var templ_7745c5c3_Var5 string | |
| 67 | - | templ_7745c5c3_Var5, templ_7745c5c3_Err = templ.JoinStringErrs(pageDocumentTitle(page)) | |
| 67 | + | templ_7745c5c3_Var5, templ_7745c5c3_Err = templ.ResolveAttributeValue(pageDocumentTitle(page)) | |
| 68 | 68 | if templ_7745c5c3_Err != nil { | |
| 69 | 69 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 4, Col: 111} | |
| 70 | 70 | } | |
| 71 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var5)) | |
| 71 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var5) | |
| 72 | 72 | if templ_7745c5c3_Err != nil { | |
| 73 | 73 | return templ_7745c5c3_Err | |
| 74 | 74 | } | |
| @@ -188,11 +188,11 @@ func Section(className string, title string) templ.Component { | |||
| 188 | 188 | return templ_7745c5c3_Err | |
| 189 | 189 | } | |
| 190 | 190 | var templ_7745c5c3_Var11 string | |
| 191 | - | templ_7745c5c3_Var11, templ_7745c5c3_Err = templ.JoinStringErrs(templ.CSSClasses(templ_7745c5c3_Var10).String()) | |
| 191 | + | templ_7745c5c3_Var11, templ_7745c5c3_Err = templ.ResolveAttributeValue(templ.CSSClasses(templ_7745c5c3_Var10).String()) | |
| 192 | 192 | if templ_7745c5c3_Err != nil { | |
| 193 | 193 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 1, Col: 0} | |
| 194 | 194 | } | |
| 195 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var11)) | |
| 195 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var11) | |
| 196 | 196 | if templ_7745c5c3_Err != nil { | |
| 197 | 197 | return templ_7745c5c3_Err | |
| 198 | 198 | } | |
| @@ -425,11 +425,11 @@ func Alert(kind string, message string) templ.Component { | |||
| 425 | 425 | return templ_7745c5c3_Err | |
| 426 | 426 | } | |
| 427 | 427 | var templ_7745c5c3_Var23 string | |
| 428 | - | templ_7745c5c3_Var23, templ_7745c5c3_Err = templ.JoinStringErrs(templ.CSSClasses(templ_7745c5c3_Var22).String()) | |
| 428 | + | templ_7745c5c3_Var23, templ_7745c5c3_Err = templ.ResolveAttributeValue(templ.CSSClasses(templ_7745c5c3_Var22).String()) | |
| 429 | 429 | if templ_7745c5c3_Err != nil { | |
| 430 | 430 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 1, Col: 0} | |
| 431 | 431 | } | |
| 432 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var23)) | |
| 432 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var23) | |
| 433 | 433 | if templ_7745c5c3_Err != nil { | |
| 434 | 434 | return templ_7745c5c3_Err | |
| 435 | 435 | } | |
| @@ -438,11 +438,11 @@ func Alert(kind string, message string) templ.Component { | |||
| 438 | 438 | return templ_7745c5c3_Err | |
| 439 | 439 | } | |
| 440 | 440 | var templ_7745c5c3_Var24 string | |
| 441 | - | templ_7745c5c3_Var24, templ_7745c5c3_Err = templ.JoinStringErrs(alertRole(kind)) | |
| 441 | + | templ_7745c5c3_Var24, templ_7745c5c3_Err = templ.ResolveAttributeValue(alertRole(kind)) | |
| 442 | 442 | if templ_7745c5c3_Err != nil { | |
| 443 | 443 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 56, Col: 56} | |
| 444 | 444 | } | |
| 445 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var24)) | |
| 445 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var24) | |
| 446 | 446 | if templ_7745c5c3_Err != nil { | |
| 447 | 447 | return templ_7745c5c3_Err | |
| 448 | 448 | } | |
| @@ -573,11 +573,11 @@ func Button(label string, action string, classes string, attrs templ.Attributes, | |||
| 573 | 573 | return templ_7745c5c3_Err | |
| 574 | 574 | } | |
| 575 | 575 | var templ_7745c5c3_Var30 string | |
| 576 | - | templ_7745c5c3_Var30, templ_7745c5c3_Err = templ.JoinStringErrs(templ.CSSClasses(templ_7745c5c3_Var29).String()) | |
| 576 | + | templ_7745c5c3_Var30, templ_7745c5c3_Err = templ.ResolveAttributeValue(templ.CSSClasses(templ_7745c5c3_Var29).String()) | |
| 577 | 577 | if templ_7745c5c3_Err != nil { | |
| 578 | 578 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 1, Col: 0} | |
| 579 | 579 | } | |
| 580 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var30)) | |
| 580 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var30) | |
| 581 | 581 | if templ_7745c5c3_Err != nil { | |
| 582 | 582 | return templ_7745c5c3_Err | |
| 583 | 583 | } | |
| @@ -586,11 +586,11 @@ func Button(label string, action string, classes string, attrs templ.Attributes, | |||
| 586 | 586 | return templ_7745c5c3_Err | |
| 587 | 587 | } | |
| 588 | 588 | var templ_7745c5c3_Var31 string | |
| 589 | - | templ_7745c5c3_Var31, templ_7745c5c3_Err = templ.JoinStringErrs(action) | |
| 589 | + | templ_7745c5c3_Var31, templ_7745c5c3_Err = templ.ResolveAttributeValue(action) | |
| 590 | 590 | if templ_7745c5c3_Err != nil { | |
| 591 | 591 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 73, Col: 77} | |
| 592 | 592 | } | |
| 593 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var31)) | |
| 593 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var31) | |
| 594 | 594 | if templ_7745c5c3_Err != nil { | |
| 595 | 595 | return templ_7745c5c3_Err | |
| 596 | 596 | } | |
| @@ -38,11 +38,11 @@ func appFrameComponent(page renderedPage, theme shellTheme, body templ.Component | |||||
| 38 | return templ_7745c5c3_Err | 38 | return templ_7745c5c3_Err | ||
| 39 | } | 39 | } | ||
| 40 | var templ_7745c5c3_Var3 string | 40 | var templ_7745c5c3_Var3 string | ||
| 41 | - | templ_7745c5c3_Var3, templ_7745c5c3_Err = templ.JoinStringErrs(templ.CSSClasses(templ_7745c5c3_Var2).String()) | 41 | + | templ_7745c5c3_Var3, templ_7745c5c3_Err = templ.ResolveAttributeValue(templ.CSSClasses(templ_7745c5c3_Var2).String()) |
| 42 | if templ_7745c5c3_Err != nil { | 42 | if templ_7745c5c3_Err != nil { | ||
| 43 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 1, Col: 0} | 43 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 1, Col: 0} | ||
| 44 | } | 44 | } | ||
| 45 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var3)) | 45 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var3) |
| 46 | if templ_7745c5c3_Err != nil { | 46 | if templ_7745c5c3_Err != nil { | ||
| 47 | return templ_7745c5c3_Err | 47 | return templ_7745c5c3_Err | ||
| 48 | } | 48 | } | ||
| @@ -51,11 +51,11 @@ func appFrameComponent(page renderedPage, theme shellTheme, body templ.Component | |||||
| 51 | return templ_7745c5c3_Err | 51 | return templ_7745c5c3_Err | ||
| 52 | } | 52 | } | ||
| 53 | var templ_7745c5c3_Var4 string | 53 | var templ_7745c5c3_Var4 string | ||
| 54 | - | templ_7745c5c3_Var4, templ_7745c5c3_Err = templ.JoinStringErrs(string(page.route)) | 54 | + | templ_7745c5c3_Var4, templ_7745c5c3_Err = templ.ResolveAttributeValue(string(page.route)) |
| 55 | if templ_7745c5c3_Err != nil { | 55 | if templ_7745c5c3_Err != nil { | ||
| 56 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 4, Col: 67} | 56 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 4, Col: 67} | ||
| 57 | } | 57 | } | ||
| 58 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var4)) | 58 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var4) |
| 59 | if templ_7745c5c3_Err != nil { | 59 | if templ_7745c5c3_Err != nil { | ||
| 60 | return templ_7745c5c3_Err | 60 | return templ_7745c5c3_Err | ||
| 61 | } | 61 | } | ||
| @@ -64,11 +64,11 @@ func appFrameComponent(page renderedPage, theme shellTheme, body templ.Component | |||||
| 64 | return templ_7745c5c3_Err | 64 | return templ_7745c5c3_Err | ||
| 65 | } | 65 | } | ||
| 66 | var templ_7745c5c3_Var5 string | 66 | var templ_7745c5c3_Var5 string | ||
| 67 | - | templ_7745c5c3_Var5, templ_7745c5c3_Err = templ.JoinStringErrs(pageDocumentTitle(page)) | 67 | + | templ_7745c5c3_Var5, templ_7745c5c3_Err = templ.ResolveAttributeValue(pageDocumentTitle(page)) |
| 68 | if templ_7745c5c3_Err != nil { | 68 | if templ_7745c5c3_Err != nil { | ||
| 69 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 4, Col: 111} | 69 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 4, Col: 111} | ||
| 70 | } | 70 | } | ||
| 71 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var5)) | 71 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var5) |
| 72 | if templ_7745c5c3_Err != nil { | 72 | if templ_7745c5c3_Err != nil { | ||
| 73 | return templ_7745c5c3_Err | 73 | return templ_7745c5c3_Err | ||
| 74 | } | 74 | } | ||
| @@ -188,11 +188,11 @@ func Section(className string, title string) templ.Component { | |||||
| 188 | return templ_7745c5c3_Err | 188 | return templ_7745c5c3_Err | ||
| 189 | } | 189 | } | ||
| 190 | var templ_7745c5c3_Var11 string | 190 | var templ_7745c5c3_Var11 string | ||
| 191 | - | templ_7745c5c3_Var11, templ_7745c5c3_Err = templ.JoinStringErrs(templ.CSSClasses(templ_7745c5c3_Var10).String()) | 191 | + | templ_7745c5c3_Var11, templ_7745c5c3_Err = templ.ResolveAttributeValue(templ.CSSClasses(templ_7745c5c3_Var10).String()) |
| 192 | if templ_7745c5c3_Err != nil { | 192 | if templ_7745c5c3_Err != nil { | ||
| 193 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 1, Col: 0} | 193 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 1, Col: 0} | ||
| 194 | } | 194 | } | ||
| 195 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var11)) | 195 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var11) |
| 196 | if templ_7745c5c3_Err != nil { | 196 | if templ_7745c5c3_Err != nil { | ||
| 197 | return templ_7745c5c3_Err | 197 | return templ_7745c5c3_Err | ||
| 198 | } | 198 | } | ||
| @@ -425,11 +425,11 @@ func Alert(kind string, message string) templ.Component { | |||||
| 425 | return templ_7745c5c3_Err | 425 | return templ_7745c5c3_Err | ||
| 426 | } | 426 | } | ||
| 427 | var templ_7745c5c3_Var23 string | 427 | var templ_7745c5c3_Var23 string | ||
| 428 | - | templ_7745c5c3_Var23, templ_7745c5c3_Err = templ.JoinStringErrs(templ.CSSClasses(templ_7745c5c3_Var22).String()) | 428 | + | templ_7745c5c3_Var23, templ_7745c5c3_Err = templ.ResolveAttributeValue(templ.CSSClasses(templ_7745c5c3_Var22).String()) |
| 429 | if templ_7745c5c3_Err != nil { | 429 | if templ_7745c5c3_Err != nil { | ||
| 430 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 1, Col: 0} | 430 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 1, Col: 0} | ||
| 431 | } | 431 | } | ||
| 432 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var23)) | 432 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var23) |
| 433 | if templ_7745c5c3_Err != nil { | 433 | if templ_7745c5c3_Err != nil { | ||
| 434 | return templ_7745c5c3_Err | 434 | return templ_7745c5c3_Err | ||
| 435 | } | 435 | } | ||
| @@ -438,11 +438,11 @@ func Alert(kind string, message string) templ.Component { | |||||
| 438 | return templ_7745c5c3_Err | 438 | return templ_7745c5c3_Err | ||
| 439 | } | 439 | } | ||
| 440 | var templ_7745c5c3_Var24 string | 440 | var templ_7745c5c3_Var24 string | ||
| 441 | - | templ_7745c5c3_Var24, templ_7745c5c3_Err = templ.JoinStringErrs(alertRole(kind)) | 441 | + | templ_7745c5c3_Var24, templ_7745c5c3_Err = templ.ResolveAttributeValue(alertRole(kind)) |
| 442 | if templ_7745c5c3_Err != nil { | 442 | if templ_7745c5c3_Err != nil { | ||
| 443 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 56, Col: 56} | 443 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 56, Col: 56} | ||
| 444 | } | 444 | } | ||
| 445 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var24)) | 445 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var24) |
| 446 | if templ_7745c5c3_Err != nil { | 446 | if templ_7745c5c3_Err != nil { | ||
| 447 | return templ_7745c5c3_Err | 447 | return templ_7745c5c3_Err | ||
| 448 | } | 448 | } | ||
| @@ -573,11 +573,11 @@ func Button(label string, action string, classes string, attrs templ.Attributes, | |||||
| 573 | return templ_7745c5c3_Err | 573 | return templ_7745c5c3_Err | ||
| 574 | } | 574 | } | ||
| 575 | var templ_7745c5c3_Var30 string | 575 | var templ_7745c5c3_Var30 string | ||
| 576 | - | templ_7745c5c3_Var30, templ_7745c5c3_Err = templ.JoinStringErrs(templ.CSSClasses(templ_7745c5c3_Var29).String()) | 576 | + | templ_7745c5c3_Var30, templ_7745c5c3_Err = templ.ResolveAttributeValue(templ.CSSClasses(templ_7745c5c3_Var29).String()) |
| 577 | if templ_7745c5c3_Err != nil { | 577 | if templ_7745c5c3_Err != nil { | ||
| 578 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 1, Col: 0} | 578 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 1, Col: 0} | ||
| 579 | } | 579 | } | ||
| 580 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var30)) | 580 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var30) |
| 581 | if templ_7745c5c3_Err != nil { | 581 | if templ_7745c5c3_Err != nil { | ||
| 582 | return templ_7745c5c3_Err | 582 | return templ_7745c5c3_Err | ||
| 583 | } | 583 | } | ||
| @@ -586,11 +586,11 @@ func Button(label string, action string, classes string, attrs templ.Attributes, | |||||
| 586 | return templ_7745c5c3_Err | 586 | return templ_7745c5c3_Err | ||
| 587 | } | 587 | } | ||
| 588 | var templ_7745c5c3_Var31 string | 588 | var templ_7745c5c3_Var31 string | ||
| 589 | - | templ_7745c5c3_Var31, templ_7745c5c3_Err = templ.JoinStringErrs(action) | 589 | + | templ_7745c5c3_Var31, templ_7745c5c3_Err = templ.ResolveAttributeValue(action) |
| 590 | if templ_7745c5c3_Err != nil { | 590 | if templ_7745c5c3_Err != nil { | ||
| 591 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 73, Col: 77} | 591 | return templ.Error{Err: templ_7745c5c3_Err, FileName: `components.templ`, Line: 73, Col: 77} | ||
| 592 | } | 592 | } | ||
| 593 | - | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var31)) | 593 | + | _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var31) |
| 594 | if templ_7745c5c3_Err != nil { | 594 | if templ_7745c5c3_Err != nil { | ||
| 595 | return templ_7745c5c3_Err | 595 | return templ_7745c5c3_Err | ||
| 596 | } | 596 | } | ||