Copilot commented on code in PR #13134:
URL: https://github.com/apache/trafficserver/pull/13134#discussion_r3174688938
##########
tests/gold_tests/logging/verify_milestone_fields.py:
##########
@@ -77,6 +81,15 @@ def validate_line(fields: dict[str, str], line_num: int) ->
list[str]:
if name not in fields:
errors.append(f'line {line_num}: missing field "{name}"')
+ ckh = fields.get('ckh')
+ if ckh is not None:
+ if ckh == '-':
+ errors.append(f'line {line_num}: ckh should not be "-" (cache
lookup was performed)')
+ elif not re.fullmatch(r'[A-Za-z0-9+/]+=*', ckh):
+ errors.append(f'line {line_num}: ckh is not valid base64: {ckh!r}')
+ elif len(ckh) not in (24, 44):
+ errors.append(f'line {line_num}: ckh has unexpected length
{len(ckh)} (expected 24 or 44)')
Review Comment:
The base64 check for ckh uses a permissive regex that can accept strings
that aren't actually valid base64 (e.g., incorrect padding or '=' in the
middle). To make this test more robust against subtle regressions, consider
validating via Python's base64 decoder (e.g., b64decode(..., validate=True))
and/or using a stricter pattern that enforces padding rules.
##########
src/api/InkAPI.cc:
##########
@@ -4477,6 +4477,35 @@ TSHttpTxnCacheLookupUrlSet(TSHttpTxn txnp, TSMBuffer
bufp, TSMLoc obj)
return TS_SUCCESS;
}
+TSReturnCode
+TSHttpTxnCacheKeyDigestGet(TSHttpTxn txnp, char *buffer, int *length)
+{
+ sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
+ sdk_assert(length != nullptr);
+
+ HttpSM *sm = reinterpret_cast<HttpSM *>(txnp);
+ const CryptoHash &hash = sm->get_cache_sm().get_cache_key().hash;
+ constexpr int size = CRYPTO_HASH_SIZE;
+
+ if (hash.is_zero()) {
+ return TS_ERROR;
+ }
+
+ if (buffer == nullptr) {
+ *length = size;
+ return TS_SUCCESS;
+ }
+
+ if (*length < size) {
+ *length = size;
+ return TS_ERROR;
+ }
+
+ memcpy(buffer, hash.u8, size);
+ *length = size;
Review Comment:
TSHttpTxnCacheKeyDigestGet() returns TS_ERROR when the hash is zero but does
not set *length in that path. The header/docs state that *length is set to the
required digest size on return (even on error). Please set *length before
returning TS_ERROR for the no-digest case (and consider ensuring the size-query
behavior matches the doc).
```suggestion
int provided_length = *length;
*length = size;
if (hash.is_zero()) {
return TS_ERROR;
}
if (buffer == nullptr) {
return TS_SUCCESS;
}
if (provided_length < size) {
return TS_ERROR;
}
memcpy(buffer, hash.u8, size);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
