feat(server): 增加 /p 及 /ap 代理接口的单 IP 并发数限制#2375
feat(server): 增加 /p 及 /ap 代理接口的单 IP 并发数限制#2375Roberta001 wants to merge 2 commits intoOpenListTeam:mainfrom
Conversation
Roberta001
changed the title
utafrali
left a comment
utafrali
left a comment
There was a problem hiding this comment.
The core logic is sound: the mutex-protected per-IP counter, deferred decrement, and sentinel values (-1 for unlimited, 0 for disabled) are all correct. The two substantive concerns are IP spoofability via unvalidated proxy headers and the undocumented per-process scoping of the counter in multi-instance setups.
| "message": "Proxy is disabled", | ||
| "data": nil, | ||
| }) | ||
| return |
There was a problem hiding this comment.
c.ClientIP() resolves the client address from X-Forwarded-For / X-Real-IP headers, which a client can freely forge if Gin's trusted proxy list hasn't been configured. An attacker could rotate spoofed IPs in that header on every request and bypass the concurrency limit entirely. Either add a code comment (and a note to the setting description) that engine.SetTrustedProxies(...) must be configured for this limit to have any teeth, or expose a separate admin option to use c.Request.RemoteAddr for strict enforcement (accepting that it breaks deployments behind CDNs/reverse-proxies).
| "github.com/OpenListTeam/OpenList/v4/internal/conf" | ||
| "github.com/OpenListTeam/OpenList/v4/internal/setting" | ||
| "github.com/gin-gonic/gin" | ||
| ) |
There was a problem hiding this comment.
The counters are stored in package-level variables, so in a horizontally-scaled deployment each process maintains its own independent counts. A single IP effectively gets configured_limit * N concurrent requests across N instances. This is not necessarily a bug, but it will surprise operators who set a tight limit and still see it exceeded. Add a comment here (or in the setting's description string in bootstrap/data/setting.go) calling this out, e.g. // NOTE: counts are per-process; in multi-instance deployments the effective limit is limit*N.
| signCheck := middlewares.Down(sign.Verify) | ||
| g.GET("/d/*path", middlewares.PathParse, signCheck, downloadLimiter, handles.Down) | ||
| g.GET("/p/*path", middlewares.PathParse, signCheck, downloadLimiter, handles.Proxy) | ||
| g.GET("/p/*path", middlewares.PathParse, signCheck, downloadLimiter, proxyConcurrencyLimiter, handles.Proxy) |
There was a problem hiding this comment.
For the GET proxy routes, proxyConcurrencyLimiter is placed after downloadLimiter. Requests that are already over the IP limit will be rejected with 429, but only after downloadLimiter has already wrapped the response writer. Swapping the two so the order becomes signCheck → proxyConcurrencyLimiter → downloadLimiter → handler short-circuits rejected requests one step earlier with no change in correctness or security.
| ip := c.ClientIP() | ||
|
|
||
| proxyIPCountsMu.Lock() | ||
| count := proxyIPCounts[ip] |
There was a problem hiding this comment.
The 429 response body is hand-rolled as gin.H{"code": ..., "message": ..., "data": nil}. Double-check that this matches the project's shared error-response helper (if one exists, e.g. common.ErrorResp or similar). Using a different shape here would make client-side error handling inconsistent with every other endpoint in the API.
…ndard error formatting
2 participants
Description / 描述
在/@manage/settings/traffic页下增加了一个配置项proxy_max_concurrent_requests_per_ip，用于控制单个IP请求
/p/和/ap/的并发数Motivation and Context / 背景
解决启用web代理时被多线程下载器刷爆的场景
How Has This Been Tested? / 测试
将Proxy max concurrent requests per ip配置为-1（无限制）或0（完全禁用
/p/和/ap/接口）以外的数，即可限制并发Checklist / 检查清单
我已阅读 CONTRIBUTING 文档。
go fmtor prettier.我已使用
go fmt或 prettier 格式化提交的代码。我已为此 PR 添加了适当的标签（如无权限或需要的标签不存在，请在描述中说明，管理员将后续处理）。
我已在适当情况下使用"Request review"功能请求相关代码作者进行审查。
我已相应更新了相关仓库（若适用）。