Skip to content

Commit d5b6e19

Browse files
committed
Apply fixed Coderabbitai review.
1 parent 6258c63 commit d5b6e19

5 files changed

Lines changed: 223 additions & 46 deletions

File tree

‎src/Exception/Message.php‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PHPForge\Debug\Exception;
6+
7+
use function sprintf;
8+
9+
/**
10+
* Exception message templates authored by this package.
11+
*
12+
* Use {@see Message::getMessage()} to format a template with `sprintf()` arguments.
13+
*/
14+
enum Message: string
15+
{
16+
/**
17+
* Indicates that a column style is not a {@see \PHPForge\Debug\ColumnStyle} case.
18+
*
19+
* Format: "Debug panel column styles must be %s cases."
20+
*/
21+
case COLUMN_STYLE_INVALID = 'Debug panel column styles must be %s cases.';
22+
23+
/**
24+
* Indicates that a column style is keyed by a column the table does not declare.
25+
*
26+
* Format: "Debug panel column styles must be keyed by an existing column index."
27+
*/
28+
case COLUMN_STYLE_KEY_INVALID = 'Debug panel column styles must be keyed by an existing column index.';
29+
30+
/**
31+
* Indicates that an inline value is neither a scalar, `null`, nor a shape this class produces.
32+
*
33+
* Format: "Debug panel inline content must be a scalar, null, or a PanelView inline value. Got %s."
34+
*/
35+
case INLINE_CONTENT_INVALID = 'Debug panel inline content must be a scalar, null, or a PanelView inline value. '
36+
. 'Got %s.';
37+
38+
/**
39+
* Indicates that a link target carries characters a browser strips while resolving a URL.
40+
*
41+
* Format: "Debug panel links must not carry control characters or surrounding spaces, because a browser strips
42+
* them while resolving the target."
43+
*/
44+
case LINK_TARGET_NORMALIZED = 'Debug panel links must not carry control characters or surrounding spaces, because '
45+
. 'a browser strips them while resolving the target.';
46+
47+
/**
48+
* Indicates that a link target declares a scheme the host must not follow.
49+
*
50+
* Format: "Debug panel links must be relative or use the http, https, or mailto scheme. Got %s."
51+
*/
52+
case LINK_TARGET_SCHEME_INVALID = 'Debug panel links must be relative or use the http, https, or mailto scheme. '
53+
. 'Got %s.';
54+
55+
/**
56+
* Indicates that a link target cannot be parsed as a URL.
57+
*
58+
* Format: "Debug panel links must be a target a browser can resolve."
59+
*/
60+
case LINK_TARGET_UNPARSABLE = 'Debug panel links must be a target a browser can resolve.';
61+
62+
/**
63+
* Indicates that paragraph content is neither a scalar, an inline value, nor a list of them.
64+
*
65+
* Format: "Debug panel paragraphs must be scalars, inline values, or lists of them."
66+
*/
67+
case PARAGRAPH_CONTENT_INVALID = 'Debug panel paragraphs must be scalars, inline values, or lists of them.';
68+
69+
/**
70+
* Indicates that a column heading is not a plain string.
71+
*
72+
* Format: "Debug panel table headers must be plain strings."
73+
*/
74+
case TABLE_HEADER_INVALID = 'Debug panel table headers must be plain strings.';
75+
76+
/**
77+
* Indicates that a table row is not a list as wide as the headers.
78+
*
79+
* Format: "Debug panel table rows must be lists whose width matches the headers."
80+
*/
81+
case TABLE_ROW_WIDTH_INVALID = 'Debug panel table rows must be lists whose width matches the headers.';
82+
83+
/**
84+
* Indicates that a captured trace frame is not an array of frame fields.
85+
*
86+
* Format: "Debug panel trace frames must be arrays of frame fields."
87+
*/
88+
case TRACE_FRAME_INVALID = 'Debug panel trace frames must be arrays of frame fields.';
89+
90+
/**
91+
* Formats the message without changing diagnostic argument values.
92+
*
93+
* @param int|string ...$argument Values to insert into the message template.
94+
*
95+
* @return string Formatted exception message.
96+
*/
97+
public function getMessage(int|string ...$argument): string
98+
{
99+
return sprintf($this->value, ...$argument);
100+
}
101+
}

