Fix/issue 70 #71
Merged
Fix/issue 70 #71
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
…ence methods - fix: distinguish params/payload parameters in sign_headers method - params: for GET requests only - payload: for POST requests only - feat: add sign_headers_get convenience method for GET requests - feat: add sign_headers_post convenience method for POST requests - docs: update docstrings with clear parameter usage examples
- fix: use params for GET and payload for POST in sign_headers tests - test: add test_sign_headers_get for GET convenience method - test: add test_sign_headers_post for POST convenience method - test: verify timestamp parameter support in convenience methods
- feat: promote sign_headers_get/post as recommended approach - feat: move build_url and build_json_body to recommended section - refactor: move sign_headers unified method to traditional methods section - refactor: collapse traditional single-field generation methods into details - improve: clearer documentation structure for better user experience
Reviewer's Guide为 Xhshow 客户端添加统一的时间戳支持和 header/trace-id 生成辅助函数，扩展加密载荷以接受可选时间戳，引入基于随机数的 Trace ID 工具，并为新的时间戳和请求头 API 编写文档和测试。 带时间戳和 Trace ID 的统一请求头签名顺序图sequenceDiagram
actor Developer
participant Xhshow
participant CryptoProcessor
participant RandomGenerator
Developer->>Xhshow: sign_headers(method, uri, a1_value, xsec_appid, params, payload, timestamp=None)
activate Xhshow
Xhshow->>Xhshow: if timestamp is None:
Xhshow->>Xhshow: timestamp = time.time()
Xhshow->>Xhshow: request_data = params or payload
Xhshow->>Xhshow: sign_xs(method, uri, a1_value, xsec_appid, request_data, timestamp)
activate Xhshow
Xhshow->>Xhshow: _build_content_string(method, uri, request_data)
Xhshow->>Xhshow: _generate_d_value(content_string)
Xhshow->>CryptoProcessor: build_payload_array(d_value, a1_value, xsec_appid, content_string, timestamp)
activate CryptoProcessor
CryptoProcessor-->>Xhshow: payload_array
deactivate CryptoProcessor
Xhshow->>CryptoProcessor: bit_ops.xor_transform_array(payload_array)
activate CryptoProcessor
CryptoProcessor-->>Xhshow: xor_result
deactivate CryptoProcessor
Xhshow->>CryptoProcessor: b64encoder.encode(xor_result)
activate CryptoProcessor
CryptoProcessor-->>Xhshow: x3_signature
deactivate CryptoProcessor
Xhshow->>Xhshow: build x_s from signature_data
Xhshow-->>Xhshow: x_s
deactivate Xhshow
Xhshow->>Xhshow: get_x_t(timestamp)
Xhshow->>Xhshow: x_t = int(timestamp * 1000)
Xhshow->>RandomGenerator: generate_b3_trace_id()
activate RandomGenerator
RandomGenerator-->>Xhshow: x_b3_traceid
deactivate RandomGenerator
Xhshow->>RandomGenerator: generate_xray_trace_id(timestamp_ms=int(timestamp * 1000))
activate RandomGenerator
RandomGenerator-->>Xhshow: x_xray_traceid
deactivate RandomGenerator
Xhshow-->>Developer: headers { x-s, x-t, x-b3-traceid, x-xray-traceid }
deactivate Xhshow
Xhshow、CryptoProcessor、RandomGenerator 和 CryptoConfig 的更新类图classDiagram
class CryptoConfig {
+int MAX_32BIT
+int MAX_SIGNED_32BIT
+int MAX_BYTE
+str STANDARD_BASE64_ALPHABET
+str URLSAFE_BASE64_ALPHABET
+str X3_PREFIX
+str XYS_PREFIX
+str HEX_CHARS
+int XRAY_TRACE_ID_SEQ_MAX
+int XRAY_TRACE_ID_TIMESTAMP_SHIFT
+int XRAY_TRACE_ID_PART1_LENGTH
+int XRAY_TRACE_ID_PART2_LENGTH
+int B3_TRACE_ID_LENGTH
+CryptoConfig with_overrides(kwargs)
}
class RandomGenerator {
-CryptoConfig config
+list~int~ generate_random_bytes(byte_count)
+int generate_random_byte_in_range(min_val, max_val)
+int generate_random_int()
+str generate_b3_trace_id()
+str generate_xray_trace_id(timestamp, seq)
}
class CryptoProcessor {
+CryptoConfig config
+RandomGenerator random_gen
+Any bit_ops
+Any b64encoder
+list~int~ build_payload_array(d_value, a1_value, app_identifier, string_param, timestamp)
}
class Xhshow {
+CryptoConfig config
+CryptoProcessor crypto_processor
+RandomGenerator random_generator
+__init__(config)
+str _build_content_string(method, uri, payload)
+str _build_signature(d_value, a1_value, xsec_appid, string_param, timestamp)
+str sign_xs(method, uri, a1_value, xsec_appid, payload, timestamp)
+str sign_xs_get(uri, a1_value, xsec_appid, params, timestamp)
+str sign_xs_post(uri, a1_value, xsec_appid, payload, timestamp)
+bytearray decode_x3(x3_signature)
+str build_url(base_url, params)
+str build_json_body(payload)
+str get_b3_trace_id()
+str get_xray_trace_id(timestamp, seq)
+int get_x_t(timestamp)
+dict~str,str~ sign_headers(method, uri, a1_value, xsec_appid, params, payload, timestamp)
+dict~str,str~ sign_headers_get(uri, a1_value, xsec_appid, params, timestamp)
+dict~str,str~ sign_headers_post(uri, a1_value, xsec_appid, payload, timestamp)
}
CryptoProcessor --> CryptoConfig : uses
CryptoProcessor --> RandomGenerator : uses
RandomGenerator --> CryptoConfig : uses
Xhshow --> CryptoConfig : holds
Xhshow --> CryptoProcessor : holds
Xhshow --> RandomGenerator : holds
文件级改动
与关联 Issue 的对照评估
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideAdd unified timestamp support and header/trace-id generation helpers to the Xhshow client, extend the crypto payload to accept an optional timestamp, introduce random-based trace ID utilities, and document and test the new timestamp and header APIs. Sequence diagram for unified header signing with timestamp and trace IDssequenceDiagram
actor Developer
participant Xhshow
participant CryptoProcessor
participant RandomGenerator
Developer->>Xhshow: sign_headers(method, uri, a1_value, xsec_appid, params, payload, timestamp=None)
activate Xhshow
Xhshow->>Xhshow: if timestamp is None:
Xhshow->>Xhshow: timestamp = time.time()
Xhshow->>Xhshow: request_data = params or payload
Xhshow->>Xhshow: sign_xs(method, uri, a1_value, xsec_appid, request_data, timestamp)
activate Xhshow
Xhshow->>Xhshow: _build_content_string(method, uri, request_data)
Xhshow->>Xhshow: _generate_d_value(content_string)
Xhshow->>CryptoProcessor: build_payload_array(d_value, a1_value, xsec_appid, content_string, timestamp)
activate CryptoProcessor
CryptoProcessor-->>Xhshow: payload_array
deactivate CryptoProcessor
Xhshow->>CryptoProcessor: bit_ops.xor_transform_array(payload_array)
activate CryptoProcessor
CryptoProcessor-->>Xhshow: xor_result
deactivate CryptoProcessor
Xhshow->>CryptoProcessor: b64encoder.encode(xor_result)
activate CryptoProcessor
CryptoProcessor-->>Xhshow: x3_signature
deactivate CryptoProcessor
Xhshow->>Xhshow: build x_s from signature_data
Xhshow-->>Xhshow: x_s
deactivate Xhshow
Xhshow->>Xhshow: get_x_t(timestamp)
Xhshow->>Xhshow: x_t = int(timestamp * 1000)
Xhshow->>RandomGenerator: generate_b3_trace_id()
activate RandomGenerator
RandomGenerator-->>Xhshow: x_b3_traceid
deactivate RandomGenerator
Xhshow->>RandomGenerator: generate_xray_trace_id(timestamp_ms=int(timestamp * 1000))
activate RandomGenerator
RandomGenerator-->>Xhshow: x_xray_traceid
deactivate RandomGenerator
Xhshow-->>Developer: headers { x-s, x-t, x-b3-traceid, x-xray-traceid }
deactivate Xhshow
Updated class diagram for Xhshow, CryptoProcessor, RandomGenerator, and CryptoConfigclassDiagram
class CryptoConfig {
+int MAX_32BIT
+int MAX_SIGNED_32BIT
+int MAX_BYTE
+str STANDARD_BASE64_ALPHABET
+str URLSAFE_BASE64_ALPHABET
+str X3_PREFIX
+str XYS_PREFIX
+str HEX_CHARS
+int XRAY_TRACE_ID_SEQ_MAX
+int XRAY_TRACE_ID_TIMESTAMP_SHIFT
+int XRAY_TRACE_ID_PART1_LENGTH
+int XRAY_TRACE_ID_PART2_LENGTH
+int B3_TRACE_ID_LENGTH
+CryptoConfig with_overrides(kwargs)
}
class RandomGenerator {
-CryptoConfig config
+list~int~ generate_random_bytes(byte_count)
+int generate_random_byte_in_range(min_val, max_val)
+int generate_random_int()
+str generate_b3_trace_id()
+str generate_xray_trace_id(timestamp, seq)
}
class CryptoProcessor {
+CryptoConfig config
+RandomGenerator random_gen
+Any bit_ops
+Any b64encoder
+list~int~ build_payload_array(d_value, a1_value, app_identifier, string_param, timestamp)
}
class Xhshow {
+CryptoConfig config
+CryptoProcessor crypto_processor
+RandomGenerator random_generator
+__init__(config)
+str _build_content_string(method, uri, payload)
+str _build_signature(d_value, a1_value, xsec_appid, string_param, timestamp)
+str sign_xs(method, uri, a1_value, xsec_appid, payload, timestamp)
+str sign_xs_get(uri, a1_value, xsec_appid, params, timestamp)
+str sign_xs_post(uri, a1_value, xsec_appid, payload, timestamp)
+bytearray decode_x3(x3_signature)
+str build_url(base_url, params)
+str build_json_body(payload)
+str get_b3_trace_id()
+str get_xray_trace_id(timestamp, seq)
+int get_x_t(timestamp)
+dict~str,str~ sign_headers(method, uri, a1_value, xsec_appid, params, payload, timestamp)
+dict~str,str~ sign_headers_get(uri, a1_value, xsec_appid, params, timestamp)
+dict~str,str~ sign_headers_post(uri, a1_value, xsec_appid, payload, timestamp)
}
CryptoProcessor --> CryptoConfig : uses
CryptoProcessor --> RandomGenerator : uses
RandomGenerator --> CryptoConfig : uses
Xhshow --> CryptoConfig : holds
Xhshow --> CryptoProcessor : holds
Xhshow --> RandomGenerator : holds
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
用于 AI 代理的提示词
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/xhshow/client.py:377-386` </location>
<code_context>
+ if timestamp is None:
+ timestamp = time.time()
+
+ # Use params for GET, payload for POST
+ request_data = params if method.upper() == "GET" else payload
+
+ x_s = self.sign_xs(method, uri, a1_value, xsec_appid, request_data, timestamp)
</code_context>
<issue_to_address>
**suggestion:** 在 `sign_headers` 中增加对不支持的 HTTP 方法以及不匹配的 params/payload 使用方式的保护。
目前任何非 `"GET"` 的方法在签名时都会被当作 `POST` 处理，而且当同时传入 `params` 和 `payload` 时也不会报错。为避免静默产生错误签名，建议显式地对 `"GET"`/`"POST"` 分支处理，对其他方法抛出 `ValueError`，并校验 `GET` 请求只使用 `params`，`POST` 请求只使用 `payload`。
```suggestion
>>> headers.keys()
dict_keys(['x-s', 'x-t', 'x-b3-traceid', 'x-xray-traceid'])
"""
if timestamp is None:
timestamp = time.time()
method_upper = method.upper()
if method_upper not in ("GET", "POST"):
raise ValueError(f"Unsupported HTTP method for signing: {method!r}")
if method_upper == "GET":
if params is not None and payload is not None:
raise ValueError(
"For GET requests, provide request data via 'params' only; "
"'payload' must be None."
)
if payload is not None:
raise ValueError(
"GET requests must not use 'payload'; use 'params' instead."
)
request_data = params
else: # POST
if params is not None and payload is not None:
raise ValueError(
"For POST requests, provide request data via 'payload' only; "
"'params' must be None."
)
if params is not None:
raise ValueError(
"POST requests must not use 'params'; use 'payload' instead."
)
request_data = payload
x_s = self.sign_xs(method_upper, uri, a1_value, xsec_appid, request_data, timestamp)
```
</issue_to_address>
### Comment 2
<location> `tests/test_crypto.py:426-435` </location>
<code_context>
+ def test_trace_id_generation(self):
</code_context>
<issue_to_address>
**suggestion (testing):** 为 xray trace ID 的 seq 范围（0 和最大值）添加边界测试。
目前的 `test_trace_id_generation` 只检测了一个自定义的 `seq` 值，而没有覆盖文档中提到的边界值（0 和 `2^23-1`）。请扩展此测试（或新增一个）调用 `get_xray_trace_id(timestamp=custom_ts, seq=0)` 和 `get_xray_trace_id(timestamp=custom_ts, seq=8388607)`，并断言它们依然返回 32 个字符长度的十六进制字符串。你也可以断言在相同的 timestamp 下，`seq=0` 与 `seq=max` 生成的 ID 的前 16 个字符是不同的，以确认 `seq` 被编码进了 ID 中。
</issue_to_address>
### Comment 3
<location> `tests/test_crypto.py:434-442` </location>
<code_context>
def test_trace_id_generation(self):
"""测试 Trace ID 生成"""
import time
client = Xhshow()
# Test b3 trace id
b3_id = client.get_b3_trace_id()
assert isinstance(b3_id, str)
assert len(b3_id) == 16
assert all(c in "0123456789abcdef" for c in b3_id)
# Test xray trace id with default timestamp
xray_id1 = client.get_xray_trace_id()
assert isinstance(xray_id1, str)
assert len(xray_id1) == 32
assert all(c in "0123456789abcdef" for c in xray_id1)
# Test xray trace id with custom timestamp
custom_ts = int(time.time() * 1000)
xray_id2 = client.get_xray_trace_id(timestamp=custom_ts)
assert isinstance(xray_id2, str)
assert len(xray_id2) == 32
# Test xray trace id with custom timestamp and seq
xray_id3 = client.get_xray_trace_id(timestamp=custom_ts, seq=12345)
assert isinstance(xray_id3, str)
assert len(xray_id3) == 32
</code_context>
<issue_to_address>
**issue (code-quality):** 将重复代码提取到一个单独的方法中（[`extract-duplicate-method`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/extract-duplicate-method/)）。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/xhshow/client.py:377-386` </location>
<code_context>
+ if timestamp is None:
+ timestamp = time.time()
+
+ # Use params for GET, payload for POST
+ request_data = params if method.upper() == "GET" else payload
+
+ x_s = self.sign_xs(method, uri, a1_value, xsec_appid, request_data, timestamp)
</code_context>
<issue_to_address>
**suggestion:** Guard against unsupported HTTP methods and mismatched params/payload usage in `sign_headers`.
Currently any non-`"GET"` method is treated like `POST` for signing, and there’s no error if both `params` and `payload` are passed. To avoid silent mis-signing, explicitly branch on `"GET"`/`"POST"`, raise `ValueError` for other methods, and validate that only `params` is used with `GET` and only `payload` with `POST`.
```suggestion
>>> headers.keys()
dict_keys(['x-s', 'x-t', 'x-b3-traceid', 'x-xray-traceid'])
"""
if timestamp is None:
timestamp = time.time()
method_upper = method.upper()
if method_upper not in ("GET", "POST"):
raise ValueError(f"Unsupported HTTP method for signing: {method!r}")
if method_upper == "GET":
if params is not None and payload is not None:
raise ValueError(
"For GET requests, provide request data via 'params' only; "
"'payload' must be None."
)
if payload is not None:
raise ValueError(
"GET requests must not use 'payload'; use 'params' instead."
)
request_data = params
else: # POST
if params is not None and payload is not None:
raise ValueError(
"For POST requests, provide request data via 'payload' only; "
"'params' must be None."
)
if params is not None:
raise ValueError(
"POST requests must not use 'params'; use 'payload' instead."
)
request_data = payload
x_s = self.sign_xs(method_upper, uri, a1_value, xsec_appid, request_data, timestamp)
```
</issue_to_address>
### Comment 2
<location> `tests/test_crypto.py:426-435` </location>
<code_context>
+ def test_trace_id_generation(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Add boundary tests for xray trace ID seq range (0 and max)
The existing `test_trace_id_generation` only checks a custom `seq` value, but not the documented boundaries (0 and `2^23-1`). Please extend this test (or add a new one) to call `get_xray_trace_id(timestamp=custom_ts, seq=0)` and `get_xray_trace_id(timestamp=custom_ts, seq=8388607)` and assert they still return 32-char hex strings. You may also want to assert that the first 16 characters differ between `seq=0` and `seq=max` for the same timestamp to confirm `seq` is encoded into the ID.
</issue_to_address>
### Comment 3
<location> `tests/test_crypto.py:434-442` </location>
<code_context>
def test_trace_id_generation(self):
"""测试 Trace ID 生成"""
import time
client = Xhshow()
# Test b3 trace id
b3_id = client.get_b3_trace_id()
assert isinstance(b3_id, str)
assert len(b3_id) == 16
assert all(c in "0123456789abcdef" for c in b3_id)
# Test xray trace id with default timestamp
xray_id1 = client.get_xray_trace_id()
assert isinstance(xray_id1, str)
assert len(xray_id1) == 32
assert all(c in "0123456789abcdef" for c in xray_id1)
# Test xray trace id with custom timestamp
custom_ts = int(time.time() * 1000)
xray_id2 = client.get_xray_trace_id(timestamp=custom_ts)
assert isinstance(xray_id2, str)
assert len(xray_id2) == 32
# Test xray trace id with custom timestamp and seq
xray_id3 = client.get_xray_trace_id(timestamp=custom_ts, seq=12345)
assert isinstance(xray_id3, str)
assert len(xray_id3) == 32
</code_context>
<issue_to_address>
**issue (code-quality):** Extract duplicate code into method ([`extract-duplicate-method`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/extract-duplicate-method/))
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- validate GET requests only use params, not payload - validate POST requests only use payload, not params - raise ValueError for unsupported HTTP methods (only GET/POST allowed) - add comprehensive test coverage for parameter validation - improve error messages for better developer experience
Closed
* Add the cryptodemo library, using a custom encode_to_b64 method instead of the official standard base64 method. Add some initialization parameters to the configuration file to generate fingerprints. Perhaps using customer_encoder to replace the encoder file name is better than directly using the encoder name, since encoders are a native Python method. * Add the cryptodemo library, using a custom encode_to_b64 method instead of the official standard base64 method. Add some initialization parameters to the configuration file to generate fingerprints. ADD CRC32_encrypt for gen xs-common Perhaps using customer_encoder to replace the encoder file name is better than directly using the encoder name, since encoders are a native Python method. Use case: cookie = "you cookie dict or string" xs_common = client.sign_xsc(cookie) * style: format code to match project standards * refactor(core): split fingerprint generation into separate modules * perf(encoder): optimize base64 encoding with cached translation tables * feat(client): add unified cookie parsing and x-s-common signature support * test: add comprehensive cookie parsing tests and update crypto tests * chore(crc32): remove example code comments * docs: update API documentation for cookie-based authentication * chore(deps): update project dependencies --------- Co-authored-by: Cloxl <cloxl@cloxl.com>
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.
Fixes #70
Summary by Sourcery
添加具备时间戳感知能力的签名与请求头生成功能，引入 Trace ID 工具方法，并补充推荐使用模式的文档。
New Features:
x-b3-traceid、x-xray-traceid和x-t请求头的值，并为 GET 和 POST 请求构建完整的已签名请求头。Enhancements:
Tests:
x-t请求头计算，以及新的请求头签名便捷方法。Original summary in English
Summary by Sourcery
Add timestamp-aware signing and header generation capabilities, along with trace ID utilities, and document the recommended usage patterns.
New Features:
Enhancements:
Tests: