fix(xueqiu): validate hot-stock limits - #620
Closed
alooshxl wants to merge 1 commit into
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
get_hot_stocks()documentslimit: 最多返回条数(上限 50)but never enforces it. The raw value goes straight into the request and the slice:So on
main:get_hot_stocks(limit=500)requestssize=500and returns every item the endpoint sends back, above the documented maximum.get_hot_stocks(limit=0)still performs a network request, then returns[].get_hot_stocks(limit=-1)requestssize=-1and, becauseitems[:-1]drops the last element, silently returns a truncated list instead of reporting the bad argument. Stubbing_get_jsonwith a 60-item payload againstmainproduces?size=-1&type=10and 59 returned items.The sibling method
get_hot_posts()already handles all three cases; the guards were added in 31b028f (fix(xueqiu): honor and clamp hot-post limits), merged via #578.Change
agent_reach/channels/xueqiu.py: apply the same three guards accepted forget_hot_posts()toget_hot_stocks()— reject a negative limit withValueError("limit must be non-negative"), clamp withmin(limit, 50), and return[]for zero before any request. The normalized value then drives both thesizequery parameter and the result slice.tests/test_xueqiu_channel.py: four offline regressions beside the existing hot-stock tests, covering the requested size passing through unchanged, clamping tosize=50with at most 50 shaped results from a 60-item payload, and the zero/negative cases with a_get_jsondouble that fails if the network path is reached.stock_type, ranking, thecode/symbolfallback, and every other Xueqiu behavior are untouched.Verification
Run on this branch (Windows, Python 3.14.5, pytest 9.1.1, ruff 0.16.3, mypy 2.3.0):
python -m pytest tests/test_xueqiu_channel.py -vpython -m ruff check agent_reach/channels/xueqiu.py tests/test_xueqiu_channel.pypython -m mypy agent_reach/channels/xueqiu.pypython -m ruff check agent_reach testspython -m mypy agent_reachgit diff --checkpython -m pytest tests/ -vThe three boundary tests were confirmed to fail before the change (
size=500, and the zero/negative cases reaching the request double) and to pass after it.The 4 failures are pre-existing in this local environment and unrelated to this change: the symlink security tests in
tests/test_channels.pyandtests/test_reddit_channel.pyraiseOSError: [WinError 1314] A required privilege is not held by the clientbecause this Windows account cannot create symlinks. They fail identically onmainbefore this change (baseline: 4 failed, 554 passed, 28 skipped), and #616 addresses them separately.Scope
get_hot_stocks()changes;get_hot_posts(),search_stock(), quote retrieval, and cookie handling are untouched._get_jsonstill propagate unchanged.