Skip to content

Commit 007f2b2

Browse files
authored
feat: sanitize root folder also in php error messages (RSS-Bridge#3262)
1 parent a01c1f6 commit 007f2b2

File tree

5 files changed

+38
-17
lines changed

5 files changed

+38
-17
lines changed

lib/Logger.php

+4-2
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ private static function log(string $level, string $message, array $context = [])
3434
unset($context['e']);
3535
$context['type'] = get_class($e);
3636
$context['code'] = $e->getCode();
37-
$context['message'] = $e->getMessage();
38-
$context['file'] = trim_path_prefix($e->getFile());
37+
$context['message'] = sanitize_root($e->getMessage());
38+
$context['file'] = sanitize_root($e->getFile());
3939
$context['line'] = $e->getLine();
4040
$context['url'] = get_current_url();
4141
$context['trace'] = trace_to_call_points(trace_from_exception($e));
@@ -58,13 +58,15 @@ private static function log(string $level, string $message, array $context = [])
5858
}
5959
}
6060
}
61+
// Intentionally not sanitizing $message
6162
$text = sprintf(
6263
"[%s] rssbridge.%s %s %s\n",
6364
now()->format('Y-m-d H:i:s'),
6465
$level,
6566
$message,
6667
$context ? Json::encode($context) : ''
6768
);
69+
// Log to stderr/stdout whatever that is
6870
error_log($text);
6971
}
7072
}

lib/RssBridge.php

+8-4
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,12 @@ private function run($request): void
3434
if ((error_reporting() & $code) === 0) {
3535
return false;
3636
}
37-
$text = sprintf('%s at %s line %s', $message, trim_path_prefix($file), $line);
38-
// Drop the current frame
37+
$text = sprintf(
38+
'%s at %s line %s',
39+
sanitize_root($message),
40+
sanitize_root($file),
41+
$line
42+
);
3943
Logger::warning($text);
4044
if (Debug::isEnabled()) {
4145
print sprintf("<pre>%s</pre>\n", e($text));
@@ -49,8 +53,8 @@ private function run($request): void
4953
$message = sprintf(
5054
'Fatal Error %s: %s in %s line %s',
5155
$error['type'],
52-
$error['message'],
53-
trim_path_prefix($error['file']),
56+
sanitize_root($error['message']),
57+
sanitize_root($error['file']),
5458
$error['line']
5559
);
5660
Logger::error($message);

lib/utils.php

+13-5
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ function create_sane_exception_message(\Throwable $e): string
5050
return sprintf(
5151
'%s: %s in %s line %s',
5252
get_class($e),
53-
$e->getMessage(),
54-
trim_path_prefix($e->getFile()),
53+
sanitize_root($e->getMessage()),
54+
sanitize_root($e->getFile()),
5555
$e->getLine()
5656
);
5757
}
@@ -74,7 +74,7 @@ function trace_from_exception(\Throwable $e): array
7474
$trace = [];
7575
foreach ($frames as $frame) {
7676
$trace[] = [
77-
'file' => trim_path_prefix($frame['file'] ?? ''),
77+
'file' => sanitize_root($frame['file'] ?? ''),
7878
'line' => $frame['line'] ?? null,
7979
'class' => $frame['class'] ?? null,
8080
'type' => $frame['type'] ?? null,
@@ -121,9 +121,17 @@ function frame_to_call_point(array $frame): string
121121
*
122122
* Example: "/home/davidsf/rss-bridge/index.php" => "index.php"
123123
*/
124-
function trim_path_prefix(string $filePath): string
124+
function sanitize_root(string $filePath): string
125125
{
126-
return mb_substr($filePath, mb_strlen(dirname(__DIR__)) + 1);
126+
// Root folder of the project e.g. /home/satoshi/repos/rss-bridge
127+
$root = dirname(__DIR__);
128+
return _sanitize_path_name($filePath, $root);
129+
}
130+
131+
function _sanitize_path_name(string $s, string $pathName): string
132+
{
133+
// Remove all occurrences of $pathName in the string
134+
return str_replace(["$pathName/", $pathName], '', $s);
127135
}
128136

129137
/**

templates/error.html.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@
1616
</div>
1717

1818
<div>
19-
<strong>Message:</strong> <?= e($e->getMessage()) ?>
19+
<strong>Message:</strong> <?= e(sanitize_root($e->getMessage())) ?>
2020
</div>
2121

2222
<div>
23-
<strong>File:</strong> <?= e(trim_path_prefix($e->getFile())) ?>
23+
<strong>File:</strong> <?= e(sanitize_root($e->getFile())) ?>
2424
</div>
2525

2626
<div>

tests/UtilsTest.php

+11-4
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,17 @@ public function testFileCache()
4141
$sut->purgeCache(-1);
4242
}
4343

44-
public function testTrimFilePath()
44+
public function testSanitizePathName()
4545
{
46-
$this->assertSame('', trim_path_prefix(dirname(__DIR__)));
47-
$this->assertSame('tests', trim_path_prefix(__DIR__));
48-
$this->assertSame('tests/UtilsTest.php', trim_path_prefix(__DIR__ . '/UtilsTest.php'));
46+
$this->assertSame('index.php', _sanitize_path_name('/home/satoshi/rss-bridge/index.php', '/home/satoshi/rss-bridge'));
47+
$this->assertSame('tests/UtilsTest.php', _sanitize_path_name('/home/satoshi/rss-bridge/tests/UtilsTest.php', '/home/satoshi/rss-bridge'));
48+
$this->assertSame('bug in lib/kek.php', _sanitize_path_name('bug in /home/satoshi/rss-bridge/lib/kek.php', '/home/satoshi/rss-bridge'));
49+
}
50+
51+
public function testSanitizePathNameInErrorMessage()
52+
{
53+
$raw = 'Error: Argument 1 passed to foo() must be an instance of kk, string given, called in /home/satoshi/rss-bridge/bridges/RumbleBridge.php';
54+
$sanitized = 'Error: Argument 1 passed to foo() must be an instance of kk, string given, called in bridges/RumbleBridge.php';
55+
$this->assertSame($sanitized, _sanitize_path_name($raw, '/home/satoshi/rss-bridge'));
4956
}
5057
}

0 commit comments

Comments
 (0)