‎src/Panel.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ abstract public function present(array $data): PanelView;
4141
/**
4242
* Returns the icon identifier declared by the extension.
4343
*
44-
* @throws InvalidArgumentException If the icon identifier is empty.
44+
* @throws InvalidArgumentException if the icon identifier is empty.
4545
*
4646
* @return string Host-interpreted icon identifier.
4747
*/
@@ -53,7 +53,7 @@ final public function icon(): string
5353
/**
5454
* Returns the stable panel identifier declared by the extension.
5555
*
56-
* @throws InvalidArgumentException If the panel identifier is empty.
56+
* @throws InvalidArgumentException if the panel identifier is empty.
5757
*
5858
* @return string Identifier used to associate the panel with captured data.
5959
*/
@@ -65,7 +65,7 @@ final public function id(): string
6565
/**
6666
* Returns the navigation title declared by the extension.
6767
*
68-
* @throws InvalidArgumentException If the panel title is empty.
68+
* @throws InvalidArgumentException if the panel title is empty.
6969
*
7070
* @return string Human-readable panel title.
7171
*/
@@ -80,7 +80,7 @@ final public function name(): string
8080
* @param string $value Metadata value to validate.
8181
* @param string $field Constant name included in the validation error.
8282
*
83-
* @throws InvalidArgumentException If the metadata value is empty.
83+
* @throws InvalidArgumentException if the metadata value is empty.
8484
*
8585
* @return string Unmodified metadata value.
8686
*/

‎src/PanelView.php‎

Lines changed: 47 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,18 @@
66

77
use InvalidArgumentException;
88
use JsonSerializable;
9+
use PHPForge\Debug\Exception\Message;
910

1011
use function count;
1112
use function in_array;
1213
use function is_array;
1314
use function is_float;
1415
use function is_int;
1516
use function is_string;
16-
use function preg_match;
17+
use function parse_url;
18+
use function strpbrk;
1719
use function strtolower;
20+
use function trim;
1821

