resolve the two remaining TODOs in httpx#132
Merged
Merged
Conversation
httpx/payload.go: the "ghetto if statement" dispatching
InternalFnInspect / InternalFnBuild is replaced with a map[string]
internalFn dispatch table at the bottom of the file. The function
signature `internalFn(w, e, *Handler) error` is uniform across both
entries; Inspect (which doesn't need the handler) is adapted via a
short closure. Adding a new internal_function name is now a one-line
map insertion, so the TODO predicting "if more than 3 blocks" no
longer applies.
types/interfaces.go: new optional Seeder interface
type Seeder interface { Seed() error }
with documented semantics — called exactly once, idempotent, runs
before Start.
httpx/handler.go: NewHandler no longer hard-codes Seed(model.DB());
instead, *httpx.Handler implements Seeder via a Seed method that
delegates to the package-level Seed function (which is itself gated
by a `seeded` bool, so it remains idempotent across calls).
xodbox/run.go: NewApp now iterates config.Handlers and calls Seed()
on each handler that implements types.Seeder. This is done in NewApp
rather than Run so that non-Run entry points like `xodbox payload
dump` still get the bundled payloads in the DB (regression spotted
during testing).
Tests:
- pkg/xodbox/seeder_test.go: NewApp calls Seed on implementers,
continues past Seeders that return errors, and the compile-time
contract is asserted.
Full suite passes under -race; golangci-lint clean.
Targets:
make help print every target
make test run the full test suite
make race run the test suite with -race -timeout 180s
make cover run -race tests with a coverage profile and print summary
make lint run golangci-lint over ./...
make fmt run gofmt -s -w .
make build build the xodbox binary into ./bin/ with the same
-trimpath / -ldflags as the release pipeline
make run make build then ./bin/xodbox serve
make tidy go mod tidy + fail if go.mod/go.sum changed
make release-dry goreleaser snapshot run (publish/sign/sbom skipped)
make clean rm -rf bin/ coverage.out dist/
`make help` self-discovers targets from `## docstring` comments so
adding a new target documents it for free.
Also add `bin/` and `coverage.out` to .gitignore so the artifacts
generated by `make build` / `make cover` don't end up staged.
No code changes; full suite still passes, golangci-lint clean.
GitHub's Dependabot has been reporting 2 moderate vulnerabilities on defektive/xodbox main but neither the Dependabot API nor vuln.go.dev is reachable from this sandbox, so a targeted fix-by-CVE wasn't possible. As a best-effort triage, bump the two security-relevant modules with available updates: github.com/go-jose/go-jose/v4 v4.1.3 -> v4.1.4 github.com/klauspost/compress v1.17.6 -> v1.18.6 go-jose minor releases routinely patch JOSE/JWT advisories, and klauspost/compress had a string of DoS findings through the 1.17 line. Also pulls in their indirect bumps via `go mod tidy`. Confirm with `govulncheck ./...` once the alert page is reachable again. If anything is still flagged after this, the offending dependency can be narrowed down with `gh api repos/defektive/xodbox/dependabot/alerts` and the remediation re-applied. Also expand the .golangci.yml exclusion regex for the mdaas main programs from `pkg/mdaas/(bind-shell|simple-ssh)/` to `(^|/)mdaas/(bind-shell|simple-ssh)/` so it covers both the embedded source under pkg/ and the runtime extraction target at ./mdaas/ that pkg/mdaas/embed.go's init populates on first use. Without this update, lint runs that follow a pkg/mdaas test session would fail on errcheck/gosec hits in the extracted source. Tests still pass under -race; golangci-lint clean.
The previous HTTPS path in pkg/handlers/httpx/handler.go was served by a package-level HTTPS() helper that managed its own singleton listeners (httpLn, httpsLn) and *http.Server instances out of package globals. The Handler never saw those servers, so Stop() in HTTPS mode was effectively a no-op — Stop returned nil but the ACME challenge and TLS servers stayed up until process exit. Refactor: - The package-level HTTPS() / httpLn / httpsLn / lnMu / httpWg are gone. Replaced by (*Handler).serveTLS, which provisions certificates via certmagic.ManageSync, binds both the ACME HTTP-01 listener on certmagic.HTTPPort and the TLS listener on certmagic.HTTPSPort, constructs the two *http.Server instances, and records them on the Handler before serving. - Two new Handler fields: httpChallengeServer (ACME HTTP-01, rendered with the ACMEIssuer's HTTPChallengeHandler wrap) and httpsServer (TLS-terminated payload mux). Both are guarded by the existing h.mu. - Stop now reads all three server fields under the lock and calls Shutdown(ctx) on each. The helper does best-effort: it always attempts every shutdown and returns the first non-nil error rather than bailing on the first failure. - The challenge-server goroutine swallows the expected http.ErrServerClosed return from Serve after Shutdown. The TLS server's Serve return is filtered similarly. Test: TestStopShutsDownHTTPSServerPair populates the three server fields with bound (unstarted) *http.Server instances and verifies Stop puts them all into closed state — a follow-up Shutdown on each either returns nil or http.ErrServerClosed. Docs: pkg/handlers/httpx/_index.md now reflects that Stop is fully graceful-shutdown-aware in both HTTP and HTTPS modes. Full suite passes under -race; golangci-lint clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
httpx/payload.go: the "ghetto if statement" dispatching
InternalFnInspect / InternalFnBuild is replaced with a map[string]
internalFn dispatch table at the bottom of the file. The function
signature
internalFn(w, e, *Handler) erroris uniform across bothentries; Inspect (which doesn't need the handler) is adapted via a
short closure. Adding a new internal_function name is now a one-line
map insertion, so the TODO predicting "if more than 3 blocks" no
longer applies.
types/interfaces.go: new optional Seeder interface
type Seeder interface { Seed() error }
with documented semantics — called exactly once, idempotent, runs
before Start.
httpx/handler.go: NewHandler no longer hard-codes Seed(model.DB());
instead, *httpx.Handler implements Seeder via a Seed method that
delegates to the package-level Seed function (which is itself gated
by a
seededbool, so it remains idempotent across calls).xodbox/run.go: NewApp now iterates config.Handlers and calls Seed()
on each handler that implements types.Seeder. This is done in NewApp
rather than Run so that non-Run entry points like
xodbox payload dumpstill get the bundled payloads in the DB (regression spottedduring testing).
Tests:
continues past Seeders that return errors, and the compile-time
contract is asserted.
Full suite passes under -race; golangci-lint clean.