Revert "Revert "Feat: A SQL function obfuscateQuery""#101027
Revert "Revert "Feat: A SQL function obfuscateQuery""#101027alexey-milovidov wants to merge 11 commits intomasterfrom
Conversation
|
Workflow [PR], commit [ea704bb] Summary: ❌
AI ReviewSummaryThis PR re-introduces PR Metadata
Suggested replacement text in PR description:
ClickHouse Rules
Final Verdict
|
|
|
||
| } | ||
|
|
||
| REGISTER_FUNCTION(obfuscateQuery) |
There was a problem hiding this comment.
The PR body does not follow .github/PULL_REQUEST_TEMPLATE.md: Changelog category and Changelog entry are missing. Since this PR re-introduces user-facing SQL functions (obfuscateQuery / obfuscateQueryWithSeed), please fill template metadata with a concrete user-readable changelog entry.
The ASan crash (stack-buffer-overflow) was caused by accessing `src_pos[-1]` on the first character of the identifier in the CamelCase split logic. For input like `'Ab'`, the first character `'A'` is uppercase and alphanumeric, so `word_has_alphanumerics` is true, and the code tried to check whether the previous character is lowercase — but there is no previous character, reading one byte before the buffer. Fix: add a bounds check `src_pos > src.data()` before the `src_pos[-1]` access. https://s3.amazonaws.com/clickhouse-test-reports/PRs/100683/14d36b8a2b692110d6a5f5eb898d4a37a3a612ca/unit_tests_asan_ubsan/job.log Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…flows to zero `readIntText` can overflow a `uint64_t` for very large numeric literals, resulting in `num == 0`. This then calls `bitScanReverse(0)` which triggers `assert(x != 0)` in debug builds. Handle the overflow case by using the obfuscated hash directly. Found by the AST fuzzer in CI: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=101027&sha=f9d39a5d7a9cd9d194db7593f4a84a0ebfd15e05&name_0=PR&name_1=AST%20fuzzer%20%28amd_debug%2C%20targeted%29 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
|
||
| obfuscated = (1ULL << log2) + obfuscated % (1ULL << log2); | ||
| writeIntText(obfuscated, result); | ||
| if (num == 0) |
There was a problem hiding this comment.
readIntText overflows to num == 0, but there is no regression test covering it.
Please add a stateless test that exercises a very large integer literal (larger than UInt64) through obfuscateQueryWithSeed to lock this behavior and prevent regressions in this branch.
Exercise the code path where `readIntText` overflows `uint64_t` to zero for very large numeric literals, as requested in review feedback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
LLVM Coverage Report
Changed lines: 94.72% (251/265) · Uncovered code |
Reverts #100711