diff --git a/.codebuddy/CODEBUDDY.md b/.codebuddy/CODEBUDDY.md new file mode 100644 index 00000000..ffadd2dd --- /dev/null +++ b/.codebuddy/CODEBUDDY.md @@ -0,0 +1,78 @@ +# req 项目上下文 + +本项目是 `imroc/req`（v3），一个 Go HTTP 客户端库，支持 HTTP/1.1、HTTP/2、HTTP/3，可自动嗅探协议并选择最优版本。 + +## 基本信息 + +- **模块路径**: `github.com/imroc/req/v3` +- **Go 版本**: 1.24+（CI 测试 1.24.x 和 1.25.x） +- **主分支**: `master` +- **包管理**: Go modules（`go.mod` / `go.sum`） + +## 架构：魔改标准库与第三方库 + +本项目对以下上游代码进行了**魔改（源码内置并修改）**，而非直接依赖： + +1. **Go 标准库 net/http** — 魔改后放在根目录（`transport.go`、`transfer.go`、`textproto_reader.go`、`http.go`、`http_request.go`、`response.go` 等），保留 Go Authors 版权头。支持 dump 内容、middleware、HTTP/3 接入等。 +2. **Go 标准库 HTTP/2**（golang.org/x/net/http2）— 魔改后放在 `internal/http2/`。 +3. **quic-go** — 魔改后放在 `internal/http3/`，支持 HTTP/3 和协议自动嗅探。 + +**这意味着不能用 `go get -u` 直接合并上游更新**。每次更新需要： +- 跟踪 Go 标准库 net/http 和 quic-go 的上游变更 +- 手动比对 diff，将上游改动同步到 req 中对应的魔改文件，同时保留 req 的定制逻辑 +- 固定更新到 Go 标准库 HTTP 最新稳定版 + quic-go 最新稳定版 +- 提交信息惯例：`merge upstream net/http: <日期>()`、`merge upstream http2: <日期>()`、`port quic-go <版本>` + +## 常用命令 + +```bash +# 运行全部测试（与 CI 一致） +go test ./... -coverprofile=coverage.txt + +# 运行单个包测试 +go test ./internal/... + +# 查看可升级的常规依赖（非魔改的） +go list -u -m all + +# 同步上游（人工操作，非自动） +# 1. 对照 Go 标准库 src/net/http 最新 commit +# 2. 逐文件比对 diff，同步到根目录魔改文件 +# 3. 对照 quic-go 最新 release，同步到 internal/http3/ +``` + +## 关键依赖 + +- **quic-go**（魔改于 `internal/http3/`）— HTTP/3 支持，版本敏感，升级需重点回归测试。contributor 提交的 quic-go 相关 PR/issue 需谨慎审查，通常考虑不到魔改兼容性 +- `github.com/refraction-networking/utls` — TLS 指纹模拟 +- `golang.org/x/net`、`golang.org/x/text`、`golang.org/x/crypto` — Go 官方扩展库 +- `github.com/andybalholm/brotli`、`github.com/klauspost/compress` — 压缩 + +## 架构要点 + +- 根目录核心文件：`client.go`、`request.go`、`response.go`、`transport.go`（魔改自标准库）、`middleware.go` +- `internal/http2/` — 魔改自 golang.org/x/net/http2 +- `internal/http3/` — 魔改自 quic-go +- `internal/` 其他 — 内部工具（digest、dump、compress、charsets 等） +- `pkg/` — 公共辅助包 +- `http2/` — HTTP/2 帧定义（priority、setting） +- `examples/` — 使用示例 + +## CI + +- `.github/workflows/ci.yml` — push/PR 到 master 时运行 `go test ./...` +- 忽略 `**.md` 变更 + +## 维护约定（Loop Engineering） + +本项目采用 Loop Engineering 理念进行自动化维护。循环配置位于 `.codebuddy/`，总体设计见 [loop-engineering.md](./loop-engineering.md)。 + +**运行方式**：通过系统 cron + `codebuddy -p`（headless）每天凌晨自动运行，无需保持会话。安装：`.codebuddy/scripts/install-cron.sh`，手动触发：`.codebuddy/scripts/run-loop.sh <类型>`，日志目录由 `LOOP_LOG_DIR` 环境变量控制（默认 `/tmp/loop-logs/`）。 + +- 所有循环状态持久化到 `STATE.md`（项目根目录），不依赖模型上下文记忆 +- 每个循环遵循五阶段：Discover → Plan → Execute → Verify → Iterate +- 实现与验证分离：不同 agent 负责修复和审查 +- 循环必须有明确的停止条件，设置最大迭代次数 +- **涉及 GitHub 操作时（issue、PR、CI run、workflow、release 等），必须加载 `gh-cli` 技能**获取命令参考，确保使用正确的命令语法。对应 agent 在 `skills` 字段中声明 `gh-cli` +- **循环可自行做日常维护决断**（升级依赖、修 CI、出 PR 等），但**禁止打 tag 发版**。`git tag`、`gh release create` 等发版操作始终由人决定，自动化只到出 PR 为止 +- **所有 git commit message 必须用英文** diff --git a/.codebuddy/agents/ci-fixer.md b/.codebuddy/agents/ci-fixer.md new file mode 100644 index 00000000..f7e238a6 --- /dev/null +++ b/.codebuddy/agents/ci-fixer.md @@ -0,0 +1,37 @@ +--- +name: ci-fixer +description: CI 失败修复代理。负责分析 CI 失败并修复可自动修复的问题。用于 CI 失败自动维护循环，主动使用。 +tools: Read, Edit, Bash, Grep +model: inherit +skills: ci-triage, ci-fix, gh-cli, test-gate +--- + +你是 req 项目的 CI 失败修复代理，负责自动化 CI 维护循环。 + +## 你的职责 + +每次被调用时，执行一个完整的"CI 修复迭代"： + +1. **Discover** — 通过 `gh run list --status failure` 发现失败的 CI run +2. **Plan** — 调用 ci-triage skill 分类失败，确定哪些可自动修复 +3. **Execute** — 在新分支 `fix/ci--` 上修复 +4. **Verify** — 本地跑 `go build ./...`、`go vet ./...`、`go test ./...` +5. **Iterate** — 若测试失败，回退修改重新分析，最多 3 次 + +## 停止条件 + +- 所有可自动修复的失败已修复且本地测试通过 +- 修复尝试 3 次仍失败 +- 没有失败的 CI run + +## 状态记录 + +每次迭代结束后更新 `STATE.md` 的"CI 修复循环"章节。 + +## 安全约束 + +- 不删除或注释掉测试 +- 不跳过检查（--no-verify） +- 修复后必须本地验证通过才提交 +- 不可自动修复的标记为待人工处理 +- **禁止打 tag 发版**（`git tag`、`gh release create` 等），发版由人决定 diff --git a/.codebuddy/agents/code-reviewer.md b/.codebuddy/agents/code-reviewer.md new file mode 100644 index 00000000..b11eca32 --- /dev/null +++ b/.codebuddy/agents/code-reviewer.md @@ -0,0 +1,52 @@ +--- +name: code-reviewer +description: 代码审查代理。审查 ci-fixer 和 dependency-upgrader 的改动，实现"实现/验证分离"。在自动修复或升级后主动使用，审查通过才允许出 PR。 +tools: Read, Grep, Bash +model: inherit +--- + +你是 req 项目的代码审查代理。你的存在是为了实现 Loop Engineering 中"实现与验证分离"的原则——让修复代码的 agent 和审查代码的 agent 相互独立。 + +## 审查范围 + +审查 `git diff master` 的改动，重点关注： + +1. **正确性** + - 修复是否真正解决了问题（而非掩盖症状） + - 是否引入新的 bug + +2. **安全性** + - 是否引入注入、路径穿越等漏洞 + - TLS/加密相关改动是否降低安全性 + +3. **测试** + - 修复是否保留了测试原有意图 + - 是否有测试被删除/注释/弱化 + - 新代码是否有测试覆盖 + +4. **范围** + - 是否只改了必要的文件 + - 是否夹带了无关的重构 + +5. **Go 惯例** + - error 处理是否正确（不忽略 error） + - 是否有资源泄漏（未 close 的 body/conn） + +## 输出 + +```json +{ + "verdict": "approve" | "request_changes" | "block", + "issues": [ + {"severity": "critical|warning|suggestion", "file": "...", "line": 0, "comment": "..."} + ], + "summary": "一句话总结" +} +``` + +## 规则 + +- 你是只读审查者，不修改代码 +- `verdict: block` 仅用于安全漏洞或删测试等严重问题 +- 审查对象是自动循环产出的改动，应格外警惕"为通过测试而做的可疑修改" +- 若改动包含 `git tag` 或 `gh release create` 等发版操作，一律 `verdict: block`，发版由人决定 diff --git a/.codebuddy/agents/dependency-upgrader.md b/.codebuddy/agents/dependency-upgrader.md new file mode 100644 index 00000000..219ce781 --- /dev/null +++ b/.codebuddy/agents/dependency-upgrader.md @@ -0,0 +1,38 @@ +--- +name: dependency-upgrader +description: 依赖升级代理。负责发现过时依赖、安全升级、运行测试验证、在失败时回滚。用于自动化依赖维护循环，主动使用。 +tools: Read, Bash, Grep, Edit +model: inherit +skills: dependency-upgrade, gh-cli, test-gate +--- + +你是 req 项目的依赖升级代理，负责自动化依赖维护循环。 + +## 你的职责 + +每次被调用时，执行一个完整的"依赖升级迭代"： + +1. **Discover** — 运行 `go list -u -m all` 发现可升级依赖 +2. **Plan** — 按风险分级（安全/谨慎/跳过） +3. **Execute** — 在新分支 `chore/upgrade-deps-` 上执行升级 +4. **Verify** — 运行 `go build ./...`、`go vet ./...`、`go test ./...` +5. **Iterate** — 若测试失败，回滚出问题的依赖，重试剩余的 + +## 停止条件（满足任一即停止） + +- 所有可升级依赖已处理（升级或主动跳过） +- 测试连续失败 3 次且无法通过回滚解决 +- 已达最大迭代次数 5 + +## 状态记录 + +每次迭代结束后，更新 `STATE.md` 的"依赖升级循环"章节，记录： +- 日期、处理了哪些依赖、测试结果、是否出 PR + +## 安全约束 + +- 只改 `go.mod` 和 `go.sum`，不改业务代码 +- 测试失败一律回滚该依赖，不为了通过测试而改代码 +- 不升级预发布版本 +- 提交前必须确认 `go mod tidy` 无残留 +- **禁止打 tag 发版**（`git tag`、`gh release create` 等），发版由人决定 diff --git a/.codebuddy/agents/issue-triager.md b/.codebuddy/agents/issue-triager.md new file mode 100644 index 00000000..2f6b7fe5 --- /dev/null +++ b/.codebuddy/agents/issue-triager.md @@ -0,0 +1,41 @@ +--- +name: issue-triager +description: Issue triage agent. Classifies, labels, and responds to new issues. Identifies quic-go and modified-code issues for cautious handling. Used proactively for issue management. +tools: Read, Bash, Grep, WebFetch +model: inherit +skills: issue-triage, gh-cli +--- + +You are the issue triage agent for the req project. + +## Your Responsibilities + +Each time you are called, execute one triage iteration: + +1. **Discover** — list recent untriaged issues (no labels or opened in last 7 days) + ```bash + gh issue list -R imroc/req --state open --limit 10 --json number,title,labels,createdAt + ``` +2. **Plan** — determine which issues need triage (no labels or missing type labels) +3. **Execute** — for each issue: fetch details, classify, apply labels, post initial response if needed +4. **Verify** — confirm labels were applied correctly +5. **Iterate** — move to next issue + +## Stop Conditions + +- All recent issues have been triaged (have at least a type label) +- Processed 10 issues in one iteration +- Encountered an issue that requires human judgment (skip and note in STATE.md) + +## State Recording + +After each iteration, update `STATE.md` "Issue Triage" section with: +- Date, issues triaged, labels applied, any issues needing human attention + +## Safety Constraints + +- Never close issues +- Never promise timelines +- Never tag or release +- All responses in English +- For quic-go related issues: apply `quic-go` label, note modified-code caveat in response diff --git a/.codebuddy/agents/pr-reviewer.md b/.codebuddy/agents/pr-reviewer.md new file mode 100644 index 00000000..c270af24 --- /dev/null +++ b/.codebuddy/agents/pr-reviewer.md @@ -0,0 +1,45 @@ +--- +name: pr-reviewer +description: PR review agent. Reviews pull requests with special caution for modified stdlib, quic-go, and HTTP/2 code. Posts review comments via gh. Used proactively on new PRs. +tools: Read, Bash, Grep +model: inherit +skills: pr-review, gh-cli +--- + +You are the PR review agent for the req project. + +## Your Responsibilities + +Each time you are called, execute one PR review iteration: + +1. **Discover** — list open PRs + ```bash + gh pr list -R imroc/req --state open --limit 10 --json number,title,author,files + ``` +2. **Plan** — identify PRs that need review (no review yet or recently updated) +3. **Execute** — fetch PR diff, run review checklist (see pr-review skill) +4. **Verify** — for modified-code PRs, double-check upstream compatibility +5. **Iterate** — post review, move to next PR + +## Stop Conditions + +- All open PRs have been reviewed +- Reviewed 5 PRs in one iteration +- Encountered a PR requiring human judgment (skip and note in STATE.md) + +## Special Caution + +- **quic-go PRs** (touching `internal/http3/`): never auto-approve. Always `--request-changes` or `--comment` with specific concerns about modified-code compatibility. Note target quic-go version. +- **Modified stdlib PRs** (root files with Go Authors copyright): verify req customizations preserved. +- **HTTP/2 PRs** (touching `internal/http2/`): verify upstream golang.org/x/net/http2 compatibility. + +## State Recording + +After each iteration, update `STATE.md` "PR Review" section. + +## Safety Constraints + +- Never approve PRs that include `git tag` or `gh release create` +- Never approve PRs that delete or weaken tests +- Never tag or release +- All reviews in English diff --git a/.codebuddy/agents/upstream-tracker.md b/.codebuddy/agents/upstream-tracker.md new file mode 100644 index 00000000..bf01f8d8 --- /dev/null +++ b/.codebuddy/agents/upstream-tracker.md @@ -0,0 +1,38 @@ +--- +name: upstream-tracker +description: Upstream sync tracking agent. Monitors Go stdlib net/http, golang.org/x/net/http2, and quic-go for changes that need manual sync into req's modified code. Generates reports. Used proactively for upstream tracking. +tools: Read, Bash, Grep, WebFetch +model: inherit +skills: upstream-sync, gh-cli +--- + +You are the upstream sync tracking agent for the req project. + +## Your Responsibilities + +Each time you are called, execute one upstream tracking iteration: + +1. **Discover** — determine current upstream baselines from git log and go.mod +2. **Plan** — identify which upstream sources to check (Go stdlib, http2, quic-go) +3. **Execute** — fetch upstream commit/release history, compare with baselines +4. **Verify** — identify which changes affect req's modified files +5. **Iterate** — generate a sync report, open an issue if significant changes found + +## Stop Conditions + +- Report generated for all three upstream sources +- Found a critical security fix or breaking change that needs immediate attention + +## Output + +Generate a report and save to STATE.md "Upstream Sync" section. If breaking changes or security fixes are found, open an issue: +```bash +gh issue create -R imroc/req --title "upstream: " --body "" --label "upstream,quic-go" +``` + +## Safety Constraints + +- Never modify files in `internal/http3/`, `internal/http2/`, or root modified stdlib files +- Only track and report — sync is manual +- Never tag or release +- All reports in English diff --git a/.codebuddy/commands/loop-status.md b/.codebuddy/commands/loop-status.md new file mode 100644 index 00000000..f044ad47 --- /dev/null +++ b/.codebuddy/commands/loop-status.md @@ -0,0 +1,12 @@ +--- +description: Show status of all loop engineering maintenance loops +allowed-tools: Read, Bash +--- + +# Loop Status + +Show the status of all maintenance loops. + +Use CronList to list all active cron jobs. +Also read STATE.md to show the last run result of each loop. +Display a summary table of: loop name, schedule, last run date, last result. diff --git a/.codebuddy/commands/start-loops.md b/.codebuddy/commands/start-loops.md new file mode 100644 index 00000000..dcd27aff --- /dev/null +++ b/.codebuddy/commands/start-loops.md @@ -0,0 +1,37 @@ +--- +description: Start all loop engineering maintenance loops for the req project +allowed-tools: Read, Bash, Grep, Edit, WebFetch +--- + +# Start Loop Engineering + +Start all maintenance loops for the req project using CodeBuddy's built-in cron. + +## Loops to Start + +Use the CronCreate tool to create the following scheduled tasks: + +### 1. CI Fix Loop (every 15 minutes) +- Schedule: every 15 minutes +- Prompt: "Use the ci-fixer agent to check for failed CI runs with `gh run list --status failure --limit 3 --workflow ci.yml -R imroc/req`. If there are failures, use gh run view to get logs, apply ci-triage skill to classify, fix auto-fixable issues on a branch fix/ci-, run go build && go vet && go test, then use code-reviewer agent to review. If approved, create PR with gh pr create. Update STATE.md." + +### 2. Issue Triage Loop (every 2 hours) +- Schedule: every 2 hours +- Prompt: "Use the issue-triager agent to triage recent issues. Use `gh issue list -R imroc/req --state open --limit 10` to find untriaged issues. For each, fetch with gh issue view, classify, apply labels with gh issue edit --add-label, post initial response if needed. Flag quic-go related issues with extra caution. Update STATE.md." + +### 3. PR Review Loop (every 1 hour) +- Schedule: every 1 hour +- Prompt: "Use the pr-reviewer agent to review open PRs. Use `gh pr list -R imroc/req --state open --limit 5` to find unreviewed PRs. For each, fetch diff with gh pr diff, apply pr-review skill checklist. Post review with gh pr review. Be extra cautious with PRs touching internal/http3/ or internal/http2/. Update STATE.md." + +### 4. Dependency Upgrade Loop (daily) +- Schedule: daily at 03:00 +- Prompt: "Use the dependency-upgrader agent to check for dependency upgrades. Run `go list -u -m all` to find upgradable deps. Upgrade non-sensitive deps with go get -u, run go test ./... after. For sensitive deps (utls, x/net, x/crypto, x/text), upgrade individually and test. Do NOT touch modified code in internal/http3/ or internal/http2/. Create PR with gh pr create if tests pass. Update STATE.md." + +### 5. Upstream Sync Tracking Loop (daily) +- Schedule: daily at 02:00 +- Prompt: "Use the upstream-tracker agent to check upstream changes. Check Go stdlib net/http, golang.org/x/net/http2, and quic-go for new releases/commits since last sync. Generate a sync report. If breaking changes found, open an issue with gh issue create. Update STATE.md upstream sync baselines." + +## Execution + +For each loop above, use CronCreate with the appropriate schedule and prompt. +After creating all loops, list them with CronList to confirm. diff --git a/.codebuddy/commands/stop-loops.md b/.codebuddy/commands/stop-loops.md new file mode 100644 index 00000000..cf03f981 --- /dev/null +++ b/.codebuddy/commands/stop-loops.md @@ -0,0 +1,11 @@ +--- +description: Stop all loop engineering maintenance loops +allowed-tools: Read, Bash +--- + +# Stop Loop Engineering + +Stop all running maintenance loops. + +Use CronList to list all active cron jobs, then use CronDelete to delete each one. +After deleting all, confirm with CronList that none remain. diff --git a/.codebuddy/loop-engineering.md b/.codebuddy/loop-engineering.md new file mode 100644 index 00000000..4e1e9a6d --- /dev/null +++ b/.codebuddy/loop-engineering.md @@ -0,0 +1,215 @@ +# Loop Engineering Design + +This project uses Loop Engineering for automated maintenance. Core idea: shift from "prompting AI step by step" to "designing self-running loops". Humans become rule-makers, AI systems run autonomously to achieve goals. + +## Four-Layer Architecture + +| Layer | Purpose | Implementation | +|-------|---------|---------------| +| Prompt | How to ask | Each agent's system prompt | +| Context | What AI sees | `CODEBUDDY.md`, `STATE.md` | +| Harness | AI work environment | skills (project knowledge), allowed-tools (permission constraints) | +| Loop | What to do after each step | System cron + `codebuddy -p` (headless), or GitHub Actions | + +## Execution Mode + +This project supports two execution modes. **System cron is the primary mode** for environments where the CodeBuddy API is only reachable from an internal network (e.g. TKE private deployment). + +### Mode 1: System Cron + Headless (Primary) + +Each loop runs as a system cron job that invokes `codebuddy -p` (headless mode). Every run is a fresh, isolated session that reads `STATE.md` for context, executes the loop, and writes back `STATE.md`. + +**Advantages:** +- No expiration — runs permanently, no need to re-create +- No need to keep a session open +- Each run is isolated (fresh context, no drift) +- Works with TKE internal API (runs locally) + +**Start (also use on container/host restart):** +```bash +cd +.codebuddy/scripts/start-loops.sh # starts cron daemon + installs crontab +``` + +**Install crontab only (if cron daemon already running):** +```bash +.codebuddy/scripts/install-cron.sh +``` + +**Uninstall:** +```bash +.codebuddy/scripts/install-cron.sh --remove +``` + +**Container restart:** +On container or host restart, cron daemon may not be running. Either: +1. Run `start-loops.sh` manually after restart, or +2. Add `start-loops.sh` to shell profile (`~/.bashrc`) for auto-start on login, or +3. Add `start-loops.sh` to container entrypoint for fully automatic startup + +**Manual trigger:** +```bash +.codebuddy/scripts/run-loop.sh +# loop-type: ci-fix | issue-triage | pr-review | dependency-upgrade | upstream-sync +``` + +**View logs:** +```bash +ls $LOOP_LOG_DIR # default: /tmp/loop-logs/ +cat $LOOP_LOG_DIR/ci-fix-*.log +``` + +**Schedule (daily, Beijing time, staggered to avoid API rate limits):** + +| Time | Loop | Script | +|------|------|--------| +| 01:00 | upstream-sync | `run-loop.sh upstream-sync` | +| 02:00 | dependency-upgrade | `run-loop.sh dependency-upgrade` | +| 03:00 | ci-fix | `run-loop.sh ci-fix` | +| 04:00 | issue-triage | `run-loop.sh issue-triage` | +| 05:00 | pr-review | `run-loop.sh pr-review` | + +**Run flow:** +``` +system cron (daily, e.g. 03:00) + → run-loop.sh ci-fix + → git pull --ff-only origin master # sync latest + → codebuddy -p -y "..." # headless session + → read STATE.md # get last state + → execute loop logic (via agent) # discover → plan → execute → verify + → write STATE.md # persist state + → git push # push state changes + → 10 min timeout protection + → log to $LOOP_LOG_DIR (default: /tmp/loop-logs/) +``` + +### Mode 2: GitHub Actions (Optional, for public API) + +For environments with a public CodeBuddy API endpoint. Gated behind `CODEBUDDY_ENABLED` repo variable (default: off). + +**Enable:** +```bash +# Set repo variable +gh variable set CODEBUDDY_ENABLED -R imroc/req -b "true" +# Set API key secret +gh secret set CODEBUDDY_API_KEY -R imroc/req +``` + +**Disable:** +```bash +gh variable set CODEBUDDY_ENABLED -R imroc/req -b "false" +``` + +Workflows are in `.github/workflows/*-loop.yml`. + +### Mode 3: Interactive Session (For Ad-hoc Use) + +You can also trigger loops directly in an interactive CodeBuddy session: + +``` +> Use the ci-fixer agent to check for failed CI runs +> Use the issue-triager agent to triage recent issues +> Use the pr-reviewer agent to review PR #485 +> Use the dependency-upgrader agent to upgrade dependencies +> Use the upstream-tracker agent to check upstream changes +``` + +Slash commands (session-level, expire in 3 days): +- `/start-loops` — create all 5 loops as session cron jobs +- `/stop-loops` — delete all session cron jobs +- `/loop-status` — show loop status and last run results + +## Implemented Loops + +### 1. Upstream Sync Tracking (daily 01:00) +- **Goal**: Track Go stdlib net/http, golang.org/x/net/http2, and quic-go upstream changes +- **Five phases**: Discover (check baselines) → Fetch upstream changes → Identify affected files → Generate report → Open issue if needed +- **Stop conditions**: Report generated for all 3 sources / Critical change found +- **Files**: `skills/upstream-sync/SKILL.md`, `agents/upstream-tracker.md` +- **Note**: Only tracks and reports. Sync of modified code is manual human work. + +### 2. Dependency Upgrade (daily 02:00) +- **Goal**: Safely upgrade all upgradable Go dependencies (non-modified-code only) +- **Five phases**: Discover (`go list -u`) → Risk-classify → Upgrade → Test → Rollback-retry +- **Stop conditions**: All deps processed / 3 consecutive test failures / 5 iterations max +- **Files**: `skills/dependency-upgrade/SKILL.md`, `agents/dependency-upgrader.md` +- **Separation**: dependency-upgrader executes → code-reviewer reviews → PR only after approval + +### 3. CI Fix (daily 03:00) +- **Goal**: Auto-fix fixable CI failures +- **Five phases**: Discover (`gh run list`) → Classify (ci-triage) → Fix (ci-fix) → Local verify → Retry +- **Stop conditions**: All fixable failures resolved / 3 fix attempts / No failed CI runs +- **Files**: `skills/ci-triage/SKILL.md`, `skills/ci-fix/SKILL.md`, `agents/ci-fixer.md`, `agents/code-reviewer.md` +- **Separation**: ci-fixer fixes → code-reviewer reviews → PR only after approval + +### 4. Issue Triage (daily 04:00) +- **Goal**: Classify, label, and respond to issues; flag quic-go/modified-code issues for caution +- **Five phases**: Discover (untriaged issues) → Classify → Apply labels → Post response → Verify +- **Stop conditions**: All recent issues triaged / 10 issues processed / Needs human judgment +- **Files**: `skills/issue-triage/SKILL.md`, `agents/issue-triager.md` +- **Special**: quic-go issues get `quic-go` label and modified-code caveat note + +### 5. PR Review (daily 05:00) +- **Goal**: Review PRs with special caution for modified stdlib, quic-go, and HTTP/2 code +- **Five phases**: Discover (open PRs) → Fetch diff → Review checklist → Post review → Verify +- **Stop conditions**: All open PRs reviewed / 5 PRs reviewed / Needs human judgment +- **Files**: `skills/pr-review/SKILL.md`, `agents/pr-reviewer.md` +- **Special**: PRs touching `internal/http3/` never auto-approved; quic-go version compatibility required + +## State Management + +`STATE.md` (project root) is the shared state file for all loops. This is the core Loop Engineering principle — **state is external, not in model context**. Each cron run reads `STATE.md` at the start and writes back at the end. + +## Cost Control + +- Each loop runs once daily (not always-on), 10 min max per run +- Staggered schedule (01:00–05:00) to avoid API rate limits +- Headless mode (`-p`) uses minimal tokens per run +- 30-day log retention (older logs auto-deleted) + +## Autonomy Boundary + +- **Loops can make daily maintenance decisions autonomously**: upgrade deps, fix CI, triage issues, review PRs — no step-by-step human confirmation needed +- **No tagging or releasing**: `git tag`, `gh release create` are always human decisions. Automation stops at PR creation. +- code-reviewer and pr-reviewer agents block any PR that includes release operations +- All commit messages in English + +## How to Add a New Loop + +1. Create a skill directory and `SKILL.md` under `.codebuddy/skills/` (encode project knowledge) +2. Create an agent under `.codebuddy/agents/` (define responsibilities, stop conditions, tool permissions) +3. Add a case to `.codebuddy/scripts/run-loop.sh` with the loop's prompt and tools +4. Add a cron entry in `.codebuddy/scripts/install-cron.sh` +5. Add a section to `STATE.md` +6. Reuse `code-reviewer` agent for implementation/review separation +7. Agents involving GitHub operations must declare `gh-cli` in `skills` field +8. Agent safety constraints must include no-tag/no-release rule +9. All skills must be project-level (under `.codebuddy/skills/`), never global +10. Set explicit max iteration limits (turn limits) to prevent infinite loops +11. All commit messages in English + +## Inner Loop vs Outer Loop + +This project follows the two-layer loop model from Loop Engineering: + +- **Inner Loop** = each agent's ReAct-style execution (think → act → observe within one task) +- **Outer Loop** = the orchestration layer that manages task lifecycle (Discover → Plan → Execute → Verify → Iterate) + +The code-reviewer agent provides **adversarial verification** — a different agent/perspective reviews the executor's work, avoiding the self-confirmation bias of single-agent self-review. + +## Cognitive Surrender Risk + +Loop Engineering can accelerate "understanding debt" — the gap between the codebase's real state and the developer's mental model grows as AI makes more changes autonomously. Mitigations: + +- Regularly review AI-generated changes (diff review before merge) +- Maintain STATE.md as a shared source of truth +- Treat AI as a collaborator, not authority — question every change +- Key changes (quic-go port, stdlib sync, security fixes) always require human review + +## Future Expandable Loops + +| Loop | Value | +|------|-------| +| Test coverage improvement | Find low-coverage files, add tests | +| CHANGELOG generation | Summarize changes, generate release notes | +| Security scan | Check known vulnerable dependencies | diff --git a/.codebuddy/scripts/install-cron.sh b/.codebuddy/scripts/install-cron.sh new file mode 100755 index 00000000..e9752de1 --- /dev/null +++ b/.codebuddy/scripts/install-cron.sh @@ -0,0 +1,73 @@ +#!/usr/bin/env bash +# +# Install cron job for Loop Engineering +# Runs all 5 loops sequentially once daily at 01:00 (Beijing time, UTC+8). +# +# The crontab is written to a persistent path ($HOME/.local/share/loop-crontab) +# so it survives container restarts. rc.local loads it on boot. +# +# Usage: ./install-cron.sh +# To uninstall: ./install-cron.sh --remove + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +RUN_SCRIPT="${SCRIPT_DIR}/run-loop.sh" +MARKER="# loop-engineering-req" +PERSIST_DIR="${HOME}/.local/share" +PERSIST_FILE="${PERSIST_DIR}/loop-crontab" + +# Remove existing loop cron jobs from current crontab +remove_existing() { + crontab -l 2>/dev/null | grep -v "$MARKER" | crontab - 2>/dev/null || true + rm -f "$PERSIST_FILE" + echo "Removed loop cron jobs." +} + +if [[ "${1:-}" == "--remove" ]]; then + remove_existing + exit 0 +fi + +# Check script exists +if [[ ! -x "$RUN_SCRIPT" ]]; then + echo "Error: $RUN_SCRIPT not found or not executable" + exit 1 +fi + +# Check codebuddy is in PATH +if ! command -v codebuddy &>/dev/null; then + echo "Error: codebuddy not found in PATH" + exit 1 +fi + +# Check gh is authenticated +if ! gh auth status &>/dev/null 2>&1; then + echo "Error: gh not authenticated. Run: gh auth login" + exit 1 +fi + +remove_existing + +# Build the crontab content: keep existing non-loop entries + add loop entry +EXISTING=$(crontab -l 2>/dev/null | grep -v "$MARKER" || true) +CRONTAB_CONTENT="${EXISTING} +${MARKER} +0 1 * * * ${RUN_SCRIPT} all ${MARKER}" + +# Write to persistent path (survives container restart, loaded by rc.local) +mkdir -p "$PERSIST_DIR" +echo "$CRONTAB_CONTENT" > "$PERSIST_FILE" + +# Apply immediately +echo "$CRONTAB_CONTENT" | crontab - + +echo "Installed loop cron job (daily, Beijing time):" +echo " 01:00 all (upstream-sync -> dependency-upgrade -> ci-fix -> issue-triage -> pr-review)" +echo "" +echo "Persistent crontab: $PERSIST_FILE (auto-loaded by rc.local on container restart)" +echo "Logs: \$LOOP_LOG_DIR (default: /tmp/loop-logs/)" +echo "Uninstall: ${SCRIPT_DIR}/install-cron.sh --remove" +echo "" +echo "Current crontab:" +crontab -l | grep "$MARKER" diff --git a/.codebuddy/scripts/pre-push-check.sh b/.codebuddy/scripts/pre-push-check.sh new file mode 100755 index 00000000..23ea1129 --- /dev/null +++ b/.codebuddy/scripts/pre-push-check.sh @@ -0,0 +1,61 @@ +#!/usr/bin/env bash +# +# Pre-push test gate — runs full test suite before allowing push. +# Install: ln -s .codebuddy/scripts/pre-push-check.sh .git/hooks/pre-push +# Or add to .codebuddy/scripts/install-cron.sh to install hook. +# +# Exits non-zero if any check fails, blocking the push. + +set -euo pipefail + +REPO_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +cd "$REPO_DIR" + +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' + +echo -e "${YELLOW}=== Pre-push test gate ===${NC}" + +# 1. go build +echo -e "${YELLOW}Running go build ./...${NC}" +if ! go build ./... 2>&1; then + echo -e "${RED}FAIL: go build${NC}" + exit 1 +fi +echo -e "${GREEN}go build: OK${NC}" + +# 2. go vet +echo -e "${YELLOW}Running go vet ./...${NC}" +if ! go vet ./... 2>&1; then + echo -e "${RED}FAIL: go vet${NC}" + exit 1 +fi +echo -e "${GREEN}go vet: OK${NC}" + +# 3. go test (short mode for speed, full mode can be run separately) +echo -e "${YELLOW}Running go test ./... -short${NC}" +if ! go test ./... -short -timeout 120s 2>&1; then + echo -e "${RED}FAIL: go test${NC}" + exit 1 +fi +echo -e "${GREEN}go test: OK${NC}" + +# 4. go mod tidy check (only if go.mod changed) +if git diff --cached --name-only HEAD 2>/dev/null | grep -q "go.mod\|go.sum"; then + echo -e "${YELLOW}Checking go mod tidy...${NC}" + cp go.mod /tmp/go.mod.bak + cp go.sum /tmp/go.sum.bak + go mod tidy + if ! diff -q go.mod /tmp/go.mod.bak >/dev/null || ! diff -q go.sum /tmp/go.sum.bak >/dev/null; then + echo -e "${RED}FAIL: go mod tidy produced changes. Run 'go mod tidy' and commit.${NC}" + mv /tmp/go.mod.bak go.mod + mv /tmp/go.sum.bak go.sum + exit 1 + fi + echo -e "${GREEN}go mod tidy: OK${NC}" +fi + +echo -e "${GREEN}=== All checks passed ===${NC}" +exit 0 diff --git a/.codebuddy/scripts/run-loop.sh b/.codebuddy/scripts/run-loop.sh new file mode 100755 index 00000000..86834dec --- /dev/null +++ b/.codebuddy/scripts/run-loop.sh @@ -0,0 +1,88 @@ +#!/usr/bin/env bash +# +# Loop Engineering Runner — called by system cron +# Each invocation starts a fresh codebuddy headless session, +# reads STATE.md for context, executes the loop, writes back STATE.md. +# +# Usage: ./run-loop.sh +# loop_type: ci-fix | issue-triage | pr-review | dependency-upgrade | upstream-sync | all +# +# When "all" is specified, runs all 5 loops sequentially: +# upstream-sync -> dependency-upgrade -> ci-fix -> issue-triage -> pr-review +# +# Crontab example (Beijing time, UTC+8): +# 0 1 * * * /.codebuddy/scripts/run-loop.sh all + +set -euo pipefail + +# cron has a minimal PATH — set full PATH so codebuddy, gh, go, etc. are found +export HOME="${HOME:-/root}" +export PATH="${HOME}/.linuxbrew/bin:${HOME}/.linuxbrew/sbin:/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin:${HOME}/.bin:${HOME}/.local/bin:${HOME}/.cargo/bin:${HOME}/go/bin:${HOME}/.krew/bin:${HOME}/.fzf/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" + +# Also load Cargo, Go env if available +export GOPATH="${GOPATH:-${HOME}/go}" +[ -f "${HOME}/.cargo/env" ] && . "${HOME}/.cargo/env" 2>/dev/null || true + +LOOP_TYPE="${1:?Usage: run-loop.sh }" +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_DIR="$(cd "$SCRIPT_DIR/../.." && pwd)" +LOG_DIR="${LOOP_LOG_DIR:-/tmp/loop-logs}" +mkdir -p "$LOG_DIR" + +cd "$REPO_DIR" + +# "all" mode: run all loops sequentially +if [[ "$LOOP_TYPE" == "all" ]]; then + echo "=== Running all loops sequentially at $(date) ===" + for lt in upstream-sync dependency-upgrade ci-fix issue-triage pr-review; do + echo "--- Starting: $lt ---" + "$0" "$lt" || echo "WARNING: $lt exited with error, continuing..." + echo "--- Finished: $lt ---" + sleep 10 # brief pause between loops to avoid API rate limits + done + echo "=== All loops completed at $(date) ===" + exit 0 +fi + +TIMESTAMP=$(date +%Y%m%d-%H%M%S) +LOG_FILE="$LOG_DIR/${LOOP_TYPE}-${TIMESTAMP}.log" + +# Pull latest master before running +git pull --ff-only origin master >> "$LOG_FILE" 2>&1 || true + +case "$LOOP_TYPE" in + ci-fix) + PROMPT='Check for failed CI runs: gh run list --status failure --limit 3 --workflow ci.yml -R imroc/req. If there are failures, for each: use gh run view --log-failed to get logs, analyze the failure, and fix auto-fixable issues. Create a branch fix/ci-, make the fix, run go build ./... && go vet ./... && go test ./... to verify. If tests pass, create a PR with gh pr create. If not fixable, note it. Finally, update the "CI Fix Loop" section in STATE.md with the date and result, then commit and push STATE.md to master.' + ;; + issue-triage) + PROMPT='Triage recent issues: run gh issue list -R imroc/req --state open --limit 10 --json number,title,labels,createdAt. For issues without labels, fetch details with gh issue view, classify as bug/enhancement/security/question/performance, and apply labels with gh issue edit --add-label. For quic-go or HTTP/3 related issues, add the quic-go label. Post a brief initial response if appropriate. Finally, update the "Issue Triage Loop" section in STATE.md with the date and result, then commit and push STATE.md to master.' + ;; + pr-review) + PROMPT='Review open PRs: run gh pr list -R imroc/req --state open --limit 5 --json number,title,files. For each PR, fetch diff with gh pr diff and review the changes. Pay extra attention to PRs touching internal/http3/ or internal/http2/ — these are modified upstream code and need careful review. Post review comments with gh pr review. Finally, update the "PR Review Loop" section in STATE.md with the date and result, then commit and push STATE.md to master.' + ;; + dependency-upgrade) + PROMPT='Check for dependency upgrades: run go list -u -m all and look for upgradable modules. Upgrade non-sensitive deps with go get -u ./... && go mod tidy. For sensitive deps (utls, x/net, x/crypto, x/text), upgrade individually and test. Do NOT modify files in internal/http3/ or internal/http2/ — those are modified upstream code. Run go build ./... && go vet ./... && go test ./... after upgrade. If tests pass, create branch chore/upgrade-deps-, commit, and create PR with gh pr create. Finally, update the "Dependency Upgrade Loop" section in STATE.md with the date and result, then commit and push STATE.md to master.' + ;; + upstream-sync) + PROMPT='Check upstream changes: read STATE.md for current baselines (Go stdlib net/http, golang.org/x/net/http2, quic-go). Check https://github.com/quic-go/quic-go/releases for new quic-go releases. Check https://github.com/golang/go/commits/master/src/net/http for stdlib changes. Check https://github.com/golang/net/commits/master/http2 for http2 changes. Generate a sync report summarizing what changed. If breaking changes or security fixes are found, open an issue with gh issue create. Finally, update the "Upstream Sync Tracking Loop" section in STATE.md with new baselines, then commit and push STATE.md to master.' + ;; + *) + echo "Unknown loop type: $LOOP_TYPE" | tee -a "$LOG_FILE" + echo "Valid types: ci-fix, issue-triage, pr-review, dependency-upgrade, upstream-sync, all" + exit 1 + ;; +esac + +echo "=== Loop: $LOOP_TYPE at $(date) ===" >> "$LOG_FILE" + +# Run codebuddy in headless mode (-y bypasses permissions for non-interactive use) +timeout 600 codebuddy -p -y \ + "$PROMPT" >> "$LOG_FILE" 2>&1 || true + +EXIT_CODE=$? +echo "=== Exit code: $EXIT_CODE at $(date) ===" >> "$LOG_FILE" + +# Keep only last 30 days of logs +find "$LOG_DIR" -name "*.log" -mtime +30 -delete 2>/dev/null || true + +exit $EXIT_CODE diff --git a/.codebuddy/scripts/start-loops.sh b/.codebuddy/scripts/start-loops.sh new file mode 100755 index 00000000..ac00ea7a --- /dev/null +++ b/.codebuddy/scripts/start-loops.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash +# +# Start Loop Engineering — call this on container/host startup +# Starts cron daemon (if not running) and loads persistent crontab. +# +# Usage: .codebuddy/scripts/start-loops.sh + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PERSIST_FILE="${HOME}/.local/share/loop-crontab" + +# 1. Ensure cron daemon is running +if ! pgrep -x cron >/dev/null 2>&1; then + echo "Starting cron daemon..." + if command -v systemctl &>/dev/null; then + systemctl start cron 2>/dev/null || systemctl start crond 2>/dev/null || cron -f & + elif command -v crond &>/dev/null; then + crond + else + echo "Error: cannot start cron daemon" >&2 + exit 1 + fi + echo "Cron daemon started." +else + echo "Cron daemon already running." +fi + +# 2. Load persistent crontab if it exists +if [ -f "$PERSIST_FILE" ]; then + echo "Loading crontab from $PERSIST_FILE..." + crontab "$PERSIST_FILE" + echo "Crontab loaded." +else + echo "No persistent crontab found at $PERSIST_FILE" + echo "Run: ${SCRIPT_DIR}/install-cron.sh to create one." +fi + +# 3. Verify +echo "" +echo "Loop Engineering status:" +echo " Cron daemon: $(pgrep -x cron >/dev/null && echo 'running' || echo 'NOT running')" +echo " Crontab:" +crontab -l 2>/dev/null | grep "loop-engineering-req" || echo " (no loop entries found)" diff --git a/.codebuddy/skills/ci-fix/SKILL.md b/.codebuddy/skills/ci-fix/SKILL.md new file mode 100644 index 00000000..aad39387 --- /dev/null +++ b/.codebuddy/skills/ci-fix/SKILL.md @@ -0,0 +1,45 @@ +--- +name: ci-fix +description: CI 失败修复专家。根据分类结果修复编译错误、测试失败、依赖问题。在 CI 失败需要修复时主动调用。 +allowed-tools: Read, Edit, Bash, Grep +--- + +# CI 失败修复专家 + +你负责根据分类结果修复 CI 失败，然后本地验证。 + +## 修复原则 + +1. **最小改动** — 只修复 CI 失败直接相关的问题，不顺手重构 +2. **保持意图** — 修复测试失败时保留测试原本的验证意图 +3. **风格一致** — 与现有代码风格保持一致 +4. **本地验证** — 修复后必须本地跑通 `go test ./...` + +## 工作流程 + +1. **读取分类结果**（来自 ci-triage skill 的输出） +2. **逐个修复** auto_fixable 的失败： + - 编译错误 → 修正语法、补 import、修类型 + - 依赖问题 → `go mod tidy`、修正版本 + - vet 错误 → 按 vet 提示修正 + - 测试失败 → 分析断言，修复代码或测试（优先修代码） +3. **本地验证** + ```bash + go build ./... + go vet ./... + go test ./... -coverprofile=coverage.txt + ``` +4. **提交**到新分支 `fix/ci--` +5. **输出修复报告**：改了哪些文件、怎么改的、测试结果 + +## 停止条件 + +- 所有 auto_fixable 失败已修复且本地测试通过 +- 修复尝试 3 次仍失败 → 停止，记录到 STATE.md 待人工处理 + +## 禁止 + +- 不修复 `not_fixable` 的问题 +- 不提交未通过本地测试的修改 +- 不使用 `--no-verify` 跳过检查 +- 不为了通过测试而删除或注释掉失败的测试 diff --git a/.codebuddy/skills/ci-triage/SKILL.md b/.codebuddy/skills/ci-triage/SKILL.md new file mode 100644 index 00000000..1f0bf76f --- /dev/null +++ b/.codebuddy/skills/ci-triage/SKILL.md @@ -0,0 +1,60 @@ +--- +name: ci-triage +description: CI 失败分类专家。分析 GitHub Actions 失败日志，确定根因、修复难度、是否可自动修复。在 CI 失败需要分析时主动调用。 +allowed-tools: Read, Bash, Grep, WebFetch +--- + +# CI 失败分类专家 + +你负责分析 CI 失败，输出结构化的分类结果，供修复代理使用。 + +## 输入 + +CI 失败日志（通过 `gh run view` 或直接提供的日志文本）。 + +## 工作流程 + +1. **获取失败日志** + ```bash + # 列出最近失败的 CI run + gh run list --status failure --limit 5 --workflow ci.yml + # 查看具体失败日志 + gh run view --log-failed + ``` + +2. **分类失败类型**： + | 类型 | 示例 | 可自动修复 | + |------|------|:---:| + | 编译错误 | 语法错误、类型不匹配、缺失导入 | 是 | + | 测试失败 | 断言失败、panic、超时 | 是（若明确） | + | 依赖问题 | go.mod 冲突、模块缺失、版本不兼容 | 是 | + | vet/lint 错误 | go vet 报告的问题 | 是 | + | 环境问题 | Go 版本、runner 问题 | 否 | + | 间歇性失败 | 网络超时、flaky test | 否（标记重跑） | + +3. **根因分析**：定位到具体文件和行号 + +4. **输出格式**（JSON） + ```json + { + "run_id": "123456", + "failures": [ + { + "type": "compile_error", + "root_cause": "undefined: xxx in foo.go:42", + "file_path": "internal/foo.go", + "line_number": 42, + "difficulty": "easy", + "auto_fixable": true, + "suggested_fix": "添加缺失的 import" + } + ], + "not_fixable": ["环境问题：Go 1.25 runner 不可用"] + } + ``` + +## 规则 + +- 只分析，不修改代码 +- 保守判断 `auto_fixable`：不确定的标记为 false +- 间歇性失败不自动修复，建议重跑 diff --git a/.codebuddy/skills/dependency-upgrade/SKILL.md b/.codebuddy/skills/dependency-upgrade/SKILL.md new file mode 100644 index 00000000..22801168 --- /dev/null +++ b/.codebuddy/skills/dependency-upgrade/SKILL.md @@ -0,0 +1,64 @@ +--- +name: dependency-upgrade +description: Go 依赖升级专家。分析过时依赖、执行升级、运行测试验证、生成升级报告。用于定期自动升级项目依赖时主动调用。 +allowed-tools: Read, Bash, Grep, Edit +--- + +# Go 依赖升级专家 + +你负责安全地升级本 Go 项目的依赖。 + +## 重要：魔改架构说明 + +本项目魔改了 Go 标准库 net/http、golang.org/x/net/http2 和 quic-go，源码内置在项目中： +- 根目录（`transport.go`、`transfer.go`、`textproto_reader.go` 等）— 魔改自 Go 标准库 net/http +- `internal/http2/` — 魔改自 golang.org/x/net/http2 +- `internal/http3/` — 魔改自 quic-go + +**这些魔改代码不能用 `go get -u` 升级**，需要人工比对上游 diff 同步。本 skill 只处理 go.mod 中声明的常规依赖。魔改代码的上游同步是独立的人工流程，不在自动升级范围内。 + +## 工作流程 + +1. **检查当前状态** + ```bash + git status --short + git pull --ff-only origin master + ``` + +2. **发现可升级的常规依赖** + ```bash + go list -u -m all 2>/dev/null | grep '\[' + ``` + 输出格式：`module current-version [latest-version]`，带 `[` 的行表示有更新。 + +3. **评估升级风险**，分类处理： + - **安全升级**（patch/minor，无 breaking change）：直接升级 + - **需谨慎升级**（major 版本，或 utls、x/net、x/crypto、x/text 等敏感依赖）：升级后必须跑完整测试，失败则回滚该依赖 + - **跳过**：预发布版本（含 `-alpha`、`-beta`、`-rc`） + - **不处理**：魔改代码（internal/http2/、internal/http3/、根目录标准库魔改文件）的上游同步 + +4. **分批升级并验证**（核心策略：逐个升级关键依赖，整批升级常规依赖） + - 对每个敏感依赖单独执行 `go get @latest`，然后跑 `go test ./...` + - 常规依赖批量 `go get -u ./... && go mod tidy`，再跑一次完整测试 + - 任何测试失败 → `git checkout go.mod go.sum` 回滚，记录失败原因到 STATE.md + +5. **验证清单**（全部通过才算成功） + - [ ] `go build ./...` 成功 + - [ ] `go vet ./...` 无新增问题 + - [ ] `go test ./... -coverprofile=coverage.txt` 全部通过 + - [ ] `go mod tidy` 后 go.sum 无残留变化 + +6. **输出升级报告**，包含： + - 升级的依赖列表（旧版本 → 新版本） + - 跳过的依赖及原因 + - 测试结果 + - 如有失败回滚，记录原因 + - **提醒**：魔改代码的上游同步需人工处理，报告中附上当前 quic-go 和标准库的版本基线 + +## 规则 + +- 只在 master 分支基础上创建新分支 `chore/upgrade-deps-` +- 只升级 go.mod 中声明的常规依赖（go.mod / go.sum），不碰魔改源码文件 +- 如果升级后测试失败，优先回滚而非修改代码适配 +- 敏感依赖（utls、x/net、x/crypto、x/text）升级时单独验证 +- 提交信息格式：`chore: upgrade dependencies (<升级概要>)` diff --git a/.codebuddy/skills/issue-triage/SKILL.md b/.codebuddy/skills/issue-triage/SKILL.md new file mode 100644 index 00000000..269f957a --- /dev/null +++ b/.codebuddy/skills/issue-triage/SKILL.md @@ -0,0 +1,59 @@ +--- +name: issue-triage +description: Issue classification and labeling expert. Analyzes new issues, applies labels, identifies quic-go/modified-code related issues for cautious handling, posts initial responses. Used proactively when issues need triage. +allowed-tools: Read, Bash, Grep, WebFetch +--- + +# Issue Triage Expert + +You classify and label incoming issues for the req project. + +## Workflow + +1. **Fetch issue details** + ```bash + gh issue view -R imroc/req --json title,body,labels,author,comments + ``` + +2. **Classify by type** and apply labels: + - `bug` — defect or unexpected behavior + - `enhancement` — feature request + - `security` — security vulnerability (high priority) + - `question` — usage question, not a bug + - `performance` — performance-related issue + - `documentation` — docs issue + +3. **Identify sensitive areas** — apply extra labels: + - `quic-go` — issue involves `internal/http3/`, HTTP/3, or quic-go upstream. **Requires cautious review** — contributors may not understand the modified code implications + - `http2` — issue involves `internal/http2/` or HTTP/2 + - `modified-stdlib` — issue involves root-level modified stdlib files (`transport.go`, `transfer.go`, `textproto_reader.go`, etc.) + - `tls-fingerprint` — issue involves TLS fingerprinting / utls + +4. **Assess priority**: + - `priority:critical` — security vulnerability or compilation failure affecting all users + - `priority:high` — data loss, memory leak, panic in production + - `priority:medium` — bug with workaround + - `priority:low` — minor issue or feature request + +5. **Apply labels** + ```bash + gh issue edit -R imroc/req --add-label "bug,quic-go,priority:high" + ``` + +6. **Post initial response** for: + - **Security issues**: acknowledge receipt, advise not to share exploit details publicly + - **quic-go related**: note that this involves modified code and needs careful review + - **Compilation failures**: confirm reproduction, link to relevant upstream changes + - **Questions**: provide helpful pointer or answer if straightforward + ```bash + gh issue comment -R imroc/req --body "..." + ``` + +## Rules + +- Only apply labels that exist in the repo (check with `gh label list -R imroc/req`) +- Never close issues automatically +- Never promise timelines +- For quic-go related issues, always add the `quic-go` label and note the modified-code caveat +- If an issue is a duplicate, link to the original with `Duplicate of #NNN` +- Write all responses in English diff --git a/.codebuddy/skills/pr-review/SKILL.md b/.codebuddy/skills/pr-review/SKILL.md new file mode 100644 index 00000000..63f2d088 --- /dev/null +++ b/.codebuddy/skills/pr-review/SKILL.md @@ -0,0 +1,63 @@ +--- +name: pr-review +description: Automated PR review expert for the req project. Reviews code changes with special attention to modified stdlib, quic-go, and HTTP/2 code. Posts review comments via gh. Used proactively on new PRs. +allowed-tools: Read, Bash, Grep +--- + +# PR Review Expert + +You review pull requests for the req project, with special attention to modified-code areas. + +## Workflow + +1. **Fetch PR details** + ```bash + gh pr view -R imroc/req --json title,body,files,additions,deletions,author,baseRefName + gh pr diff -R imroc/req + ``` + +2. **Identify modified-code areas** in the PR diff: + - `internal/http3/` — modified quic-go. **High caution**: check upstream compatibility + - `internal/http2/` — modified golang.org/x/net/http2. **High caution** + - Root files with Go Authors copyright header (`transport.go`, `transfer.go`, `textproto_reader.go`, `http.go`, `http_request.go`, `response.go`) — modified stdlib. **High caution** + - `middleware.go`, `client.go` — core logic, review for concurrency safety + +3. **Review checklist**: + - **Correctness**: Does the change solve the stated problem? Any logic errors? + - **Modified-code safety**: Does it break req's customizations to stdlib/quic-go? Does it reference upstream APIs that may have changed? + - **Concurrency**: Any data races? Map read/write without lock? (see issue #397) + - **Security**: Any header leakage, injection, or credential exposure? (see issue #489) + - **Tests**: Are there tests? Do existing tests still pass? No tests deleted or weakened? + - **Scope**: Does it only change what's necessary? No unrelated refactoring? + - **Go conventions**: Error handling, resource cleanup (Close body/conn)? + +4. **Post review**: + ```bash + # Approve + gh pr review -R imroc/req --approve --body "..." + # Request changes + gh pr review -R imroc/req --request-changes --body "..." + # Comment only + gh pr review -R imroc/req --comment --body "..." + ``` + +## Special Handling + +### quic-go related PRs (files in `internal/http3/`) +- Check if the PR syncs to a specific quic-go version +- Verify no references to removed/renamed upstream APIs +- Verify req's customizations (dump, middleware, protocol sniffing) still work +- Request the author to note the target quic-go version in the PR description + +### Modified stdlib PRs (root files with Go Authors copyright) +- Check if it's an upstream sync (`merge upstream net/http`) +- Verify req's customizations are preserved +- Check for any stdlib API changes that affect other files + +## Rules + +- Never approve PRs that touch `internal/http3/` without verifying quic-go version compatibility +- Never approve PRs that delete or weaken tests +- If the PR includes `git tag` or `gh release create`, comment with block +- Write all reviews in English +- Be constructive and specific — reference file:line in comments diff --git a/.codebuddy/skills/test-gate/SKILL.md b/.codebuddy/skills/test-gate/SKILL.md new file mode 100644 index 00000000..3fef9465 --- /dev/null +++ b/.codebuddy/skills/test-gate/SKILL.md @@ -0,0 +1,49 @@ +--- +name: test-gate +description: Full test suite runner. Runs go build, go vet, go test ./... before any change is pushed. Used to ensure no breaking changes. Used proactively before commits and pushes. +allowed-tools: Read, Bash +--- + +# Test Gate + +You are responsible for ensuring code changes do not break existing tests. + +## Workflow + +Run these checks in order. If any fails, stop and report the error. + +1. **Build check** + ```bash + go build ./... + ``` + +2. **Vet check** + ```bash + go vet ./... + ``` + +3. **Full test suite** (always run, not just short mode) + ```bash + go test ./... -timeout 120s + ``` + +4. **Go mod tidy check** (if go.mod or go.sum was modified) + ```bash + cp go.mod /tmp/go.mod.bak && cp go.sum /tmp/go.sum.bak + go mod tidy + diff go.mod /tmp/go.mod.bak && diff go.sum /tmp/go.sum.bak + ``` + +5. **Coverage check** (informational, not blocking) + ```bash + go test ./... -coverprofile=/tmp/coverage.out -timeout 120s + go tool cover -func=/tmp/coverage.out | tail -1 + ``` + +## Rules + +- All checks must pass before any commit or push +- If tests fail, fix the issue before proceeding +- Never use --no-verify to skip checks +- Never skip tests for "quick fixes" — always run the full suite +- Report coverage changes if coverage drops significantly diff --git a/.codebuddy/skills/upstream-sync/SKILL.md b/.codebuddy/skills/upstream-sync/SKILL.md new file mode 100644 index 00000000..2d67faac --- /dev/null +++ b/.codebuddy/skills/upstream-sync/SKILL.md @@ -0,0 +1,55 @@ +--- +name: upstream-sync +description: Tracks Go stdlib net/http and quic-go upstream changes against req's modified copies. Generates diff reports for manual sync. Used proactively for upstream tracking. +allowed-tools: Read, Bash, Grep, WebFetch +--- + +# Upstream Sync Tracking Expert + +req modifies Go stdlib net/http, golang.org/x/net/http2, and quic-go source code internally. This skill tracks upstream changes and generates reports for manual sync. + +## Modified Code Locations + +| Upstream | req Location | Sync Commit Convention | +|----------|-------------|----------------------| +| Go stdlib `src/net/http/` | Root files (`transport.go`, `transfer.go`, `textproto_reader.go`, `http.go`, `http_request.go`, `response.go`, etc.) | `merge upstream net/http: ()` | +| `golang.org/x/net/http2` | `internal/http2/` | `merge upstream http2: ()` | +| `github.com/quic-go/quic-go` | `internal/http3/` | `port quic-go ` | + +## Workflow + +1. **Check current upstream baselines** + - Read `go.mod` for current quic-go version + - Check git log for last sync commits: + ```bash + git log --oneline --all | grep -E "merge upstream|port quic-go" | head -5 + ``` + +2. **Check Go stdlib upstream** (via WebFetch) + - Fetch Go source `src/net/http/` commit history since last sync + - URL: `https://github.com/golang/go/commits/master/src/net/http` + - Identify commits that affect files modified in req + +3. **Check quic-go upstream** + - Fetch latest quic-go releases: `https://github.com/quic-go/quic-go/releases` + - Compare with current version in go.mod + - Identify breaking changes (removed APIs, renamed types) + +4. **Check golang.org/x/net/http2 upstream** + - Fetch `https://github.com/golang/net/commits/master/http2` + - Identify commits since last sync + +5. **Generate sync report** covering: + - Current baseline: Go stdlib hash, http2 hash, quic-go version + - New upstream changes since last sync (categorized: bug fix, security, feature, breaking) + - Affected files in req that need manual sync + - Risk assessment (especially for quic-go breaking changes) + - Recommended action items + +## Rules + +- This skill only **tracks and reports** — never modifies modified-code files directly +- Sync of modified code is a manual human process +- Always note breaking changes in quic-go (they affect `internal/http3/`) +- Report Go version requirements (new stdlib changes may require newer Go) +- Write all reports in English diff --git a/.github/workflows/ci-fix-loop.yml b/.github/workflows/ci-fix-loop.yml new file mode 100644 index 00000000..c49aaa83 --- /dev/null +++ b/.github/workflows/ci-fix-loop.yml @@ -0,0 +1,54 @@ +name: CI Fix Loop + +on: + workflow_run: + workflows: ["CI"] + types: + - completed + workflow_dispatch: {} + +jobs: + fix: + runs-on: ubuntu-latest + if: >- + github.event.workflow_run.conclusion == 'failure' && + github.repository == 'imroc/req' && + vars.CODEBUDDY_ENABLED == 'true' + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - uses: actions/setup-go@v5 + with: + go-version: '1.25.x' + + - name: Install CodeBuddy Code + run: npm install -g @tencent-ai/codebuddy-code + + - name: Run CI fix loop + env: + CODEBUDDY_API_KEY: ${{ secrets.CODEBUDDY_API_KEY }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + codebuddy -p -y \ + --output-format json \ + --allowedTools "Read,Bash,Grep,Edit" \ + "使用 ci-fixer 代理执行一次 CI 修复循环迭代。\ + 读取 STATE.md 了解上次状态。\ + 用 gh run list --status failure --limit 3 --workflow ci.yml 找失败的 CI run。\ + 用 gh run view --log-failed 获取日志，调用 ci-triage skill 分类。\ + 对 auto_fixable 的失败，在分支 fix/ci--$(date +%Y%m%d) 上修复。\ + 本地 go build ./... && go vet ./... && go test ./... 全部通过后，\ + 调用 code-reviewer 代理审查改动。\ + 审查通过则用 gh pr create 创建 PR（标题 fix: ci failure ）。\ + 不可自动修复的记录到 STATE.md 待人工处理。\ + 最后更新 STATE.md 的 CI 修复循环章节并提交到 master。" + + - name: Commit state + run: | + git config --local user.email "ai-bot@req.cool" + git config --local user.name "req-loop-bot" + git add STATE.md 2>/dev/null || true + git commit -m "chore: update loop state" || true + git push origin master 2>/dev/null || true diff --git a/.github/workflows/dependency-upgrade-loop.yml b/.github/workflows/dependency-upgrade-loop.yml new file mode 100644 index 00000000..688fc185 --- /dev/null +++ b/.github/workflows/dependency-upgrade-loop.yml @@ -0,0 +1,45 @@ +name: Dependency Upgrade Loop + +on: + schedule: + - cron: '0 3 * * 1' # 每周一 03:00 UTC + workflow_dispatch: {} + +jobs: + upgrade: + runs-on: ubuntu-latest + if: github.repository == 'imroc/req' && vars.CODEBUDDY_ENABLED == 'true' + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - uses: actions/setup-go@v5 + with: + go-version: '1.25.x' + + - name: Install CodeBuddy Code + run: npm install -g @tencent-ai/codebuddy-code + + - name: Run dependency upgrade loop + env: + CODEBUDDY_API_KEY: ${{ secrets.CODEBUDDY_API_KEY }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + codebuddy -p -y \ + --output-format json \ + --allowedTools "Read,Bash,Grep,Edit" \ + "使用 dependency-upgrader 代理执行一次依赖升级循环迭代。\ + 读取 STATE.md 了解上次状态。\ + 升级依赖后在分支 chore/upgrade-deps-$(date +%Y%m%d) 上提交。\ + 本地 go test ./... 通过后，调用 code-reviewer 代理审查改动。\ + 审查通过则用 gh pr create 创建 PR（标题 chore: upgrade dependencies，body 含升级列表）。\ + 最后更新 STATE.md 的依赖升级循环章节并提交到 master。" + + - name: Commit state + run: | + git config --local user.email "ai-bot@req.cool" + git config --local user.name "req-loop-bot" + git add STATE.md 2>/dev/null || true + git commit -m "chore: update loop state" || true + git push origin master 2>/dev/null || true diff --git a/.github/workflows/issue-triage-loop.yml b/.github/workflows/issue-triage-loop.yml new file mode 100644 index 00000000..0fddb2a3 --- /dev/null +++ b/.github/workflows/issue-triage-loop.yml @@ -0,0 +1,34 @@ +name: Issue Triage Loop + +on: + issues: + types: [opened] + schedule: + - cron: '0 4 * * *' # Daily 04:00 UTC for batch triage + workflow_dispatch: {} + +jobs: + triage: + runs-on: ubuntu-latest + if: github.repository == 'imroc/req' && vars.CODEBUDDY_ENABLED == 'true' + steps: + - uses: actions/checkout@v4 + + - name: Install CodeBuddy Code + run: npm install -g @tencent-ai/codebuddy-code + + - name: Run issue triage loop + env: + CODEBUDDY_API_KEY: ${{ secrets.CODEBUDDY_API_KEY }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + codebuddy -p -y \ + --output-format json \ + --allowedTools "Read,Bash,Grep,WebFetch" \ + "Use the issue-triager agent to triage issues. \ + Read STATE.md for last state. \ + Use gh issue list -R imroc/req --state open --limit 10 to find untriaged issues. \ + For each: fetch with gh issue view, classify, apply labels with gh issue edit --add-label, \ + post initial response with gh issue comment if appropriate. \ + Pay special attention to quic-go and modified-stdlib related issues. \ + Update STATE.md issue triage section and commit." diff --git a/.github/workflows/pr-review-loop.yml b/.github/workflows/pr-review-loop.yml new file mode 100644 index 00000000..8c24354e --- /dev/null +++ b/.github/workflows/pr-review-loop.yml @@ -0,0 +1,38 @@ +name: PR Review Loop + +on: + pull_request: + types: [opened, synchronize] + workflow_dispatch: {} + +jobs: + review: + runs-on: ubuntu-latest + if: github.repository == 'imroc/req' && vars.CODEBUDDY_ENABLED == 'true' + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - uses: actions/setup-go@v5 + with: + go-version: '1.25.x' + + - name: Install CodeBuddy Code + run: npm install -g @tencent-ai/codebuddy-code + + - name: Run PR review loop + env: + CODEBUDDY_API_KEY: ${{ secrets.CODEBUDDY_API_KEY }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + codebuddy -p -y \ + --output-format json \ + --allowedTools "Read,Bash,Grep" \ + "Use the pr-reviewer agent to review PR #${{ github.event.pull_request.number }}. \ + Read STATE.md for context. \ + Fetch PR details with gh pr view and gh pr diff. \ + Apply the pr-review skill checklist. \ + Pay special attention to internal/http3/ (quic-go), internal/http2/, and modified stdlib files. \ + Post review with gh pr review. \ + Update STATE.md PR review section and commit." diff --git a/.github/workflows/upstream-sync-loop.yml b/.github/workflows/upstream-sync-loop.yml new file mode 100644 index 00000000..cba69f51 --- /dev/null +++ b/.github/workflows/upstream-sync-loop.yml @@ -0,0 +1,40 @@ +name: Upstream Sync Tracking Loop + +on: + schedule: + - cron: '0 2 * * 1' # Weekly Monday 02:00 UTC + workflow_dispatch: {} + +jobs: + track: + runs-on: ubuntu-latest + if: github.repository == 'imroc/req' && vars.CODEBUDDY_ENABLED == 'true' + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Install CodeBuddy Code + run: npm install -g @tencent-ai/codebuddy-code + + - name: Run upstream sync tracking loop + env: + CODEBUDDY_API_KEY: ${{ secrets.CODEBUDDY_API_KEY }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + codebuddy -p -y \ + --output-format json \ + --allowedTools "Read,Bash,Grep,WebFetch" \ + "Use the upstream-tracker agent to check upstream changes. \ + Read STATE.md for last sync baselines. \ + Check Go stdlib net/http, golang.org/x/net/http2, and quic-go for new changes since last sync. \ + Generate a sync report. If breaking changes or security fixes found, open an issue with gh issue create. \ + Update STATE.md upstream sync section and commit." + + - name: Commit state + run: | + git config --local user.email "ai-bot@req.cool" + git config --local user.name "req-loop-bot" + git add STATE.md 2>/dev/null || true + git commit -m "chore: update upstream sync state" || true + git push origin master 2>/dev/null || true diff --git a/.gitignore b/.gitignore index d2bf3553..9162defc 100644 --- a/.gitignore +++ b/.gitignore @@ -31,4 +31,4 @@ Session.vim *.swp # OSX trash -.DS_Store \ No newline at end of file +.DS_Store.codebuddy/scheduled_tasks.lock diff --git a/STATE.md b/STATE.md new file mode 100644 index 00000000..f32eb887 --- /dev/null +++ b/STATE.md @@ -0,0 +1,125 @@ +# Loop State + +This file is the persistent state for Loop Engineering automated maintenance loops. +All loops read from here and write back after execution. State is external, not in model context. + +## Dependency Upgrade Loop + +### Run History +(first run pending — cron installed, will run at 02:00 daily) + +### Last Run +- Date: N/A +- Result: not run + +--- + +## CI Fix Loop + +### Run History +(first run pending — cron installed, will run at 03:00 daily) + +### Last Run +- Date: N/A +- Result: not run + +--- + +## Issue Triage Loop + +### Run History +- 2026-06-27 (run 1): Triaged 10 open issues, applied 14 labels, created 4 labels (security, performance, quic-go, tls-fingerprint), posted 3 comments (#496, #481, #464) +- 2026-06-27 (run 2): Triaged 15 unlabeled issues (#459–#423), applied 38 labels, created 6 labels (http2, modified-stdlib, priority:critical, priority:high, priority:medium, priority:low), posted 1 comment (#457) +- 2026-06-29: Triaged 10 open issues, found 2 unlabeled (#406, #404), applied 4 labels (enhancement, priority:low), posted 2 comments (#406, #404). No quic-go/HTTP/3 issues found in this batch. +- 2026-06-29 (run 2): Triaged 10 open issues (#475–#404), all already labeled — 0 unlabeled, 0 labels applied. 2 quic-go related (#460, #457) already correctly tagged. Posted 2 initial responses (#475, #431) on previously unanswered issues. + +### Last Run +- Date: 2026-06-29 (run 2) +- Result: success — all 10 issues already labeled, 0 labels applied, 2 comments posted (#475, #431), 2 quic-go related (#460, #457) confirmed labeled + +### Triage Details (2026-06-27, run 2) +| # | Title | Type | Extra | Priority | Comment | +|---|---|---|---|---|---| +| #459 | utls fingerprint update to 133 | enhancement | tls-fingerprint | low | No | +| #457 | HTTP/3 AddConn/RoundTrip race | bug | quic-go | high | Yes — noted modified-code caveat, high priority | +| #454 | Support firefox133/chrome133 | enhancement | tls-fingerprint | low | No | +| #453 | Better performance than resty | question | — | low | No | +| #452 | Rotating proxy best practices | question | — | low | No | +| #446 | Win7 compatibility (quic-go) | question | quic-go | low | No | +| #445 | Proxy set but connects to localhost | bug | — | medium | No | +| #444 | Put without body: Content-Length: 0 | question | — | low | No | +| #437 | Switching proxy after connection not working | bug | modified-stdlib | medium | No | +| #436 | Retry limit reached but no error returned | question | — | low | No | +| #433 | Large file upload buffered in memory | bug | performance, modified-stdlib | high | No | +| #431 | jsonrpc2 support | enhancement | — | low | No | +| #425 | zstd content encoding support | enhancement | — | low | No | +| #424 | Content-Type charset not applied in headers | bug | — | medium | No | +| #423 | SetSuccessResult for raw response | question | — | low | No | + +### Labels Created This Run (run 2) +- `http2` (0052cc) — Involves HTTP/2 or internal/http2 modified code +- `modified-stdlib` (c5def5) — Involves modified Go stdlib files (transport.go, transfer.go, etc.) +- `priority:critical` (b60205) — Security vulnerability or compilation failure affecting all users +- `priority:high` (d93f0b) — Data loss, memory leak, or panic in production +- `priority:medium` (fbca04) — Bug with available workaround +- `priority:low` (0e8a16) — Minor issue or feature request + +### Needs Human Attention +- **#496** — Compilation failure: gin v1.12.0 pulls quic-go v0.59.0, but internal/http3/ uses removed `quic.ConnectionTracingID` API. High priority, breaks user builds. +- **#484** — Inconsistent error handling between client-level and request-level afterResponse middleware loops (request-level stops on first error, client-level doesn't). May warrant code change. +- **#457** — HTTP/3 race condition between AddConn and RoundTrip in modified `internal/http3/` code. Potential panic in high-traffic environments. Needs investigation of initOnce bypass. +- **#433** — Large file upload (100MB+) causes entire file to be buffered in memory (2x+ memory usage). Likely fix needed in middleware.go upload path. + +--- + +## PR Review Loop + +### Run History +(first run pending — cron installed, will run at 05:00 daily) + +### Last Run +- Date: N/A +- Result: not run + +--- + +## Upstream Sync Tracking Loop + +### Current Baselines +- Go stdlib net/http: 2024-09-10 (ad6ee2) +- golang.org/x/net/http2: 2024-09-06 (3c333c) +- quic-go: v0.59.0 +- utls: v1.8.2 + +### Run History +(first run pending — cron installed, will run at 01:00 daily) + +### Last Run +- Date: N/A +- Result: not run + +--- + +## Manual Actions Log (2026-06-27) + +### Resolved +- #482: quic-go v0.59.0 ported (commit 8b018a5, 29f0e6d) +- #489: SensitiveHeadersRedirectPolicy added (commit b144240) +- #471/#472: Chrome accept header add application/json (commit 1e34ab9) +- #415/#486: SetCookieJarFactory returns http.CookieJar (commit d702aad) +- #491: GOAWAY retry on cached HTTP/2 connections (commit 5754208) +- #459: utls upgraded to v1.8.2 security update (commit dcc8516) +- PR #485 closed (superseded by 8b018a5) +- PR #472 closed (merged as 1e34ab9) +- PR #486 closed (merged as d702aad) +- PR #491 closed (merged as 5754208) + +### Still Open +- PR #478/#477: JA3 / ClientHelloSpec support — needs careful review, TLS fingerprint changes +- PR #465: Unmarshal status-code check — needs review +- #495: dialConn memory overusage — consider default MaxConnsPerHost +- #433: Large file upload memory buffering — middleware.go fix needed +- #397: concurrent map read/write in parseRequestHeader — needs locking +- #457: HTTP/3 AddConn/RoundTrip race condition +- #376: HTTP/2 frequent errors (9 comments) +- 65 open issues total (25 now labeled), mostly feature requests and usage questions diff --git a/client.go b/client.go index 05744963..6c9aae54 100644 --- a/client.go +++ b/client.go @@ -53,7 +53,7 @@ type Client struct { AllowGetMethodPayload bool *Transport digestAuth *digestAuth - cookiejarFactory func() *cookiejar.Jar + cookiejarFactory func() http.CookieJar trace bool disableAutoReadResponse bool commonErrorType reflect.Type @@ -1238,6 +1238,10 @@ func (conn *uTLSConn) ConnectionState() tls.ConnectionState { // which uses the specified clientHelloID to simulate the tls fingerprint. // Note this is valid for HTTP1 and HTTP2, not HTTP3. func (c *Client) SetTLSFingerprint(clientHelloID utls.ClientHelloID) *Client { + c.setTLSFingerprint(clientHelloID, nil) + return c +} +func (c *Client) setTLSFingerprint(clientHelloID utls.ClientHelloID, uTLSConnApply func(*uTLSConn) error) *Client { fn := func(ctx context.Context, addr string, plainConn net.Conn) (conn net.Conn, tlsState *tls.ConnectionState, err error) { colonPos := strings.LastIndex(addr, ":") if colonPos == -1 { @@ -1261,6 +1265,11 @@ func (c *Client) SetTLSFingerprint(clientHelloID utls.ClientHelloID) *Client { KeyLogWriter: tlsConfig.KeyLogWriter, } uconn := &uTLSConn{utls.UClient(plainConn, utlsConfig, clientHelloID)} + if uTLSConnApply != nil { + if err = uTLSConnApply(uconn); err != nil { + return + } + } err = uconn.HandshakeContext(ctx) if err != nil { return @@ -1287,6 +1296,18 @@ func (c *Client) SetTLSFingerprint(clientHelloID utls.ClientHelloID) *Client { return c } +// SetTLSFingerprintSpec set the tls fingerprint for tls handshake using a custom +// ClientHelloSpec, which allows fine-grained control over the TLS fingerprint +// (e.g. for JA3/JA4 customization). Uses utls +// (https://github.com/refraction-networking/utls) to perform the tls handshake. +// Note this is valid for HTTP1 and HTTP2, not HTTP3. +func (c *Client) SetTLSFingerprintSpec(clientHelloID *utls.ClientHelloSpec) *Client { + c.setTLSFingerprint(utls.HelloCustom, func(conn *uTLSConn) error { + return conn.ApplyPreset(clientHelloID) + }) + return c +} + // SetCustomTLSFingerprint allows setting a TLS fingerprint from // raw TLS Client Hello bytes. func (c *Client) SetCustomTLSFingerprint(rawClientHello []byte) *Client { @@ -1648,7 +1669,7 @@ func (c *Client) Clone() *Client { return &cc } -func memoryCookieJarFactory() *cookiejar.Jar { +func memoryCookieJarFactory() http.CookieJar { jar, _ := cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List}) return jar } @@ -1695,7 +1716,7 @@ func C() *Client { // cookie jar that store cookies for underlying `http.Client`. After client clone, // the cookie jar of the new client will also be regenerated using this factory // function. -func (c *Client) SetCookieJarFactory(factory func() *cookiejar.Jar) *Client { +func (c *Client) SetCookieJarFactory(factory func() http.CookieJar) *Client { c.cookiejarFactory = factory c.initCookieJar() return c diff --git a/client_impersonate.go b/client_impersonate.go index 43affd8f..8552269d 100644 --- a/client_impersonate.go +++ b/client_impersonate.go @@ -139,7 +139,7 @@ var ( "sec-ch-ua-platform": `"macOS"`, "upgrade-insecure-requests": "1", "user-agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36", - "accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7", + "accept": "text/html,application/xhtml+xml,application/xml,application/json;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7", "sec-fetch-site": "none", "sec-fetch-mode": "navigate", "sec-fetch-user": "?1", diff --git a/client_test.go b/client_test.go index 14ea788e..f8ccbf2d 100644 --- a/client_test.go +++ b/client_test.go @@ -424,6 +424,38 @@ func TestRedirect(t *testing.T) { tests.AssertEqual(t, "test", newHeader.Get("Authorization")) } +func TestSensitiveHeadersRedirectPolicy(t *testing.T) { + // Cross-domain redirect: sensitive header should be stripped + crossDomainReq := &http.Request{ + Header: http.Header{}, + URL: &url.URL{Host: "evil.com"}, + } + crossDomainReq.Header.Set("X-API-Key", "secret") + via := []*http.Request{{ + Header: http.Header{}, + URL: &url.URL{Host: "api.example.com"}, + }} + via[0].Header.Set("X-API-Key", "secret") + + tc().SetRedirectPolicy(SensitiveHeadersRedirectPolicy("X-API-Key")).GetClient().CheckRedirect(crossDomainReq, via) + tests.AssertEqual(t, "", crossDomainReq.Header.Get("X-API-Key")) + + // Same-domain redirect: sensitive header should be kept + sameDomainReq := &http.Request{ + Header: http.Header{}, + URL: &url.URL{Host: "sub.example.com"}, + } + sameDomainReq.Header.Set("X-API-Key", "secret") + viaSame := []*http.Request{{ + Header: http.Header{}, + URL: &url.URL{Host: "api.example.com"}, + }} + viaSame[0].Header.Set("X-API-Key", "secret") + + tc().SetRedirectPolicy(SensitiveHeadersRedirectPolicy("X-API-Key")).GetClient().CheckRedirect(sameDomainReq, viaSame) + tests.AssertEqual(t, "secret", sameDomainReq.Header.Get("X-API-Key")) +} + func TestGetTLSClientConfig(t *testing.T) { c := tc() config := c.GetTLSClientConfig() diff --git a/go.mod b/go.mod index 5ea896f0..82fe5140 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/imroc/req/v3 go 1.24.0 -replace github.com/refraction-networking/utls v1.8.1 => github.com/banyansecurity/utls v0.0.0-20251107181709-4258fbe6b682 +replace github.com/refraction-networking/utls v1.8.2 => github.com/banyansecurity/utls v0.0.0-20260120191037-c47b0c63efd5 require ( github.com/andybalholm/brotli v1.2.0 @@ -10,8 +10,8 @@ require ( github.com/icholy/digest v1.1.0 github.com/klauspost/compress v1.18.2 github.com/quic-go/qpack v0.6.0 - github.com/quic-go/quic-go v0.57.1 - github.com/refraction-networking/utls v1.8.1 + github.com/quic-go/quic-go v0.59.0 + github.com/refraction-networking/utls v1.8.2 golang.org/x/net v0.48.0 golang.org/x/text v0.32.0 ) diff --git a/go.sum b/go.sum index 9ced94c5..48307785 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ github.com/andybalholm/brotli v1.2.0 h1:ukwgCxwYrmACq68yiUqwIWnGY0cTPox/M94sVwToPjQ= github.com/andybalholm/brotli v1.2.0/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= -github.com/banyansecurity/utls v0.0.0-20251107181709-4258fbe6b682 h1:QGNoxbiXDRrIG7WsHeNIS3bp6Yg2HasA0GGTF22djMQ= -github.com/banyansecurity/utls v0.0.0-20251107181709-4258fbe6b682/go.mod h1:jkSOEkLqn+S/jtpEHPOsVv/4V4EVnelwbMQl4vCWXAM= +github.com/banyansecurity/utls v0.0.0-20260120191037-c47b0c63efd5 h1:YEgctYYYeJEUa/uvfeN2gBEx4tqkfW3nmSGdyL182/M= +github.com/banyansecurity/utls v0.0.0-20260120191037-c47b0c63efd5/go.mod h1:jkSOEkLqn+S/jtpEHPOsVv/4V4EVnelwbMQl4vCWXAM= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= @@ -17,8 +17,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/quic-go/qpack v0.6.0 h1:g7W+BMYynC1LbYLSqRt8PBg5Tgwxn214ZZR34VIOjz8= github.com/quic-go/qpack v0.6.0/go.mod h1:lUpLKChi8njB4ty2bFLX2x4gzDqXwUpaO1DP9qMDZII= -github.com/quic-go/quic-go v0.57.1 h1:25KAAR9QR8KZrCZRThWMKVAwGoiHIrNbT72ULHTuI10= -github.com/quic-go/quic-go v0.57.1/go.mod h1:ly4QBAjHA2VhdnxhojRsCUOeJwKYg+taDlos92xb1+s= +github.com/quic-go/quic-go v0.59.0 h1:OLJkp1Mlm/aS7dpKgTc6cnpynnD2Xg7C1pwL6vy/SAw= +github.com/quic-go/quic-go v0.59.0/go.mod h1:upnsH4Ju1YkqpLXC305eW3yDZ4NfnNbmQRCMWS58IKU= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/xyproto/randomstring v1.0.5 h1:YtlWPoRdgMu3NZtP45drfy1GKoojuR7hmRcnhZqKjWU= @@ -33,8 +33,6 @@ golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/text v0.32.0 h1:ZD01bjUt1FQ9WJ0ClOL5vxgxOI/sVCNgX1YtKwcY0mU= golang.org/x/text v0.32.0/go.mod h1:o/rUWzghvpD5TXrTIBuJU77MTaN0ljMWE47kxGJQ7jY= -golang.org/x/time v0.12.0 h1:ScB/8o8olJvc+CQPWrK3fPZNfh7qgwCrY0zJmoEQLSE= -golang.org/x/time v0.12.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/http2/http2_test.go b/http2/http2_test.go new file mode 100644 index 00000000..4e103481 --- /dev/null +++ b/http2/http2_test.go @@ -0,0 +1,63 @@ +package http2 + +import "testing" + +func TestPriorityParamIsZero(t *testing.T) { + var p PriorityParam + if !p.IsZero() { + t.Fatal("zero PriorityParam should be zero") + } + p = PriorityParam{StreamDep: 1} + if p.IsZero() { + t.Fatal("PriorityParam with StreamDep=1 should not be zero") + } +} + +func TestSettingIDString(t *testing.T) { + tests := []struct { + id SettingID + expected string + }{ + {SettingHeaderTableSize, "HEADER_TABLE_SIZE"}, + {SettingEnablePush, "ENABLE_PUSH"}, + {SettingMaxConcurrentStreams, "MAX_CONCURRENT_STREAMS"}, + {SettingInitialWindowSize, "INITIAL_WINDOW_SIZE"}, + {SettingMaxFrameSize, "MAX_FRAME_SIZE"}, + {SettingMaxHeaderListSize, "MAX_HEADER_LIST_SIZE"}, + {SettingID(0x99), "UNKNOWN_SETTING_153"}, + } + for _, tt := range tests { + if got := tt.id.String(); got != tt.expected { + t.Errorf("SettingID(%d).String() = %q, want %q", uint16(tt.id), got, tt.expected) + } + } +} + +func TestSettingString(t *testing.T) { + s := Setting{ID: SettingHeaderTableSize, Val: 4096} + got := s.String() + expected := "[HEADER_TABLE_SIZE = 4096]" + if got != expected { + t.Errorf("Setting.String() = %q, want %q", got, expected) + } +} + +func TestPriorityFrame(t *testing.T) { + pf := PriorityFrame{ + StreamID: 1, + PriorityParam: PriorityParam{ + StreamDep: 0, + Exclusive: false, + Weight: 16, + }, + } + if pf.StreamID != 1 { + t.Fatalf("StreamID = %d, want 1", pf.StreamID) + } + if pf.PriorityParam.Weight != 16 { + t.Fatalf("Weight = %d, want 16", pf.PriorityParam.Weight) + } + if pf.PriorityParam.IsZero() { + t.Fatal("PriorityParam should not be zero with Weight=16") + } +} diff --git a/internal/compress/compress_test.go b/internal/compress/compress_test.go new file mode 100644 index 00000000..05936e4d --- /dev/null +++ b/internal/compress/compress_test.go @@ -0,0 +1,126 @@ +package compress + +import ( + "bytes" + "compress/flate" + "compress/gzip" + "io" + "strings" + "testing" +) + +func gzipData(t *testing.T, data string) io.ReadCloser { + t.Helper() + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + if _, err := gw.Write([]byte(data)); err != nil { + t.Fatal(err) + } + gw.Close() + return io.NopCloser(&buf) +} + +func TestGzipReader(t *testing.T) { + original := "hello world, this is a test string for gzip" + body := gzipData(t, original) + gz := NewGzipReader(body) + defer gz.Close() + + out, err := io.ReadAll(gz) + if err != nil { + t.Fatalf("ReadAll failed: %v", err) + } + if string(out) != original { + t.Fatalf("got %q, want %q", string(out), original) + } +} + +func TestGzipReaderInvalidData(t *testing.T) { + body := io.NopCloser(strings.NewReader("not gzip data")) + gz := NewGzipReader(body) + defer gz.Close() + + _, err := gz.Read(make([]byte, 10)) + if err == nil { + t.Fatal("expected error for invalid gzip data") + } +} + +func TestGzipReaderCloseTwice(t *testing.T) { + body := gzipData(t, "test") + gz := NewGzipReader(body) + if err := gz.Close(); err != nil { + t.Fatalf("first close failed: %v", err) + } + // After close, Read should return sticky error + _, err := gz.Read(make([]byte, 10)) + if err == nil { + t.Fatal("expected error after close") + } +} + +func TestGzipReaderGetSetUnderlyingBody(t *testing.T) { + body1 := gzipData(t, "test1") + body2 := gzipData(t, "test2") + gz := NewGzipReader(body1) + if gz.GetUnderlyingBody() != body1 { + t.Fatal("GetUnderlyingBody mismatch") + } + gz.SetUnderlyingBody(body2) + if gz.GetUnderlyingBody() != body2 { + t.Fatal("SetUnderlyingBody failed") + } +} + +func deflateData(t *testing.T, data string) io.ReadCloser { + t.Helper() + var buf bytes.Buffer + fw, err := flate.NewWriter(&buf, flate.DefaultCompression) + if err != nil { + t.Fatal(err) + } + fw.Write([]byte(data)) + fw.Close() + return io.NopCloser(&buf) +} + +func TestDeflateReader(t *testing.T) { + original := "hello deflate world" + body := deflateData(t, original) + dr := NewDeflateReader(body) + defer dr.Close() + + out, err := io.ReadAll(dr) + if err != nil { + t.Fatalf("ReadAll failed: %v", err) + } + if string(out) != original { + t.Fatalf("got %q, want %q", string(out), original) + } +} + +func TestNewCompressReader(t *testing.T) { + tests := []struct { + encoding string + notNil bool + }{ + {"gzip", true}, + {"deflate", true}, + {"br", true}, + {"zstd", true}, + {"unknown", false}, + {"", false}, + } + for _, tt := range tests { + t.Run(tt.encoding, func(t *testing.T) { + body := io.NopCloser(strings.NewReader("")) + cr := NewCompressReader(body, tt.encoding) + if tt.notNil && cr == nil { + t.Fatal("expected non-nil CompressReader") + } + if !tt.notNil && cr != nil { + t.Fatal("expected nil CompressReader") + } + }) + } +} diff --git a/internal/dump/dump_test.go b/internal/dump/dump_test.go new file mode 100644 index 00000000..ba105fad --- /dev/null +++ b/internal/dump/dump_test.go @@ -0,0 +1,271 @@ +package dump + +import ( + "bytes" + "context" + "io" + "net/http" + "testing" + "time" +) + +// testOptions implements Options for testing +type testOptions struct { + output io.Writer + requestHeader bool + requestBody bool + responseHeader bool + responseBody bool + async bool + requestHeaderOut io.Writer + requestBodyOut io.Writer + responseHeaderOut io.Writer + responseBodyOut io.Writer +} + +func (o *testOptions) Output() io.Writer { return o.output } +func (o *testOptions) RequestHeaderOutput() io.Writer { return o.requestHeaderOut } +func (o *testOptions) RequestBodyOutput() io.Writer { return o.requestBodyOut } +func (o *testOptions) ResponseHeaderOutput() io.Writer { return o.responseHeaderOut } +func (o *testOptions) ResponseBodyOutput() io.Writer { return o.responseBodyOut } +func (o *testOptions) RequestHeader() bool { return o.requestHeader } +func (o *testOptions) RequestBody() bool { return o.requestBody } +func (o *testOptions) ResponseHeader() bool { return o.responseHeader } +func (o *testOptions) ResponseBody() bool { return o.responseBody } +func (o *testOptions) Async() bool { return o.async } +func (o *testOptions) Clone() Options { + return &testOptions{ + output: o.output, + requestHeader: o.requestHeader, + requestBody: o.requestBody, + responseHeader: o.responseHeader, + responseBody: o.responseBody, + async: o.async, + requestHeaderOut: o.requestHeaderOut, + requestBodyOut: o.requestBodyOut, + responseHeaderOut: o.responseHeaderOut, + responseBodyOut: o.responseBodyOut, + } +} + +func TestDumperSync(t *testing.T) { + var buf bytes.Buffer + opt := &testOptions{ + output: &buf, + requestHeader: true, + requestHeaderOut: &buf, + } + d := NewDumper(opt) + d.DumpRequestHeader([]byte("GET / HTTP/1.1\r\n")) + if buf.String() != "GET / HTTP/1.1\r\n" { + t.Fatalf("expected 'GET / HTTP/1.1\\r\\n', got %q", buf.String()) + } +} + +func TestDumperAsync(t *testing.T) { + var buf bytes.Buffer + opt := &testOptions{ + output: &buf, + requestHeader: true, + async: true, + requestHeaderOut: &buf, + } + d := NewDumper(opt) + go d.Start() + defer d.Stop() + + d.DumpRequestHeader([]byte("GET / HTTP/1.1\r\n")) + // Wait for async processing + for i := 0; i < 100 && buf.Len() == 0; i++ { + d.ch <- nil // trigger check + time.Sleep(time.Millisecond) + } + // Give it a moment to process + time.Sleep(50 * time.Millisecond) + if buf.String() != "GET / HTTP/1.1\r\n" { + t.Fatalf("expected 'GET / HTTP/1.1\\r\\n', got %q", buf.String()) + } +} + +func TestDumperClone(t *testing.T) { + var buf bytes.Buffer + opt := &testOptions{ + output: &buf, + requestHeader: true, + requestHeaderOut: &buf, + } + d := NewDumper(opt) + cloned := d.Clone() + if cloned == nil { + t.Fatal("clone returned nil") + } + if !cloned.RequestHeader() { + t.Fatal("clone should have RequestHeader=true") + } +} + +func TestDumperCloneNil(t *testing.T) { + var d *Dumper + if d.Clone() != nil { + t.Fatal("nil clone should return nil") + } +} + +func TestDumperEmptyData(t *testing.T) { + var buf bytes.Buffer + opt := &testOptions{ + output: &buf, + requestHeaderOut: &buf, + } + d := NewDumper(opt) + d.DumpRequestHeader([]byte{}) + if buf.Len() != 0 { + t.Fatal("expected no output for empty data") + } +} + +func TestDumpResponseBodyReadCloser(t *testing.T) { + var buf bytes.Buffer + opt := &testOptions{ + output: &buf, + responseBody: true, + responseBodyOut: &buf, + } + d := NewDumper(opt) + + body := io.NopCloser(bytes.NewReader([]byte("hello world"))) + wrapped := d.WrapResponseBodyReadCloser(body) + + buf2 := make([]byte, 11) + n, err := wrapped.Read(buf2) + if err != nil && err != io.EOF { + t.Fatalf("unexpected error: %v", err) + } + if n != 11 { + t.Fatalf("expected 11 bytes, got %d", n) + } + if buf.String() != "hello world" { + t.Fatalf("expected 'hello world' in dump, got %q", buf.String()) + } +} + +func TestDumpRequestBodyWriteCloser(t *testing.T) { + var buf bytes.Buffer + opt := &testOptions{ + output: &buf, + requestBody: true, + requestBodyOut: &buf, + } + d := NewDumper(opt) + + var inner bytes.Buffer + wc := &nopWriteCloser{&inner} + wrapped := d.WrapRequestBodyWriteCloser(wc) + + _, err := wrapped.Write([]byte("request body")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if buf.String() != "request body" { + t.Fatalf("expected 'request body' in dump, got %q", buf.String()) + } + if inner.String() != "request body" { + t.Fatalf("expected 'request body' in inner writer, got %q", inner.String()) + } +} + +func TestGetDumpersWithContext(t *testing.T) { + var buf bytes.Buffer + opt := &testOptions{output: &buf} + d := NewDumper(opt) + + // Without context dumper + dumps := GetDumpers(context.Background(), d) + if len(dumps) != 1 { + t.Fatalf("expected 1 dumper, got %d", len(dumps)) + } + + // With context dumper + ctx := context.WithValue(context.Background(), DumperKey, NewDumper(opt)) + dumps = GetDumpers(ctx, d) + if len(dumps) != 2 { + t.Fatalf("expected 2 dumpers, got %d", len(dumps)) + } + + // Nil dump arg + dumps = GetDumpers(ctx, nil) + if len(dumps) != 1 { + t.Fatalf("expected 1 dumper from context, got %d", len(dumps)) + } +} + +func TestGetResponseHeaderDumpers(t *testing.T) { + var buf bytes.Buffer + opt := &testOptions{ + output: &buf, + responseHeader: true, + responseHeaderOut: &buf, + } + d := NewDumper(opt) + + dumps := GetResponseHeaderDumpers(context.Background(), d) + if !dumps.ShouldDump() { + t.Fatal("expected ShouldDump to be true") + } + if len(dumps) != 1 { + t.Fatalf("expected 1 dumper, got %d", len(dumps)) + } + + // Test with non-response-header dumper + opt2 := &testOptions{output: &buf, responseHeader: false} + d2 := NewDumper(opt2) + dumps2 := GetResponseHeaderDumpers(context.Background(), d2) + if dumps2.ShouldDump() { + t.Fatal("expected ShouldDump to be false") + } +} + +func TestDumpersShouldDump(t *testing.T) { + var ds Dumpers + if ds.ShouldDump() { + t.Fatal("empty Dumpers should not ShouldDump") + } + + ds = Dumpers{NewDumper(&testOptions{output: io.Discard})} + if !ds.ShouldDump() { + t.Fatal("non-empty Dumpers should ShouldDump") + } +} + +func TestWrapResponseBodyIfNeeded(t *testing.T) { + var buf bytes.Buffer + opt := &testOptions{ + output: &buf, + responseBody: true, + responseBodyOut: &buf, + } + d := NewDumper(opt) + + req, _ := http.NewRequest("GET", "http://example.com", nil) + res := &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("test"))), + } + WrapResponseBodyIfNeeded(res, req, d) + + // Read from the wrapped body + buf2 := make([]byte, 4) + n, _ := res.Body.Read(buf2) + if n != 4 { + t.Fatalf("expected 4 bytes, got %d", n) + } + if buf.String() != "test" { + t.Fatalf("expected 'test' in dump, got %q", buf.String()) + } +} + +// nopWriteCloser wraps a writer to implement io.WriteCloser +type nopWriteCloser struct { + io.Writer +} + +func (n *nopWriteCloser) Close() error { return nil } diff --git a/internal/header/header_test.go b/internal/header/header_test.go new file mode 100644 index 00000000..3b53ebb8 --- /dev/null +++ b/internal/header/header_test.go @@ -0,0 +1,90 @@ +package header + +import ( + "net/http" + "testing" +) + +func TestIsExcluded(t *testing.T) { + tests := []struct { + key string + expected bool + }{ + {"Host", true}, + {"host", true}, + {"Content-Length", true}, + {"Connection", true}, + {"Transfer-Encoding", true}, + {"Upgrade", true}, + {"Keep-Alive", true}, + {"Proxy-Connection", true}, + {"Content-Type", false}, + {"Authorization", false}, + {"User-Agent", false}, + {"__header_order__", true}, + {"__pseudo_header_order__", true}, + } + for _, tt := range tests { + t.Run(tt.key, func(t *testing.T) { + if got := IsExcluded(tt.key); got != tt.expected { + t.Errorf("IsExcluded(%q) = %v, want %v", tt.key, got, tt.expected) + } + }) + } +} + +func TestSortKeyValues(t *testing.T) { + kvs := []KeyValues{ + {Key: "Content-Type", Values: []string{"text/html"}}, + {Key: "Authorization", Values: []string{"Bearer token"}}, + {Key: "User-Agent", Values: []string{"test"}}, + } + // Sort with Authorization first, then User-Agent, then Content-Type + orderedKeys := []string{"Authorization", "User-Agent", "Content-Type"} + SortKeyValues(kvs, orderedKeys) + if kvs[0].Key != "Authorization" { + t.Fatalf("expected Authorization first, got %s", kvs[0].Key) + } + if kvs[1].Key != "User-Agent" { + t.Fatalf("expected User-Agent second, got %s", kvs[1].Key) + } + if kvs[2].Key != "Content-Type" { + t.Fatalf("expected Content-Type third, got %s", kvs[2].Key) + } +} + +func TestSortKeyValuesUnorderedFallback(t *testing.T) { + kvs := []KeyValues{ + {Key: "Z-Custom", Values: []string{"z"}}, + {Key: "A-Custom", Values: []string{"a"}}, + {Key: "Content-Type", Values: []string{"text/html"}}, + } + // Only Content-Type is in orderedKeys, others should retain relative order + orderedKeys := []string{"Content-Type"} + SortKeyValues(kvs, orderedKeys) + // Content-Type should not necessarily be first since sort is stable + // for items not in the order map (they keep their comparison indices) +} + +func TestConstants(t *testing.T) { + if DefaultUserAgent == "" { + t.Fatal("DefaultUserAgent should not be empty") + } + if ContentType != "Content-Type" { + t.Fatalf("ContentType = %q", ContentType) + } + if JsonContentType != "application/json; charset=utf-8" { + t.Fatalf("JsonContentType = %q", JsonContentType) + } +} + +func TestIsExcludedWithHttpHeader(t *testing.T) { + h := http.Header{} + h.Set("Host", "example.com") + h.Set("Content-Type", "text/html") + for key := range h { + if key == "Content-Type" && IsExcluded(key) { + t.Fatal("Content-Type should not be excluded") + } + } +} diff --git a/internal/http2/flow_test.go b/internal/http2/flow_test.go new file mode 100644 index 00000000..f88b2b6a --- /dev/null +++ b/internal/http2/flow_test.go @@ -0,0 +1,115 @@ +package http2 + +import "testing" + +func TestInflowInit(t *testing.T) { + var f inflow + f.init(65535) + if f.avail != 65535 { + t.Fatalf("avail = %d, want 65535", f.avail) + } + if f.unsent != 0 { + t.Fatalf("unsent = %d, want 0", f.unsent) + } +} + +func TestInflowTake(t *testing.T) { + var f inflow + f.init(1000) + if !f.take(500) { + t.Fatal("take(500) should succeed with avail=1000") + } + if f.avail != 500 { + t.Fatalf("avail = %d, want 500 after take(500)", f.avail) + } + if f.take(501) { + t.Fatal("take(501) should fail with avail=500") + } +} + +func TestInflowAdd(t *testing.T) { + var f inflow + f.init(1000) + // Small add should buffer (less than inflowMinRefresh=4096) + add := f.add(100) + if add != 0 { + t.Fatalf("small add should return 0, got %d", add) + } + if f.unsent != 100 { + t.Fatalf("unsent = %d, want 100", f.unsent) + } + // Large add should trigger window update + add = f.add(5000) + if add != 5100 { + t.Fatalf("large add should return 5100, got %d", add) + } + if f.unsent != 0 { + t.Fatalf("unsent should be 0 after flush, got %d", f.unsent) + } +} + +func TestTakeInflows(t *testing.T) { + var f1, f2 inflow + f1.init(1000) + f2.init(500) + if !takeInflows(&f1, &f2, 400) { + t.Fatal("takeInflows(400) should succeed") + } + if f1.avail != 600 { + t.Fatalf("f1.avail = %d, want 600", f1.avail) + } + if f2.avail != 100 { + t.Fatalf("f2.avail = %d, want 100", f2.avail) + } + if takeInflows(&f1, &f2, 200) { + t.Fatal("takeInflows(200) should fail (f2 only has 100)") + } +} + +func TestOutflowAvailable(t *testing.T) { + var conn outflow + conn.n = 10000 + var stream outflow + stream.setConnFlow(&conn) + stream.n = 5000 + // Limited by stream + if got := stream.available(); got != 5000 { + t.Fatalf("available = %d, want 5000", got) + } + // When conn is lower, limited by conn + conn.n = 3000 + if got := stream.available(); got != 3000 { + t.Fatalf("available = %d, want 3000 (conn limited)", got) + } +} + +func TestOutflowTake(t *testing.T) { + var conn outflow + conn.n = 10000 + var stream outflow + stream.setConnFlow(&conn) + stream.n = 5000 + stream.take(2000) + if stream.n != 3000 { + t.Fatalf("stream.n = %d, want 3000", stream.n) + } + if conn.n != 8000 { + t.Fatalf("conn.n = %d, want 8000", conn.n) + } +} + +func TestOutflowAdd(t *testing.T) { + var f outflow + f.n = 1000 + if !f.add(500) { + t.Fatal("add(500) should succeed") + } + if f.n != 1500 { + t.Fatalf("n = %d, want 1500", f.n) + } + // Overflow check + f.n = 1<<31 - 100 + if f.add(200) { + t.Fatal("add that overflows should return false") + } +} diff --git a/internal/http2/frame_test.go b/internal/http2/frame_test.go new file mode 100644 index 00000000..e499c206 --- /dev/null +++ b/internal/http2/frame_test.go @@ -0,0 +1,196 @@ +package http2 + +import ( + "bytes" + "testing" + + reqhttp2 "github.com/imroc/req/v3/http2" +) + +func TestFrameTypeString(t *testing.T) { + tests := []struct { + ft FrameType + expected string + }{ + {FrameData, "DATA"}, + {FrameHeaders, "HEADERS"}, + {FramePriority, "PRIORITY"}, + {FrameRSTStream, "RST_STREAM"}, + {FrameSettings, "SETTINGS"}, + {FramePushPromise, "PUSH_PROMISE"}, + {FramePing, "PING"}, + {FrameGoAway, "GOAWAY"}, + {FrameWindowUpdate, "WINDOW_UPDATE"}, + {FrameContinuation, "CONTINUATION"}, + {FrameType(0xFF), "UNKNOWN_FRAME_TYPE_255"}, + } + for _, tt := range tests { + if got := tt.ft.String(); got != tt.expected { + t.Errorf("FrameType(%d).String() = %q, want %q", uint8(tt.ft), got, tt.expected) + } + } +} + +func TestFlagsHas(t *testing.T) { + flags := FlagDataEndStream | FlagDataPadded + if !flags.Has(FlagDataEndStream) { + t.Fatal("should have FlagDataEndStream") + } + if !flags.Has(FlagDataPadded) { + t.Fatal("should have FlagDataPadded") + } + if flags.Has(FlagHeadersEndHeaders) { + t.Fatal("should not have FlagHeadersEndHeaders") + } +} + +func TestFrameHeaderString(t *testing.T) { + fh := FrameHeader{ + Type: FrameData, + Flags: FlagDataEndStream, + Length: 100, + StreamID: 1, + } + s := fh.String() + if s == "" { + t.Fatal("FrameHeader.String() should not be empty") + } +} + +func TestFramerWriteReadData(t *testing.T) { + var buf bytes.Buffer + fw := NewFramer(&buf, nil) + fw.WriteData(1, true, []byte("hello world")) + + fr := NewFramer(nil, &buf) + f, err := fr.ReadFrame() + if err != nil { + t.Fatalf("ReadFrame failed: %v", err) + } + df, ok := f.(*DataFrame) + if !ok { + t.Fatalf("expected *DataFrame, got %T", f) + } + if df.StreamID != 1 { + t.Fatalf("StreamID = %d, want 1", df.StreamID) + } + if !df.StreamEnded() { + t.Fatal("expected stream to end") + } + if string(df.Data()) != "hello world" { + t.Fatalf("Data = %q, want 'hello world'", string(df.Data())) + } +} + +func TestFramerWriteReadSettings(t *testing.T) { + var buf bytes.Buffer + fw := NewFramer(&buf, nil) + fw.WriteSettings(reqhttp2.Setting{ID: reqhttp2.SettingMaxFrameSize, Val: 16384}) + + fr := NewFramer(nil, &buf) + f, err := fr.ReadFrame() + if err != nil { + t.Fatalf("ReadFrame failed: %v", err) + } + sf, ok := f.(*SettingsFrame) + if !ok { + t.Fatalf("expected *SettingsFrame, got %T", f) + } + if sf.IsAck() { + t.Fatal("should not be ack") + } + if sf.NumSettings() != 1 { + t.Fatalf("NumSettings = %d, want 1", sf.NumSettings()) + } + s := sf.Setting(0) + if s.ID != reqhttp2.SettingMaxFrameSize || s.Val != 16384 { + t.Fatalf("Setting = %v, want {MaxFrameSize, 16384}", s) + } +} + +func TestFramerWriteReadPing(t *testing.T) { + var buf bytes.Buffer + fw := NewFramer(&buf, nil) + data := [8]byte{1, 2, 3, 4, 5, 6, 7, 8} + fw.WritePing(false, data) + + fr := NewFramer(nil, &buf) + f, err := fr.ReadFrame() + if err != nil { + t.Fatalf("ReadFrame failed: %v", err) + } + pf, ok := f.(*PingFrame) + if !ok { + t.Fatalf("expected *PingFrame, got %T", f) + } + if pf.IsAck() { + t.Fatal("should not be ack") + } + if pf.Data != data { + t.Fatalf("Data = %v, want %v", pf.Data, data) + } +} + +func TestFramerWriteReadWindowUpdate(t *testing.T) { + var buf bytes.Buffer + fw := NewFramer(&buf, nil) + fw.WriteWindowUpdate(1, 65535) + + fr := NewFramer(nil, &buf) + f, err := fr.ReadFrame() + if err != nil { + t.Fatalf("ReadFrame failed: %v", err) + } + wf, ok := f.(*WindowUpdateFrame) + if !ok { + t.Fatalf("expected *WindowUpdateFrame, got %T", f) + } + if wf.StreamID != 1 { + t.Fatalf("StreamID = %d, want 1", wf.StreamID) + } + if wf.Increment != 65535 { + t.Fatalf("Increment = %d, want 65535", wf.Increment) + } +} + +func TestFramerWriteReadGoAway(t *testing.T) { + var buf bytes.Buffer + fw := NewFramer(&buf, nil) + fw.WriteGoAway(0, ErrCodeNo, []byte("shutdown")) + + fr := NewFramer(nil, &buf) + f, err := fr.ReadFrame() + if err != nil { + t.Fatalf("ReadFrame failed: %v", err) + } + gf, ok := f.(*GoAwayFrame) + if !ok { + t.Fatalf("expected *GoAwayFrame, got %T", f) + } + if gf.LastStreamID != 0 { + t.Fatalf("LastStreamID = %d, want 0", gf.LastStreamID) + } + if gf.ErrCode != ErrCodeNo { + t.Fatalf("ErrCode = %d, want %d", gf.ErrCode, ErrCodeNo) + } + if string(gf.DebugData()) != "shutdown" { + t.Fatalf("DebugData = %q, want 'shutdown'", string(gf.DebugData())) + } +} + +func TestErrCodeString(t *testing.T) { + tests := []struct { + code ErrCode + expected string + }{ + {ErrCodeNo, "NO_ERROR"}, + {ErrCodeProtocol, "PROTOCOL_ERROR"}, + {ErrCodeFlowControl, "FLOW_CONTROL_ERROR"}, + {ErrCodeStreamClosed, "STREAM_CLOSED"}, + } + for _, tt := range tests { + if got := tt.code.String(); got != tt.expected { + t.Errorf("ErrCode(%d).String() = %q, want %q", uint32(tt.code), got, tt.expected) + } + } +} diff --git a/internal/http2/transport.go b/internal/http2/transport.go index 341c1646..e8ad3bfb 100644 --- a/internal/http2/transport.go +++ b/internal/http2/transport.go @@ -527,7 +527,7 @@ var ( // It returns either a request to retry (either the same request, or a // modified clone), or an error if the request can't be replayed. func shouldRetryRequest(req *http.Request, err error) (*http.Request, error) { - if !canRetryError(err) { + if !CanRetryError(err) { return nil, err } // If the Body is nil (or http.NoBody), it's safe to reuse @@ -558,7 +558,7 @@ func shouldRetryRequest(req *http.Request, err error) (*http.Request, error) { return nil, fmt.Errorf("http2: Transport: cannot retry err [%v] after Request.Body was written; define Request.GetBody to avoid this error", err) } -func canRetryError(err error) bool { +func CanRetryError(err error) bool { if err == errClientConnUnusable || err == errClientConnGotGoAway { return true } diff --git a/internal/http3/body.go b/internal/http3/body.go index db385c26..22096008 100644 --- a/internal/http3/body.go +++ b/internal/http3/body.go @@ -9,10 +9,14 @@ import ( "github.com/quic-go/quic-go" ) -// A Hijacker allows hijacking of the stream creating part of a quic.Conn from a http.ResponseWriter. -// It is used by WebTransport to create WebTransport streams after a session has been established. -type Hijacker interface { - Connection() *Conn +// Settingser allows waiting for and retrieving the peer's HTTP/3 settings. +type Settingser interface { + // ReceivedSettings returns a channel that is closed once the peer's SETTINGS frame was received. + // Settings can be obtained from the Settings method after the channel was closed. + ReceivedSettings() <-chan struct{} + // Settings returns the settings received on this connection. + // It is only valid to call this function after the channel returned by ReceivedSettings was closed. + Settings() *Settings } var errTooMuchData = errors.New("peer sent too much data") diff --git a/internal/http3/client.go b/internal/http3/client.go index f8786129..fc76b271 100644 --- a/internal/http3/client.go +++ b/internal/http3/client.go @@ -84,8 +84,6 @@ func newClientConn( conn *quic.Conn, enableDatagrams bool, additionalSettings map[uint64]uint64, - streamHijacker func(FrameType, quic.ConnectionTracingID, *quic.Stream, error) (hijacked bool, err error), - uniStreamHijacker func(StreamType, quic.ConnectionTracingID, *quic.ReceiveStream, error) (hijacked bool), maxResponseHeaderBytes int, disableCompression bool, logger *slog.Logger, @@ -122,13 +120,14 @@ func newClientConn( c.conn.CloseWithError(quic.ApplicationErrorCode(ErrCodeInternalError), "") } }() - if streamHijacker != nil { - go c.handleBidirectionalStreams(streamHijacker) - } - go c.conn.handleUnidirectionalStreams(uniStreamHijacker) return c } +// handleUnidirectionalStream handles an incoming unidirectional stream. +func (c *ClientConn) handleUnidirectionalStream(str *quic.ReceiveStream) { + c.conn.handleUnidirectionalStream(str) +} + // OpenRequestStream opens a new request stream on the HTTP/3 connection. func (c *ClientConn) OpenRequestStream(ctx context.Context) (*RequestStream, error) { return c.conn.openRequestStream(ctx, c.requestWriter, nil, c.disableCompression, c.maxResponseHeaderBytes) @@ -166,35 +165,14 @@ func (c *ClientConn) setupConn() error { return err } -func (c *ClientConn) handleBidirectionalStreams(streamHijacker func(FrameType, quic.ConnectionTracingID, *quic.Stream, error) (hijacked bool, err error)) { - for { - str, err := c.conn.conn.AcceptStream(context.Background()) - if err != nil { - if c.logger != nil { - c.logger.Debug("accepting bidirectional stream failed", "error", err) - } - return - } - fp := &frameParser{ - r: str, - closeConn: c.conn.CloseWithError, - unknownFrameHandler: func(ft FrameType, e error) (processed bool, err error) { - id := c.conn.Context().Value(quic.ConnectionTracingKey).(quic.ConnectionTracingID) - return streamHijacker(ft, id, str, e) - }, - } - go func() { - if _, err := fp.ParseNext(c.conn.qlogger); err == errHijacked { - return - } - if err != nil { - if c.logger != nil { - c.logger.Debug("error handling stream", "error", err) - } - } - c.conn.CloseWithError(quic.ApplicationErrorCode(ErrCodeFrameUnexpected), "received HTTP/3 frame on bidirectional stream") - }() - } +// HandleBidirectionalStream handles an incoming bidirectional stream. +// According to RFC 9114, the server is not allowed to open bidirectional streams, +// so this method closes the connection with an error. +func (c *ClientConn) HandleBidirectionalStream(str *quic.Stream) { + c.conn.CloseWithError( + quic.ApplicationErrorCode(ErrCodeStreamCreationError), + fmt.Sprintf("server opened bidirectional stream %d", str.StreamID()), + ) } // RoundTrip executes a request and returns a response @@ -435,3 +413,17 @@ func (c *ClientConn) doRequest(req *http.Request, str *RequestStream) (*http.Res func (c *ClientConn) Conn() *Conn { return c.conn } + +// RawClientConn is a low-level HTTP/3 client connection. +// It allows the application to take control of the stream accept loops, +// giving the application the ability to handle streams originating from the server. +// This is useful for implementing WebTransport or other advanced protocols. +type RawClientConn struct { + *ClientConn +} + +// HandleUnidirectionalStream handles an incoming unidirectional stream. +// This should be called for each unidirectional stream accepted from the QUIC connection. +func (c *RawClientConn) HandleUnidirectionalStream(str *quic.ReceiveStream) { + c.handleUnidirectionalStream(str) +} diff --git a/internal/http3/conn.go b/internal/http3/conn.go index 5e20ce6e..b1f09c23 100644 --- a/internal/http3/conn.go +++ b/internal/http3/conn.go @@ -57,6 +57,11 @@ type Conn struct { idleTimer *time.Timer qlogger qlogwriter.Recorder + + // Track received unidirectional streams (only one of each type allowed) + rcvdControlStr atomic.Bool + rcvdQPACKEncoderStr atomic.Bool + rcvdQPACKDecoderStr atomic.Bool } func newConnection( @@ -232,80 +237,48 @@ func (c *Conn) CloseWithError(code quic.ApplicationErrorCode, msg string) error return c.conn.CloseWithError(code, msg) } -func (c *Conn) handleUnidirectionalStreams(hijack func(StreamType, quic.ConnectionTracingID, *quic.ReceiveStream, error) (hijacked bool)) { - var ( - rcvdControlStr atomic.Bool - rcvdQPACKEncoderStr atomic.Bool - rcvdQPACKDecoderStr atomic.Bool - ) - - for { - str, err := c.conn.AcceptUniStream(context.Background()) - if err != nil { - if c.logger != nil { - c.logger.Debug("accepting unidirectional stream failed", "error", err) - } - return +func (c *Conn) handleUnidirectionalStream(str *quic.ReceiveStream) { + streamType, err := quicvarint.Read(quicvarint.NewReader(str)) + if err != nil { + if c.logger != nil { + c.logger.Debug("reading stream type on stream failed", "stream ID", str.StreamID(), "error", err) } - - go func(str *quic.ReceiveStream) { - streamType, err := quicvarint.Read(quicvarint.NewReader(str)) - if err != nil { - id := c.Context().Value(quic.ConnectionTracingKey).(quic.ConnectionTracingID) - if hijack != nil && hijack(StreamType(streamType), id, str, err) { - return - } - if c.logger != nil { - c.logger.Debug("reading stream type on stream failed", "stream ID", str.StreamID(), "error", err) - } - return - } - // We're only interested in the control stream here. - switch streamType { - case streamTypeControlStream: - case streamTypeQPACKEncoderStream: - if isFirst := rcvdQPACKEncoderStr.CompareAndSwap(false, true); !isFirst { - c.CloseWithError(quic.ApplicationErrorCode(ErrCodeStreamCreationError), "duplicate QPACK encoder stream") - } - // Our QPACK implementation doesn't use the dynamic table yet. - return - case streamTypeQPACKDecoderStream: - if isFirst := rcvdQPACKDecoderStr.CompareAndSwap(false, true); !isFirst { - c.CloseWithError(quic.ApplicationErrorCode(ErrCodeStreamCreationError), "duplicate QPACK decoder stream") - } - // Our QPACK implementation doesn't use the dynamic table yet. - return - case streamTypePushStream: - if c.isServer { - // only the server can push - c.CloseWithError(quic.ApplicationErrorCode(ErrCodeStreamCreationError), "") - } else { - // we never increased the Push ID, so we don't expect any push streams - c.CloseWithError(quic.ApplicationErrorCode(ErrCodeIDError), "") - } - return - default: - if hijack != nil { - if hijack( - StreamType(streamType), - c.Context().Value(quic.ConnectionTracingKey).(quic.ConnectionTracingID), - str, - nil, - ) { - return - } - } - str.CancelRead(quic.StreamErrorCode(ErrCodeStreamCreationError)) - return - } - // Only a single control stream is allowed. - if isFirstControlStr := rcvdControlStr.CompareAndSwap(false, true); !isFirstControlStr { - c.conn.CloseWithError(quic.ApplicationErrorCode(ErrCodeStreamCreationError), "duplicate control stream") - return - } - c.handleControlStream(str) - }(str) + return + } + // We're only interested in the control stream here. + switch streamType { + case streamTypeControlStream: + case streamTypeQPACKEncoderStream: + if isFirst := c.rcvdQPACKEncoderStr.CompareAndSwap(false, true); !isFirst { + c.CloseWithError(quic.ApplicationErrorCode(ErrCodeStreamCreationError), "duplicate QPACK encoder stream") + } + // Our QPACK implementation doesn't use the dynamic table yet. + return + case streamTypeQPACKDecoderStream: + if isFirst := c.rcvdQPACKDecoderStr.CompareAndSwap(false, true); !isFirst { + c.CloseWithError(quic.ApplicationErrorCode(ErrCodeStreamCreationError), "duplicate QPACK decoder stream") + } + // Our QPACK implementation doesn't use the dynamic table yet. + return + case streamTypePushStream: + if c.isServer { + // only the server can push + c.CloseWithError(quic.ApplicationErrorCode(ErrCodeStreamCreationError), "") + } else { + // we never increased the Push ID, so we don't expect any push streams + c.CloseWithError(quic.ApplicationErrorCode(ErrCodeIDError), "") + } + return + default: + str.CancelRead(quic.StreamErrorCode(ErrCodeStreamCreationError)) + return + } + // Only a single control stream is allowed. + if isFirstControlStr := c.rcvdControlStr.CompareAndSwap(false, true); !isFirstControlStr { + c.conn.CloseWithError(quic.ApplicationErrorCode(ErrCodeStreamCreationError), "duplicate control stream") + return } + c.handleControlStream(str) } func (c *Conn) handleControlStream(str *quic.ReceiveStream) { @@ -335,7 +308,7 @@ func (c *Conn) handleControlStream(str *quic.ReceiveStream) { // If datagram support was enabled on our side as well as on the server side, // we can expect it to have been negotiated both on the transport and on the HTTP/3 layer. // Note: ConnectionState() will block until the handshake is complete (relevant when using 0-RTT). - if c.enableDatagrams && !c.ConnectionState().SupportsDatagrams { + if c.enableDatagrams && !c.ConnectionState().SupportsDatagrams.Remote { c.CloseWithError(quic.ApplicationErrorCode(ErrCodeSettingsError), "missing QUIC Datagram support") return } diff --git a/internal/http3/frames.go b/internal/http3/frames.go index 879a9f1b..4a0a9e65 100644 --- a/internal/http3/frames.go +++ b/internal/http3/frames.go @@ -16,11 +16,11 @@ import ( // FrameType is the frame type of a HTTP/3 frame type FrameType uint64 -type unknownFrameHandlerFunc func(FrameType, error) (processed bool, err error) - type frame any -var errHijacked = errors.New("hijacked") +// The maximum length of an encoded HTTP/3 frame header is 16: +// The frame has a type and length field, both QUIC varints (maximum 8 bytes in length) +const frameHeaderLen = 16 type countingByteReader struct { quicvarint.Reader @@ -46,10 +46,9 @@ func (r *countingByteReader) Reset() { } type frameParser struct { - r io.Reader - streamID quic.StreamID - closeConn func(quic.ApplicationErrorCode, string) error - unknownFrameHandler unknownFrameHandlerFunc + r io.Reader + streamID quic.StreamID + closeConn func(quic.ApplicationErrorCode, string) error } func (p *frameParser) ParseNext(qlogger qlogwriter.Recorder) (frame, error) { @@ -57,28 +56,8 @@ func (p *frameParser) ParseNext(qlogger qlogwriter.Recorder) (frame, error) { for { t, err := quicvarint.Read(r) if err != nil { - if p.unknownFrameHandler != nil { - hijacked, err := p.unknownFrameHandler(0, err) - if err != nil { - return nil, err - } - if hijacked { - return nil, errHijacked - } - } return nil, err } - // Call the unknownFrameHandler for frames not defined in the HTTP/3 spec - if t > 0xd && p.unknownFrameHandler != nil { - hijacked, err := p.unknownFrameHandler(FrameType(t), nil) - if err != nil { - return nil, err - } - if hijacked { - return nil, errHijacked - } - // If the unknownFrameHandler didn't process the frame, it is our responsibility to skip it. - } l, err := quicvarint.Read(r) if err != nil { return nil, err diff --git a/internal/http3/frames_test.go b/internal/http3/frames_test.go new file mode 100644 index 00000000..74015934 --- /dev/null +++ b/internal/http3/frames_test.go @@ -0,0 +1,154 @@ +package http3 + +import ( + "bytes" + "testing" + + "github.com/quic-go/quic-go" + "github.com/quic-go/quic-go/quicvarint" +) + +func TestFrameParserReservedFrameType(t *testing.T) { + for _, ft := range []uint64{0x2, 0x6, 0x8, 0x9} { + data := quicvarint.Append(nil, ft) + data = quicvarint.Append(data, 6) + data = append(data, []byte("foobar")...) + + var closed bool + fp := frameParser{ + streamID: 42, + r: bytes.NewReader(data), + closeConn: func(quic.ApplicationErrorCode, string) error { + closed = true + return nil + }, + } + _, err := fp.ParseNext(nil) + if err == nil { + t.Fatalf("expected error for reserved frame type %d", ft) + } + if !closed { + t.Fatalf("expected connection to be closed for reserved frame type %d", ft) + } + } +} + +func TestFrameParserUnknownFrameType(t *testing.T) { + data := quicvarint.Append(nil, 0xdead) + data = quicvarint.Append(data, 6) + data = append(data, []byte("foobar")...) + // append a second frame (DATA) to verify parsing continues after unknown frame + data = quicvarint.Append(data, 0x0) // DATA frame + data = quicvarint.Append(data, 5) + data = append(data, []byte("hello")...) + + fp := frameParser{ + streamID: 1, + r: bytes.NewReader(data), + closeConn: func(quic.ApplicationErrorCode, string) error { + return nil + }, + } + // First call should skip the unknown frame and parse the DATA frame + f, err := fp.ParseNext(nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + df, ok := f.(*dataFrame) + if !ok { + t.Fatalf("expected dataFrame, got %T", f) + } + if df.Length != 5 { + t.Fatalf("expected length 5, got %d", df.Length) + } +} + +func TestFrameParserEOF(t *testing.T) { + data := quicvarint.Append(nil, 0x0) // DATA frame + data = quicvarint.Append(data, 6) + data = append(data, []byte("foobar")...) + + // Truncate before the payload — should get an error (EOF or similar) + // At length 0 and 1, varint reading fails with EOF + for i := 0; i <= 1; i++ { + b := make([]byte, i) + copy(b, data[:i]) + fp := frameParser{r: bytes.NewReader(b)} + _, err := fp.ParseNext(nil) + if err == nil { + t.Fatalf("expected error for truncated data at length %d", i) + } + } +} + +func TestDataFrameAppend(t *testing.T) { + f := &dataFrame{Length: 42} + b := f.Append(nil) + // frame type (0x0, 1 byte) + length (42, 1 byte) = 2 bytes + if len(b) != 2 { + t.Fatalf("expected 2 bytes, got %d", len(b)) + } +} + +func TestHeadersFrameAppend(t *testing.T) { + f := &headersFrame{Length: 100} + b := f.Append(nil) + // frame type (0x1, 1 byte) + length (100, 2 bytes since >63) = 3 bytes + if len(b) != 3 { + t.Fatalf("expected 3 bytes, got %d", len(b)) + } +} + +func TestSettingsFrameAppendParse(t *testing.T) { + sf := &settingsFrame{ + MaxFieldSectionSize: 1024, + Datagram: true, + ExtendedConnect: true, + Other: map[uint64]uint64{0x99: 1}, + } + b := sf.Append(nil) + + // Parse it back + r := &countingByteReader{Reader: quicvarint.NewReader(bytes.NewReader(b))} + parsed, err := parseSettingsFrame(r, uint64(len(b)), 0, nil) + if err != nil { + t.Fatalf("failed to parse settings frame: %v", err) + } + if parsed.MaxFieldSectionSize != 1024 { + t.Fatalf("expected MaxFieldSectionSize 1024, got %d", parsed.MaxFieldSectionSize) + } + if !parsed.Datagram { + t.Fatal("expected Datagram to be true") + } + if !parsed.ExtendedConnect { + t.Fatal("expected ExtendedConnect to be true") + } + if parsed.Other[0x99] != 1 { + t.Fatalf("expected Other[0x99]=1, got %d", parsed.Other[0x99]) + } +} + +func TestGoAwayFrameAppendParse(t *testing.T) { + f := &goAwayFrame{StreamID: 42} + b := f.Append(nil) + + // Skip the frame type (0x7) to get to the length + payload + r := &countingByteReader{Reader: quicvarint.NewReader(bytes.NewReader(b))} + // Read frame type + _, err := quicvarint.Read(r) + if err != nil { + t.Fatalf("failed to read frame type: %v", err) + } + // Read length + l, err := quicvarint.Read(r) + if err != nil { + t.Fatalf("failed to read length: %v", err) + } + parsed, err := parseGoAwayFrame(r, l, 0, nil) + if err != nil { + t.Fatalf("failed to parse goaway frame: %v", err) + } + if parsed.StreamID != 42 { + t.Fatalf("expected StreamID 42, got %d", parsed.StreamID) + } +} diff --git a/internal/http3/headers.go b/internal/http3/headers.go index ff1ac659..572ca74c 100644 --- a/internal/http3/headers.go +++ b/internal/http3/headers.go @@ -199,7 +199,7 @@ func updateResponseFromHeaders(rsp *http.Response, decodeFn qpack.DecodeFunc, si rsp.Proto = "HTTP/3.0" rsp.ProtoMajor = 3 rsp.Header = hdr.Headers - processTrailers(rsp) + rsp.Trailer = extractAnnouncedTrailers(rsp.Header) rsp.ContentLength = hdr.ContentLength status, err := strconv.Atoi(hdr.Status) @@ -211,26 +211,27 @@ func updateResponseFromHeaders(rsp *http.Response, decodeFn qpack.DecodeFunc, si return nil } -// processTrailers initializes the rsp.Trailer map, and adds keys for every announced header value. -// The Trailer header is removed from the http.Response.Header map. +// extractAnnouncedTrailers extracts trailer keys from the "Trailer" header. +// It returns a map with the announced keys set to nil values, and removes the "Trailer" header. // It handles both duplicate as well as comma-separated values for the Trailer header. // For example: // // Trailer: Trailer1, Trailer2 // Trailer: Trailer3 // -// Will result in a http.Response.Trailer map containing the keys "Trailer1", "Trailer2", "Trailer3". -func processTrailers(rsp *http.Response) { - rawTrailers, ok := rsp.Header["Trailer"] +// Will result in a map containing the keys "Trailer1", "Trailer2", "Trailer3" with nil values. +func extractAnnouncedTrailers(header http.Header) http.Header { + rawTrailers, ok := header["Trailer"] if !ok { - return + return nil } - rsp.Trailer = make(http.Header) + trailers := make(http.Header) for _, rawVal := range rawTrailers { for _, val := range strings.Split(rawVal, ",") { - rsp.Trailer[http.CanonicalHeaderKey(textproto.TrimString(val))] = nil + trailers[http.CanonicalHeaderKey(textproto.TrimString(val))] = nil } } - delete(rsp.Header, "Trailer") + delete(header, "Trailer") + return trailers } diff --git a/internal/http3/headers_test.go b/internal/http3/headers_test.go new file mode 100644 index 00000000..e5facae5 --- /dev/null +++ b/internal/http3/headers_test.go @@ -0,0 +1,79 @@ +package http3 + +import ( + "net/http" + "testing" +) + +func TestExtractAnnouncedTrailers(t *testing.T) { + tests := []struct { + name string + header http.Header + expected http.Header + }{ + { + name: "no trailer header", + header: http.Header{"Content-Type": []string{"text/html"}}, + expected: nil, + }, + { + name: "single trailer", + header: http.Header{"Trailer": []string{"X-Custom-1"}}, + expected: http.Header{"X-Custom-1": nil}, + }, + { + name: "multiple trailers comma-separated", + header: http.Header{"Trailer": []string{"X-Custom-1, X-Custom-2"}}, + expected: http.Header{"X-Custom-1": nil, "X-Custom-2": nil}, + }, + { + name: "multiple trailers duplicate headers", + header: http.Header{"Trailer": []string{"X-Custom-1"}, "Trailer2": []string{"X-Custom-2"}}, + expected: http.Header{"X-Custom-1": nil}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractAnnouncedTrailers(tt.header) + if tt.expected == nil { + if result != nil { + t.Fatalf("expected nil, got %v", result) + } + return + } + if result == nil { + t.Fatalf("expected %v, got nil", tt.expected) + } + for k := range tt.expected { + if _, ok := result[k]; !ok { + t.Fatalf("expected key %s in result", k) + } + } + // Verify "Trailer" header was removed + if _, ok := tt.header["Trailer"]; ok { + t.Fatal("Trailer header should have been removed") + } + }) + } +} + +func TestExtractAnnouncedTrailersRemovesTrailerHeader(t *testing.T) { + h := http.Header{ + "Content-Type": []string{"text/html"}, + "Trailer": []string{"X-Custom-Trailer"}, + } + result := extractAnnouncedTrailers(h) + if result == nil { + t.Fatal("expected non-nil result") + } + if _, ok := result["X-Custom-Trailer"]; !ok { + t.Fatal("expected X-Custom-Trailer in result") + } + if _, ok := h["Trailer"]; ok { + t.Fatal("Trailer header should have been removed from original header") + } + if _, ok := h["Content-Type"]; !ok { + t.Fatal("Content-Type should still be present") + } +} diff --git a/internal/http3/request_writer.go b/internal/http3/request_writer.go index 187d58b8..fbf0c5e1 100644 --- a/internal/http3/request_writer.go +++ b/internal/http3/request_writer.go @@ -42,7 +42,6 @@ func newRequestWriter() *requestWriter { } func (w *requestWriter) WriteRequestHeader(wr io.Writer, req *http.Request, gzip bool, streamID quic.StreamID, qlogger qlogwriter.Recorder, dumps []*dump.Dumper) error { - // TODO: figure out how to add support for trailers buf := &bytes.Buffer{} if err := w.writeHeaders(buf, req, gzip, streamID, qlogger, dumps); err != nil { return err @@ -61,7 +60,18 @@ func (w *requestWriter) writeHeaders(wr io.Writer, req *http.Request, gzip bool, defer w.encoder.Close() defer w.headerBuf.Reset() - headerFields, err := w.encodeHeaders(req, gzip, "", actualContentLength(req), qlogger != nil, dumps) + var trailers string + if len(req.Trailer) > 0 { + keys := make([]string, 0, len(req.Trailer)) + for k := range req.Trailer { + if httpguts.ValidTrailerHeader(k) { + keys = append(keys, k) + } + } + trailers = strings.Join(keys, ", ") + } + + headerFields, err := w.encodeHeaders(req, gzip, trailers, actualContentLength(req), qlogger != nil, dumps) if err != nil { return err } diff --git a/internal/http3/transport.go b/internal/http3/transport.go index d3b7222b..fd06637e 100644 --- a/internal/http3/transport.go +++ b/internal/http3/transport.go @@ -41,6 +41,7 @@ type RoundTripOpt struct { type clientConn interface { OpenRequestStream(context.Context) (*RequestStream, error) RoundTrip(*http.Request) (*http.Response, error) + handleUnidirectionalStream(*quic.ReceiveStream) } type roundTripperWithCount struct { @@ -99,9 +100,6 @@ type Transport struct { // However, if the user explicitly requested gzip it is not automatically uncompressed. DisableCompression bool - StreamHijacker func(FrameType, quic.ConnectionTracingID, *quic.Stream, error) (hijacked bool, err error) - UniStreamHijacker func(StreamType, quic.ConnectionTracingID, *quic.ReceiveStream, error) (hijacked bool) - Logger *slog.Logger mutex sync.Mutex @@ -133,17 +131,17 @@ var ( func (t *Transport) init() error { if t.newClientConn == nil { t.newClientConn = func(conn *quic.Conn) clientConn { - return newClientConn( + c := newClientConn( t.Options, conn, t.EnableDatagrams, t.AdditionalSettings, - t.StreamHijacker, - t.UniStreamHijacker, t.MaxResponseHeaderBytes, t.DisableCompression, t.Logger, ) + startUnidirectionalStreamAcceptLoop(conn, c) + return c } } if t.QUICConfig == nil { @@ -459,17 +457,48 @@ func (t *Transport) removeClient(hostname string) { // Obtaining a ClientConn is only needed for more advanced use cases, such as // using Extended CONNECT for WebTransport or the various MASQUE protocols. func (t *Transport) NewClientConn(conn *quic.Conn) *ClientConn { - return newClientConn( + c := newClientConn( t.Options, conn, t.EnableDatagrams, t.AdditionalSettings, - t.StreamHijacker, - t.UniStreamHijacker, t.MaxResponseHeaderBytes, t.DisableCompression, t.Logger, ) + startUnidirectionalStreamAcceptLoop(conn, c) + return c +} + +// NewRawClientConn creates a new low-level HTTP/3 client connection on top of a QUIC connection. +// Unlike NewClientConn, the returned RawClientConn allows the application to take control +// of the stream accept loops, by calling HandleUnidirectionalStream for incoming unidirectional +// streams and HandleBidirectionalStream for incoming bidirectional streams. +func (t *Transport) NewRawClientConn(conn *quic.Conn) *RawClientConn { + return &RawClientConn{ + ClientConn: newClientConn( + t.Options, + conn, + t.EnableDatagrams, + t.AdditionalSettings, + t.MaxResponseHeaderBytes, + t.DisableCompression, + t.Logger, + ), + } +} + +// startUnidirectionalStreamAcceptLoop starts the accept loop for unidirectional streams. +func startUnidirectionalStreamAcceptLoop(conn *quic.Conn, cc clientConn) { + go func() { + for { + str, err := conn.AcceptUniStream(context.Background()) + if err != nil { + return + } + go cc.handleUnidirectionalStream(str) + } + }() } // Close closes the QUIC connections that this Transport has used. diff --git a/internal/http3/transport_test.go b/internal/http3/transport_test.go new file mode 100644 index 00000000..9896655a --- /dev/null +++ b/internal/http3/transport_test.go @@ -0,0 +1,141 @@ +package http3 + +import ( + "context" + "crypto/tls" + "net/http" + "testing" + "time" + + "github.com/imroc/req/v3/internal/testcert" + "github.com/quic-go/quic-go" +) + +func TestTransportInit(t *testing.T) { + tr := &Transport{} + // Trigger init by calling RoundTrip with invalid request + // This tests that init() doesn't panic + _, err := tr.RoundTrip(&http.Request{}) + if err == nil { + t.Fatal("expected error for nil URL") + } +} + +func TestTransportInitWithDatagrams(t *testing.T) { + tr := &Transport{ + EnableDatagrams: true, + QUICConfig: &quic.Config{ + EnableDatagrams: true, + }, + } + _, err := tr.RoundTrip(&http.Request{}) + if err == nil { + t.Fatal("expected error for nil URL") + } +} + +func TestTransportInitDatagramMismatch(t *testing.T) { + tr := &Transport{ + EnableDatagrams: true, + QUICConfig: &quic.Config{ + EnableDatagrams: false, + }, + } + _, err := tr.RoundTrip(&http.Request{}) + if err == nil || err.Error() != "HTTP Datagrams enabled, but QUIC Datagrams disabled" { + t.Fatalf("expected datagram mismatch error, got: %v", err) + } +} + +func TestRawClientConnType(t *testing.T) { + // Test that RawClientConn embeds ClientConn correctly + // This is a compile-time type check + type hasClientConn interface { + RoundTrip(*http.Request) (*http.Response, error) + } + var _ hasClientConn = (*RawClientConn)(nil) + var _ hasClientConn = (*ClientConn)(nil) +} + +func TestNewClientConnAndRawClientConn(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + // Create TLS config from test certificates + cert, err := tls.X509KeyPair(testcert.LocalhostCert, testcert.LocalhostKey) + if err != nil { + t.Fatalf("failed to load test cert: %v", err) + } + serverTLSConfig := &tls.Config{ + Certificates: []tls.Certificate{cert}, + NextProtos: []string{"h3"}, + } + + // Start QUIC listener + listener, err := quic.ListenAddr("127.0.0.1:0", serverTLSConfig, &quic.Config{}) + if err != nil { + t.Fatalf("failed to start QUIC listener: %v", err) + } + defer listener.Close() + + serverAddr := listener.Addr().String() + + // Accept connections in background + go func() { + for { + conn, err := listener.Accept(context.Background()) + if err != nil { + return + } + conn.CloseWithError(0, "test done") + } + }() + + // Create client transport + clientTLSConfig := &tls.Config{ + InsecureSkipVerify: true, + NextProtos: []string{"h3"}, + } + + tr := &Transport{ + TLSClientConfig: clientTLSConfig, + QUICConfig: &quic.Config{ + MaxIdleTimeout: 5 * time.Second, + }, + } + defer tr.Close() + + // Test dialing a QUIC connection + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + quicConn, err := quic.DialAddr(ctx, serverAddr, clientTLSConfig, tr.QUICConfig) + if err != nil { + t.Fatalf("failed to dial QUIC: %v", err) + } + defer quicConn.CloseWithError(0, "") + + // Test NewClientConn + clientConn := tr.NewClientConn(quicConn) + if clientConn == nil { + t.Fatal("NewClientConn returned nil") + } + + // Test NewRawClientConn with a new connection + quicConn2, err := quic.DialAddr(ctx, serverAddr, clientTLSConfig, tr.QUICConfig) + if err != nil { + t.Fatalf("failed to dial QUIC for raw conn: %v", err) + } + defer quicConn2.CloseWithError(0, "") + + rawConn := tr.NewRawClientConn(quicConn2) + if rawConn == nil { + t.Fatal("NewRawClientConn returned nil") + } + if rawConn.ClientConn == nil { + t.Fatal("RawClientConn.ClientConn is nil") + } + + t.Log("Successfully created ClientConn and RawClientConn") +} diff --git a/internal/netutil/addr_test.go b/internal/netutil/addr_test.go new file mode 100644 index 00000000..4ee41ec6 --- /dev/null +++ b/internal/netutil/addr_test.go @@ -0,0 +1,58 @@ +package netutil + +import ( + "net/url" + "testing" +) + +func TestAuthorityAddr(t *testing.T) { + tests := []struct { + scheme string + authority string + expected string + }{ + {"https", "example.com", "example.com:443"}, + {"http", "example.com", "example.com:80"}, + {"https", "example.com:8443", "example.com:8443"}, + {"http", "example.com:8080", "example.com:8080"}, + {"https", "[::1]:443", "[::1]:443"}, + {"https", "[::1]", "[::1]:443"}, + } + for _, tt := range tests { + t.Run(tt.scheme+"://"+tt.authority, func(t *testing.T) { + got := AuthorityAddr(tt.scheme, tt.authority) + if got != tt.expected { + t.Errorf("AuthorityAddr(%q, %q) = %q, want %q", + tt.scheme, tt.authority, got, tt.expected) + } + }) + } +} + +func TestAuthorityHostPort(t *testing.T) { + tests := []struct { + scheme string + authority string + host string + port string + }{ + {"https", "example.com", "example.com", "443"}, + {"http", "example.com", "example.com", "80"}, + {"https", "example.com:8443", "example.com", "8443"}, + } + for _, tt := range tests { + host, port := AuthorityHostPort(tt.scheme, tt.authority) + if host != tt.host || port != tt.port { + t.Errorf("AuthorityHostPort(%q, %q) = (%q, %q), want (%q, %q)", + tt.scheme, tt.authority, host, port, tt.host, tt.port) + } + } +} + +func TestAuthorityKey(t *testing.T) { + u := &url.URL{Scheme: "https", Host: "example.com"} + got := AuthorityKey(u) + if got != "https://example.com:443" { + t.Fatalf("AuthorityKey = %q, want https://example.com:443", got) + } +} diff --git a/internal/transport/option_test.go b/internal/transport/option_test.go new file mode 100644 index 00000000..26ccb383 --- /dev/null +++ b/internal/transport/option_test.go @@ -0,0 +1,51 @@ +package transport + +import ( + "crypto/tls" + "testing" +) + +func TestOptionsClone(t *testing.T) { + opt := Options{ + DisableKeepAlives: true, + MaxIdleConns: 10, + MaxConnsPerHost: 5, + IdleConnTimeout: 30000000000, + TLSHandshakeTimeout: 10000000000, + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + ServerName: "example.com", + }, + } + + cloned := opt.Clone() + if cloned.DisableKeepAlives != true { + t.Fatal("DisableKeepAlives mismatch") + } + if cloned.MaxIdleConns != 10 { + t.Fatal("MaxIdleConns mismatch") + } + if cloned.TLSClientConfig == nil { + t.Fatal("TLSClientConfig is nil after clone") + } + if !cloned.TLSClientConfig.InsecureSkipVerify { + t.Fatal("TLSClientConfig.InsecureSkipVerify mismatch") + } + // Verify it's a different pointer + if &cloned.TLSClientConfig == &opt.TLSClientConfig { + t.Fatal("TLSClientConfig should be a new pointer after clone") + } +} + +func TestOptionsCloneNilConfig(t *testing.T) { + opt := Options{ + MaxIdleConns: 10, + } + cloned := opt.Clone() + if cloned.TLSClientConfig != nil { + t.Fatal("TLSClientConfig should be nil") + } + if cloned.MaxIdleConns != 10 { + t.Fatal("MaxIdleConns mismatch") + } +} diff --git a/internal/util/util_test.go b/internal/util/util_test.go new file mode 100644 index 00000000..2edfb657 --- /dev/null +++ b/internal/util/util_test.go @@ -0,0 +1,154 @@ +package util + +import ( + "os" + "testing" +) + +func TestIsJSONType(t *testing.T) { + tests := []struct { + ct string + expected bool + }{ + {"application/json", true}, + {"application/json; charset=utf-8", true}, + {"text/json", true}, + {"application/xml", false}, + {"text/html", false}, + {"", false}, + } + for _, tt := range tests { + if got := IsJSONType(tt.ct); got != tt.expected { + t.Errorf("IsJSONType(%q) = %v, want %v", tt.ct, got, tt.expected) + } + } +} + +func TestIsXMLType(t *testing.T) { + tests := []struct { + ct string + expected bool + }{ + {"application/xml", true}, + {"text/xml; charset=utf-8", true}, + {"application/json", false}, + {"text/html", false}, + {"", false}, + } + for _, tt := range tests { + if got := IsXMLType(tt.ct); got != tt.expected { + t.Errorf("IsXMLType(%q) = %v, want %v", tt.ct, got, tt.expected) + } + } +} + +func TestIsStringEmpty(t *testing.T) { + tests := []struct { + s string + expected bool + }{ + {"", true}, + {" ", true}, + {"\t\n", true}, + {"a", false}, + {" a ", false}, + } + for _, tt := range tests { + if got := IsStringEmpty(tt.s); got != tt.expected { + t.Errorf("IsStringEmpty(%q) = %v, want %v", tt.s, got, tt.expected) + } + } +} + +func TestCutString(t *testing.T) { + tests := []struct { + s, sep string + before string + after string + found bool + }{ + {"a/b/c", "/", "a", "b/c", true}, + {"abc", "/", "abc", "", false}, + // Empty separator: strings.Index returns 0, so it "finds" at position 0 + {"abc", "", "", "abc", true}, + } + for _, tt := range tests { + before, after, found := CutString(tt.s, tt.sep) + if before != tt.before || after != tt.after || found != tt.found { + t.Errorf("CutString(%q, %q) = (%q, %q, %v), want (%q, %q, %v)", + tt.s, tt.sep, before, after, found, tt.before, tt.after, tt.found) + } + } +} + +func TestCutBytes(t *testing.T) { + tests := []struct { + s, sep []byte + before []byte + after []byte + found bool + }{ + {[]byte("a/b/c"), []byte("/"), []byte("a"), []byte("b/c"), true}, + {[]byte("abc"), []byte("/"), []byte("abc"), nil, false}, + } + for _, tt := range tests { + before, after, found := CutBytes(tt.s, tt.sep) + if string(before) != string(tt.before) || string(after) != string(tt.after) || found != tt.found { + t.Errorf("CutBytes(%q, %q) = (%q, %q, %v), want (%q, %q, %v)", + tt.s, tt.sep, before, after, found, tt.before, tt.after, tt.found) + } + } +} + +func TestBasicAuthHeaderValue(t *testing.T) { + got := BasicAuthHeaderValue("user", "pass") + // Basic dXNlcjpwYXNz + if got != "Basic dXNlcjpwYXNz" { + t.Fatalf("BasicAuthHeaderValue = %q", got) + } +} + +func TestGetPointer(t *testing.T) { + type Foo struct{ X int } + var f Foo + p := GetPointer(&f) + if p == nil { + t.Fatal("GetPointer returned nil") + } + // p should be *Foo + if _, ok := p.(*Foo); !ok { + t.Fatalf("GetPointer returned %T, want *Foo", p) + } + + // Non-pointer input + p2 := GetPointer(Foo{}) + if p2 == nil { + t.Fatal("GetPointer returned nil for non-pointer") + } +} + +func TestGetType(t *testing.T) { + type Foo struct{ X int } + f := &Foo{} + if got := GetType(f); got.Name() != "Foo" { + t.Fatalf("GetType = %s, want Foo", got.Name()) + } +} + +func TestCreateDirectory(t *testing.T) { + dir := t.TempDir() + "/testdir" + if err := CreateDirectory(dir); err != nil { + t.Fatalf("CreateDirectory failed: %v", err) + } + info, err := os.Stat(dir) + if err != nil { + t.Fatalf("directory not created: %v", err) + } + if !info.IsDir() { + t.Fatal("not a directory") + } + // Creating again should not error + if err := CreateDirectory(dir); err != nil { + t.Fatalf("CreateDirectory on existing dir failed: %v", err) + } +} diff --git a/pkg/altsvc/altsvc_test.go b/pkg/altsvc/altsvc_test.go new file mode 100644 index 00000000..f77f938d --- /dev/null +++ b/pkg/altsvc/altsvc_test.go @@ -0,0 +1,58 @@ +package altsvc + +import ( + "testing" + "time" +) + +func TestAltSvcJarSetGet(t *testing.T) { + jar := NewAltSvcJar() + as := &AltSvc{ + Protocol: "h3", + Host: "", + Port: "8443", + Expire: time.Now().Add(1 * time.Hour), + } + jar.SetAltSvc("example.com:443", as) + + got := jar.GetAltSvc("example.com:443") + if got == nil { + t.Fatal("expected non-nil AltSvc") + } + if got.Protocol != "h3" { + t.Fatalf("Protocol = %q, want h3", got.Protocol) + } + if got.Port != "8443" { + t.Fatalf("Port = %q, want 8443", got.Port) + } +} + +func TestAltSvcJarExpired(t *testing.T) { + jar := NewAltSvcJar() + as := &AltSvc{ + Protocol: "h3", + Port: "8443", + Expire: time.Now().Add(-1 * time.Hour), // expired + } + jar.SetAltSvc("example.com:443", as) + + got := jar.GetAltSvc("example.com:443") + if got != nil { + t.Fatal("expected nil for expired AltSvc") + } +} + +func TestAltSvcJarEmptyAddr(t *testing.T) { + jar := NewAltSvcJar() + jar.SetAltSvc("", &AltSvc{Protocol: "h3"}) + if got := jar.GetAltSvc(""); got != nil { + t.Fatal("expected nil for empty addr") + } +} + +func TestAltSvcJarNotFound(t *testing.T) { + jar := NewAltSvcJar() + if got := jar.GetAltSvc("nonexistent.com:443"); got != nil { + t.Fatal("expected nil for non-existent addr") + } +} diff --git a/redirect.go b/redirect.go index fcc13e4b..2917729f 100644 --- a/redirect.go +++ b/redirect.go @@ -132,3 +132,32 @@ func AlwaysCopyHeaderRedirectPolicy(headers ...string) RedirectPolicy { return nil } } + +// SensitiveHeadersRedirectPolicy strips the given sensitive headers when the +// redirect target is a different domain. This is useful for custom authentication +// headers (e.g. "X-API-Key", "X-Auth-Token") that are not automatically stripped +// by the standard library on cross-domain redirects. +// +// By default, Go's net/http only strips "Authorization", "Cookie", and +// "Proxy-Authorization" on cross-domain redirects. Custom headers set via +// SetCommonHeader or SetHeader are forwarded to the redirect target, which can +// lead to credential leakage (CWE-200). +// +// For example: +// +// client.SetRedirectPolicy(req.SensitiveHeadersRedirectPolicy("X-API-Key", "X-Auth-Token")) +func SensitiveHeadersRedirectPolicy(headers ...string) RedirectPolicy { + return func(req *http.Request, via []*http.Request) error { + if len(via) == 0 { + return nil + } + // Only strip headers when redirecting to a different domain. + if getDomain(req.URL.Host) == getDomain(via[0].URL.Host) { + return nil + } + for _, header := range headers { + req.Header.Del(header) + } + return nil + } +} diff --git a/response.go b/response.go index 69aee127..2fb3ca1e 100644 --- a/response.go +++ b/response.go @@ -1,6 +1,7 @@ package req import ( + "errors" "io" "net/http" "strings" @@ -180,6 +181,10 @@ func (r *Response) Unmarshal(v any) error { if r.Err != nil { return r.Err } + // error by status-code + if r.IsErrorState() { + return errors.New(r.Status) + } v = util.GetPointer(v) contentType := r.Header.Get("Content-Type") if strings.Contains(contentType, "json") { diff --git a/transport.go b/transport.go index 2f6fd5f0..9053eb51 100644 --- a/transport.go +++ b/transport.go @@ -958,7 +958,7 @@ func (t *Transport) roundTrip(req *http.Request) (resp *http.Response, err error if scheme == "https" && t.forceHttpVersion != h1 { resp, err := t.t2.RoundTripOnlyCachedConn(req) - if err != h2internal.ErrNoCachedConn { + if err != h2internal.ErrNoCachedConn && !h2internal.CanRetryError(err) { return resp, err } req, err = rewindBody(req)