1922
/**
2023
* Builds immutable panel descriptions without exposing the host's markup or styles.
@@ -117,7 +120,7 @@ public function blocks(): array
117120
* @param Tone $tone Callout tone interpreted by the host frontend.
118121
* @param mixed ...$content Ordered factory-produced inline values, scalars, or `null`.
119122
*
120-
* @throws InvalidArgumentException If an argument is not an accepted inline value.
123+
* @throws InvalidArgumentException if an argument is not an accepted inline value.
121124
*
122125
* @return self New view with the callout appended.
123126
*/
@@ -177,7 +180,7 @@ public function disclosure(string $title, string $content): self
177180
* @param mixed ...$paragraphs Ordered explanations, each one paragraph: a scalar, a factory-produced inline value,
178181
* or a list of inline values and scalars.
179182
*
180-
* @throws InvalidArgumentException If a paragraph is neither an inline value nor a list of them.
183+
* @throws InvalidArgumentException if a paragraph is neither an inline value nor a list of them.
181184
*
182185
* @return self New view with the empty state appended.
183186
*/
@@ -253,7 +256,7 @@ public function jsonSerialize(): array
253256
* @param string $href Relative target, or an absolute `http`, `https`, or `mailto` URL.
254257
* @param bool $external Whether the host opens the target in a new browsing context.
255258
*
256-
* @throws InvalidArgumentException If the target declares a scheme the host must not follow.
259+
* @throws InvalidArgumentException if the target declares a scheme the host must not follow.
257260
*
258261
* @return LinkInline Inline link accepted by every content method.
259262
*/
@@ -275,7 +278,7 @@ public static function link(string $label, string $href, bool $external = false)
275278
* @param array<array-key, mixed> $values Labeled values in display order, each an inline value or a scalar.
276279
* @param bool $compact Whether to request compact presentation from the host.
277280
*
278-
* @throws InvalidArgumentException If a value is not an accepted inline value.
281+
* @throws InvalidArgumentException if a value is not an accepted inline value.
279282
*
280283
* @return self New view with the overview appended.
281284
*/
@@ -295,7 +298,7 @@ public function overview(array $values, bool $compact = false): self
295298
*
296299
* @param mixed ...$content Ordered factory-produced inline values, scalars, or `null`.
297300
*
298-
* @throws InvalidArgumentException If an argument is not an accepted inline value.
301+
* @throws InvalidArgumentException if an argument is not an accepted inline value.
299302
*
300303
* @return self New view with the paragraph appended.
301304
*/
@@ -395,7 +398,7 @@ public function summaryMetrics(): array
395398
* @param array<array-key, mixed> $styles Optional {@see ColumnStyle} cases keyed by column index.
396399
* @param bool $filterable Whether to request the host's in-place row filter for the table.
397400
*
398-
* @throws InvalidArgumentException If a header, row width, cell, or column style is invalid.
401+
* @throws InvalidArgumentException if a header, row width, cell, or column style is invalid.
399402
*
400403
* @return self New view with the table appended.
401404
*/
@@ -477,7 +480,7 @@ public function toolbarMetrics(): array
477480
*
478481
* @param array<array-key, mixed> $frames Captured frames in call order, each an array of frame fields.
479482
*
480-
* @throws InvalidArgumentException If a frame is not an array of fields.
483+
* @throws InvalidArgumentException if a frame is not an array of fields.
481484
*
482485
* @return TraceInline Inline trace accepted by every content method.
483486
*/
@@ -488,7 +491,7 @@ public static function trace(array $frames): array
488491
foreach ($frames as $frame) {
489492
if (is_array($frame) === false) {
490493
throw new InvalidArgumentException(
491-
'Debug panel trace frames must be arrays of frame fields.',
494+
Message::TRACE_FRAME_INVALID->getMessage(),
492495
);
493496
}
494497

@@ -546,7 +549,7 @@ private function append(array $block): self
546549
*
547550
* @param array<array-key, mixed> $headers Column headings to validate.
548551
*
549-
* @throws InvalidArgumentException If a heading is not a string.
552+
* @throws InvalidArgumentException if a heading is not a string.
550553
*
551554
* @return list<string> Validated column headings in display order.
552555
*/
@@ -557,7 +560,7 @@ private static function headers(array $headers): array
557560
foreach ($headers as $header) {
558561
if (is_string($header) === false) {
559562
throw new InvalidArgumentException(
560-
'Debug panel table headers must be plain strings.',
563+
Message::TABLE_HEADER_INVALID->getMessage(),
561564
);
562565
}
563566

@@ -572,7 +575,7 @@ private static function headers(array $headers): array
572575
*
573576
* @param mixed $value Inline value or plain value to normalize.
574577
*
575-
* @throws InvalidArgumentException If the value is not an accepted inline input.
578+
* @throws InvalidArgumentException if the value is not an accepted inline input.
576579
*
577580
* @return Inline Normalized inline value.
578581
*/
@@ -594,7 +597,7 @@ private static function inline(mixed $value): array
594597
*
595598
* @param array<array-key, mixed> $value Candidate inline value.
596599
*
597-
* @throws InvalidArgumentException If the array was not produced by an inline factory.
600+
* @throws InvalidArgumentException if the array was not produced by an inline factory.
598601
*
599602
* @return Inline Validated inline value.
600603
*/
@@ -660,7 +663,7 @@ private static function inlineShape(array $value): array
660663
* @param array<array-key, mixed> $content Inline values or plain values in display order.
661664
* @param Tone|null $tone Callout tone, or `null` for an ordinary paragraph.
662665
*
663-
* @throws InvalidArgumentException If an item is not an accepted inline value.
666+
* @throws InvalidArgumentException if an item is not an accepted inline value.
664667
*
665668
* @return ParagraphBlock Validated paragraph block.
666669
*/
@@ -684,7 +687,7 @@ private static function paragraphBlock(array $content, Tone|null $tone): array
684687
*
685688
* @param mixed $paragraph Paragraph description.
686689
*
687-
* @throws InvalidArgumentException If the description is neither an inline value nor a list of them.
690+
* @throws InvalidArgumentException if the description is neither an inline value nor a list of them.
688691
*
689692
* @return ParagraphBlock Validated paragraph block.
690693
*/
@@ -696,7 +699,7 @@ private static function paragraphOf(mixed $paragraph): array
696699

697700
if (array_is_list($paragraph) === false) {
698701
throw new InvalidArgumentException(
699-
'Debug panel paragraphs must be scalars, inline values, or lists of them.',
702+
Message::PARAGRAPH_CONTENT_INVALID->getMessage(),
700703
);
701704
}
702705

@@ -709,7 +712,7 @@ private static function paragraphOf(mixed $paragraph): array
709712
* @param array<array-key, mixed> $rows Rows to validate.
710713
* @param int $columns Number of declared columns.
711714
*
712-
* @throws InvalidArgumentException If a row is not a list or its width differs from the headers.
715+
* @throws InvalidArgumentException if a row is not a list or its width differs from the headers.
713716
*
714717
* @return list<list<Inline>> Validated rows in display order.
715718
*/
@@ -720,7 +723,7 @@ private static function rows(array $rows, int $columns): array
720723
foreach ($rows as $row) {
721724
if (is_array($row) === false || array_is_list($row) === false || count($row) !== $columns) {
722725
throw new InvalidArgumentException(
723-
'Debug panel table rows must be lists whose width matches the headers.',
726+
Message::TABLE_ROW_WIDTH_INVALID->getMessage(),
724727
);
725728
}
726729

@@ -742,7 +745,7 @@ private static function rows(array $rows, int $columns): array
742745
* @param array<array-key, mixed> $styles Column styles keyed by column index.
743746
* @param int $columns Number of declared columns.
744747
*
745-
* @throws InvalidArgumentException If a key is not an existing column index or a value is not a style.
748+
* @throws InvalidArgumentException if a key is not an existing column index or a value is not a style.
746749
*
747750
* @return array<int, ColumnStyle> Validated column styles.
748751
*/
@@ -753,13 +756,13 @@ private static function styles(array $styles, int $columns): array
753756
foreach ($styles as $column => $style) {
754757
if (is_int($column) === false || $column < 0 || $column >= $columns) {
755758
throw new InvalidArgumentException(
756-
'Debug panel column styles must be keyed by an existing column index.',
759+
Message::COLUMN_STYLE_KEY_INVALID->getMessage(),
757760
);
758761
}
759762

760763
if ($style instanceof ColumnStyle === false) {
761764
throw new InvalidArgumentException(
762-
'Debug panel column styles must be ' . ColumnStyle::class . ' cases.',
765+
Message::COLUMN_STYLE_INVALID->getMessage(ColumnStyle::class),
763766
);
764767
}
765768

@@ -772,20 +775,37 @@ private static function styles(array $styles, int $columns): array
772775
/**
773776
* Rejects a link target the host must not follow.
774777
*
778+
* Characters a browser strips while resolving a URL are rejected first, because they move the scheme: a tab, a line
779+
* break, or a surrounding space turns `" javascript:alert(1)"` into an executable target after the check.
780+
*
775781
* @param string $href Candidate target.
776782
*
777-
* @throws InvalidArgumentException If the target declares a scheme other than `http`, `https`, or `mailto`.
783+
* @throws InvalidArgumentException if the target carries characters a browser strips, cannot be parsed, or declares
784+
* a scheme other than `http`, `https`, or `mailto`.
778785
*
779786
* @return string Unmodified target.
780787
*/
781788
private static function target(string $href): string
782789
{
783-
$scheme = [];
790+
if (strpbrk($href, "\t\n\r") !== false || trim($href, "\x00..\x20") !== $href) {
791+
throw new InvalidArgumentException(
792+
Message::LINK_TARGET_NORMALIZED->getMessage(),
793+
);
794+
}
795+
796+
$parts = parse_url($href);
797+
798+
if ($parts === false) {
799+
throw new InvalidArgumentException(
800+
Message::LINK_TARGET_UNPARSABLE->getMessage(),
801+
);
802+
}
803+
804+
$scheme = $parts['scheme'] ?? null;
784805

785-
if (preg_match('/^([A-Za-z][A-Za-z0-9+.\-]*):/', $href, $scheme) === 1
786-
&& in_array(strtolower($scheme[1]), ['http', 'https', 'mailto'], true) === false) {
806+
if ($scheme !== null && in_array(strtolower($scheme), ['http', 'https', 'mailto'], true) === false) {
787807
throw new InvalidArgumentException(
788-
"Debug panel links must be relative or use the http, https, or mailto scheme. Got {$scheme[1]}.",
808+
Message::LINK_TARGET_SCHEME_INVALID->getMessage($scheme),
789809
);
790810
}
791811

@@ -802,8 +822,7 @@ private static function target(string $href): string
802822
private static function unsupportedInline(mixed $value): InvalidArgumentException
803823
{
804824
return new InvalidArgumentException(
805-
'Debug panel inline content must be a scalar, null, or a PanelView inline value. Got '
806-
. get_debug_type($value) . '.',
825+
Message::INLINE_CONTENT_INVALID->getMessage(get_debug_type($value)),
807826
);
808827
}
809828
}

0 commit comments

Comments
 (0)