Skip to content

Commit ce21b4b

Browse files
authored
Merge pull request #261: Fix false positive error detection when JSON data contains ClickHouse exception text
Fix false positive error detection when JSON data contains ClickHouse exception text
2 parents ab164a6 + c725389 commit ce21b4b

2 files changed

Lines changed: 45 additions & 1 deletion

File tree

src/Statement.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,15 @@ private function hasErrorClickhouse(string $body, ?string $contentType): bool {
111111

112112
if (strlen($body) > 4096) {
113113
$tail = substr($body, -4096);
114-
return preg_match(self::CLICKHOUSE_ERROR_REGEX, $tail) === 1;
114+
if (preg_match(self::CLICKHOUSE_ERROR_REGEX, $tail) === 1) {
115+
// Regex also matches if the actual data contains a ClickHouse error
116+
// string. Use json_validate() to confirm the response is truly broken.
117+
if (function_exists('json_validate')) {
118+
return !json_validate($body);
119+
}
120+
return true;
121+
}
122+
return false;
115123
}
116124

117125
try {

tests/LargeStreamTest.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,42 @@ public function testLargeBodyWithErrorAtEnd(): void
6565
$this->assertTrue($statement->isError());
6666
}
6767

68+
/**
69+
* Large body with valid JSON containing ClickHouse error text as data.
70+
*/
71+
public function testLargeJsonWithErrorPatternInDataIsNotError(): void
72+
{
73+
if (!function_exists('json_validate')) {
74+
$this->markTestSkipped('json_validate() not available');
75+
}
76+
77+
$rows = [];
78+
for ($i = 0; $i < 100; $i++) {
79+
$rows[] = '{"id":' . $i . ',"message":"Code: 60. DB::Exception: Table default.xxx doesn\'t exist. (UNKNOWN_TABLE) (version 24.3.2.23 (official build))"}';
80+
}
81+
$body = '{"meta":[{"name":"id","type":"UInt64"},{"name":"message","type":"String"}],'
82+
. '"data":[' . implode(',', $rows) . '],'
83+
. '"rows":100,'
84+
. '"statistics":{"elapsed":0.001,"rows_read":100,"bytes_read":4096}}';
85+
86+
// Ensure body exceeds the 4096-byte threshold
87+
$this->assertGreaterThan(4096, strlen($body));
88+
89+
$responseMock = $this->createMock(CurlerResponse::class);
90+
$responseMock->method('http_code')->willReturn(200);
91+
$responseMock->method('error_no')->willReturn(0);
92+
$responseMock->method('content_type')->willReturn('application/json; charset=UTF-8');
93+
$responseMock->method('body')->willReturn($body);
94+
95+
$requestMock = $this->createMock(CurlerRequest::class);
96+
$requestMock->method('response')->willReturn($responseMock);
97+
$requestMock->method('isResponseExists')->willReturn(true);
98+
99+
$statement = new Statement($requestMock);
100+
101+
$this->assertFalse($statement->isError());
102+
}
103+
68104
/**
69105
* Small body with valid JSON should still be checked for JSON validity.
70106
*/

0 commit comments

Comments
 (0)