Skip to content

Commit fa0add9

Browse files
committed
debug, help: escape </xmp so values can't break out of the wrapper
1 parent d40f45d commit fa0add9

5 files changed

Lines changed: 74 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ These still work, they're just no longer featured in the docs - no changes requi
159159

160160
### Other
161161

162+
- The `<xmp>` wrapper on `help()`'s web output escapes a literal `</xmp` as
163+
`<\/xmp`, same as CMSB's `xmp_safe()` - keeps the shared helper in sync
164+
with SmartArray's `debug()` fix (the help text itself is static)
162165
- Misc internal code cleanup and modernization
163166

164167
## [2.6.3] - 2026-04-27

docs/method-reference.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,15 +97,15 @@ default when a field is empty.*
9797
### [Require a Value](conditionals-and-error-checking.md#requiring-a-value---or404-ordie-orthrow-orredirect)
9898

9999
*Use these for values that must exist, like a record ID from the URL. If the
100-
value is missing (null or "") they stop the page; otherwise they do nothing and
101-
the chain continues. Zero counts as present.*
100+
value is missing (null or "") they stop the page; otherwise the chain
101+
continues. Zero counts as present.*
102102

103103
| Method | Description |
104104
|-------------------------|--------------------------------------------------------------------------------------|
105105
| `->orDie($text)` | Outputs the message and exits |
106106
| `->or404($text = null)` | Outputs a 404 header and the message (default: standard not-found text), then exits |
107107
| `->orThrow($text)` | Throws a RuntimeException with the message |
108-
| `->orRedirect($url)` | Redirects to `$url` and exits (throws RuntimeException if headers were already sent) |
108+
| `->orRedirect($url)` | Redirects to `$url` and exits. The headers-already-sent check throws even when the value is present, so misuse fails on the first request |
109109

110110
### [Value Checks](conditionals-and-error-checking.md#truefalse-checks---isempty-isnotempty-ismissing-isnull)
111111

src/SharedHelpers.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ private static function xmpWrap(string $output): string
133133
return $plain;
134134
}
135135

136-
return "\n<xmp>\n$output\n</xmp>\n";
136+
// escape "</xmp" so output can't break out of the block, same as CMSB's xmp_safe()
137+
return "\n<xmp>\n" . str_ireplace('</xmp', '<\/xmp', $output) . "\n</xmp>\n";
137138
}
138139

139140
}

tests/Support/bin/xmp-breakout.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
/**
5+
* Web target for HelpTest: prints xmpWrap() output for text holding a
6+
* literal </xmp> closing tag. Run under PHP's built-in server (php -S),
7+
* whose cli-server SAPI takes the <xmp>-wrapping web branch that CLI
8+
* tests can't reach. xmpWrap() is private, so this calls it by reflection
9+
* (help()'s own output is static doc links, nothing injectable).
10+
*/
11+
12+
require dirname(__DIR__, 3) . '/vendor/autoload.php';
13+
14+
$xmpWrap = new ReflectionMethod(\Itools\SmartString\SmartString::class, 'xmpWrap');
15+
echo $xmpWrap->invoke(null, '</xmp><script>alert(1)</script>');

tests/Unit/HelpTest.php

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99
/**
1010
* help() is deprecated: it prints links to the online docs, plain (no <xmp>)
1111
* on CLI. The <xmp> wrap only happens for text/html web responses, which
12-
* can't be simulated in-process (xmpWrap reads PHP_SAPI).
12+
* can't be simulated in-process (xmpWrap reads PHP_SAPI); one test reaches
13+
* it through PHP's built-in server.
1314
*
1415
* n/a dimensions: encoding, global settings, immutability, argument matrix
1516
* ($value passes through untouched by design).
@@ -39,4 +40,53 @@ public function testHelpReturnsValuePassthroughOnBothCallForms(): void
3940
$this->assertSame('original value', $instanceResult);
4041
$this->assertSame($staticOutput, $instanceOutput);
4142
}
43+
44+
/**
45+
* The <xmp> web branch is reachable under PHP's built-in server (SAPI
46+
* cli-server), so this is the one test that asserts the wrapped path: a
47+
* literal </xmp> can't end the block early - it displays as <\/xmp>, the
48+
* same escaping as CMSB's xmp_safe().
49+
*/
50+
public function testXmpWrapEscapesXmpClosingTagOnWebResponses(): void
51+
{
52+
$body = $this->requestViaBuiltInServer('xmp-breakout.php');
53+
54+
$this->assertStringContainsString('<xmp>', $body);
55+
$this->assertStringContainsString('<\/xmp><script>alert(1)</script>', $body, 'payload displays escaped');
56+
$this->assertSame(1, substr_count($body, '</xmp>'), 'only the wrapper itself closes the block');
57+
}
58+
59+
/**
60+
* Serve one Support/bin script through php -S and return the response
61+
* body. The built-in server is the one place tests can reach xmpWrap()'s
62+
* web branch (PHP_SAPI is 'cli' everywhere else in the suite).
63+
*/
64+
private function requestViaBuiltInServer(string $script): string
65+
{
66+
$docRoot = dirname(__DIR__) . '/Support/bin';
67+
68+
// find a free port, then hand it to php -S (it can't pick its own)
69+
$socket = stream_socket_server('tcp://127.0.0.1:0');
70+
$this->assertNotFalse($socket, 'could not find a free port');
71+
$port = (int)substr(strrchr(stream_socket_get_name($socket, false), ':'), 1);
72+
fclose($socket);
73+
74+
$pipes = [];
75+
$server = proc_open([PHP_BINARY, '-S', "127.0.0.1:$port", '-t', $docRoot], [1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes);
76+
$this->assertIsResource($server, 'could not start php -S');
77+
78+
try {
79+
$context = stream_context_create(['http' => ['timeout' => 1]]);
80+
$body = false;
81+
for ($attempt = 0; $attempt < 50 && $body === false; $attempt++) {
82+
usleep(100_000);
83+
$body = @file_get_contents("http://127.0.0.1:$port/$script", false, $context);
84+
}
85+
$this->assertIsString($body, 'no response from php -S after 5 seconds');
86+
return $body;
87+
} finally {
88+
proc_terminate($server);
89+
proc_close($server);
90+
}
91+
}
4292
}

0 commit comments

Comments
 (0)