Skip to content

Commit 0b13d01

Browse files
spazespaze
committed
Register callbacks just once otherwise back-to-back check() calls accumulate handlers and handler calls
1 parent 8d1d8b5 commit 0b13d01

3 files changed

Lines changed: 75 additions & 52 deletions

File tree

‎src/Check/SecurityTxtCheckHost.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ public function __construct(
7575
private readonly SecurityTxtFetcher $fetcher,
7676
private readonly SecurityTxtCheckHostResultFactory $resultFactory,
7777
) {
78+
$this->initFetcherCallbacks();
7879
}
7980

8081

@@ -99,8 +100,6 @@ public function __construct(
99100
*/
100101
public function check(Url $url, ?int $expiresWarningThreshold = null, bool $strictMode = false, bool $requireTopLevelLocation = false, bool $noIpv6 = false, ?int $maxAllowedRedirects = null): SecurityTxtCheckHostResult
101102
{
102-
$this->initFetcherCallbacks();
103-
104103
$host = $url->getUnicodeHost();
105104
if ($host === null) {
106105
throw new SecurityTxtCannotParseHostnameException($url->toUnicodeString());

‎src/Check/SecurityTxtCheckHostCli.php‎

Lines changed: 56 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,21 @@
88
use Spaze\SecurityTxt\Fetcher\Exceptions\SecurityTxtFetcherException;
99
use Uri\WhatWg\Url;
1010

11-
final readonly class SecurityTxtCheckHostCli
11+
final class SecurityTxtCheckHostCli
1212
{
1313

14+
private bool $verbose = false;
15+
16+
1417
/**
1518
* @param Closure(int): void $exit
1619
*/
1720
public function __construct(
18-
private ConsolePrinter $consolePrinter,
19-
private SecurityTxtCheckHost $checkHost,
20-
private Closure $exit,
21+
private readonly ConsolePrinter $consolePrinter,
22+
private readonly SecurityTxtCheckHost $checkHost,
23+
private readonly Closure $exit,
2124
) {
25+
$this->initCheckHostCallbacks();
2226
}
2327

2428

@@ -33,6 +37,7 @@ public function check(
3337
bool $showUsageHelp,
3438
string $usageHelp,
3539
): void {
40+
$this->verbose = $verbose;
3641
if ($colors) {
3742
$this->consolePrinter->enableColors();
3843
}
@@ -45,24 +50,57 @@ public function check(
4550
$this->exit(CheckExitStatus::NoFile);
4651
return;
4752
}
53+
try {
54+
$checkResult = $this->checkHost->check(
55+
$url,
56+
$expiresWarningThreshold,
57+
$strictMode,
58+
$requireTopLevelLocation,
59+
$noIpv6,
60+
);
61+
if (!$checkResult->isValid()) {
62+
$this->consolePrinter->error($this->consolePrinter->colorRed('The file is invalid'));
63+
$this->exit(CheckExitStatus::Error);
64+
} else {
65+
$this->consolePrinter->ok($this->consolePrinter->colorGreen('The file is valid'));
66+
$this->exit(CheckExitStatus::Ok);
67+
}
68+
} catch (SecurityTxtFetcherException $e) {
69+
$this->consolePrinter->error($e->getMessage());
70+
$this->exit(CheckExitStatus::FileError);
71+
}
72+
}
4873

49-
if ($verbose) {
50-
$this->checkHost->addOnUrl(
51-
function (string $url): void {
74+
75+
private function exit(CheckExitStatus $exitStatus): void
76+
{
77+
($this->exit)($exitStatus->value);
78+
}
79+
80+
81+
private function initCheckHostCallbacks(): void
82+
{
83+
$this->checkHost->addOnUrl(
84+
function (string $url): void {
85+
if ($this->verbose) {
5286
$this->consolePrinter->info('Loading security.txt from ' . $this->consolePrinter->colorBold($url));
53-
},
54-
);
55-
$this->checkHost->addOnRedirect(
56-
function (string $url, string $destination): void {
87+
}
88+
},
89+
);
90+
$this->checkHost->addOnRedirect(
91+
function (string $url, string $destination): void {
92+
if ($this->verbose) {
5793
$this->consolePrinter->info('Redirected from ' . $this->consolePrinter->colorBold($url) . ' to ' . $this->consolePrinter->colorBold($destination));
58-
},
59-
);
60-
$this->checkHost->addOnUrlNotFound(
61-
function (string $url): void {
94+
}
95+
},
96+
);
97+
$this->checkHost->addOnUrlNotFound(
98+
function (string $url): void {
99+
if ($this->verbose) {
62100
$this->consolePrinter->info('Not found ' . $this->consolePrinter->colorBold($url));
63-
},
64-
);
65-
}
101+
}
102+
},
103+
);
66104
$this->checkHost->addOnFinalUrl(
67105
function (string $url): void {
68106
$this->consolePrinter->info('Using ' . $this->consolePrinter->colorBold($url));
@@ -118,32 +156,6 @@ function (string $keyFingerprint, DateTimeImmutable $signatureDate): void {
118156
$this->checkHost->addOnFetchWarning($onWarning);
119157
$this->checkHost->addOnLineWarning($onWarning);
120158
$this->checkHost->addOnFileWarning($onWarning);
121-
122-
try {
123-
$checkResult = $this->checkHost->check(
124-
$url,
125-
$expiresWarningThreshold,
126-
$strictMode,
127-
$requireTopLevelLocation,
128-
$noIpv6,
129-
);
130-
if (!$checkResult->isValid()) {
131-
$this->consolePrinter->error($this->consolePrinter->colorRed('The file is invalid'));
132-
$this->exit(CheckExitStatus::Error);
133-
} else {
134-
$this->consolePrinter->ok($this->consolePrinter->colorGreen('The file is valid'));
135-
$this->exit(CheckExitStatus::Ok);
136-
}
137-
} catch (SecurityTxtFetcherException $e) {
138-
$this->consolePrinter->error($e->getMessage());
139-
$this->exit(CheckExitStatus::FileError);
140-
}
141-
}
142-
143-
144-
private function exit(CheckExitStatus $exitStatus): void
145-
{
146-
($this->exit)($exitStatus->value);
147159
}
148160

149161
}

‎tests/Check/SecurityTxtCheckHostCliTest.phpt‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,26 +74,38 @@ final class SecurityTxtCheckHostCliTest extends TestCase
7474
}
7575

7676

77-
public function testCheckErrorWarningNonVerbose(): void
77+
public function testCheckErrorWarningVerboseThenNonVerbose(): void
7878
{
79-
$contents = "Contact: mailto:foo@example.com\nExpires: {$this->expires}\n";
8079
$httpClient = $this->getHttpClient(
81-
new SecurityTxtFetcherResponse(200, ['content-type' => 'text/plain; charset=utf-8'], $contents, false, '1.1.1.0', SecurityTxtIpAddressType::V4),
82-
new SecurityTxtFetcherResponse(302, ['location' => 'https://nah.example/'], 'yes but', false, '1.1.1.0', SecurityTxtIpAddressType::V4),
80+
new SecurityTxtFetcherResponse(200, ['content-type' => 'text/plain; charset=utf-8'], "Contact: mailto:foo@example.com\nExpires: {$this->expires}\n", false, '1.1.1.0', SecurityTxtIpAddressType::V4),
81+
new SecurityTxtFetcherResponse(302, ['location' => 'https://nah1.example/'], 'yes but', false, '1.1.1.0', SecurityTxtIpAddressType::V4),
82+
new SecurityTxtFetcherResponse(404, [], 'yeah nah', false, '1.1.1.0', SecurityTxtIpAddressType::V4),
83+
new SecurityTxtFetcherResponse(200, ['content-type' => 'text/plain; charset=utf-8'], "Contact: mailto:foo@two.example\n", false, '1.1.1.0', SecurityTxtIpAddressType::V4),
84+
new SecurityTxtFetcherResponse(302, ['location' => 'https://nah2.example/'], 'yes but', false, '1.1.1.0', SecurityTxtIpAddressType::V4),
8385
new SecurityTxtFetcherResponse(404, [], 'yeah nah', false, '1.1.1.0', SecurityTxtIpAddressType::V4),
8486
);
8587
$checkHostCli = $this->getCheckHostCli($httpClient);
8688

8789
ob_start();
90+
$checkHostCli->check(new Url('https://verbose.example'), null, true, true, true, true, true, false, 'Help I need some<body>');
8891
$checkHostCli->check(new Url('https://non-verbose.example'), null, true, false, true, true, true, false, 'Help I need some<body>');
8992
$output = ob_get_clean();
9093
$expected = <<< EOT
91-
[Info] Parsing security.txt for non-verbose.example
92-
[Info] Using https://non-verbose.example/.well-known/security.txt
94+
[Info] Parsing security.txt for verbose.example
95+
[Info] Loading security.txt from https://verbose.example/.well-known/security.txt
96+
[Info] Loading security.txt from https://verbose.example/security.txt
97+
[Info] Redirected from https://verbose.example/security.txt to https://nah1.example/
98+
[Info] Not found https://nah1.example/
99+
[Info] Using https://verbose.example/.well-known/security.txt
93100
[Error] on line 2: The file is considered stale and should not be used (How to fix: The Expires field should contain a date and time in the future formatted according to the Internet profile of ISO 8601 as defined in RFC 3339, e.g. {$this->getExpiresExample()})
94101
[Warning] security.txt not found at the top-level path (How to fix: Redirect the top-level file to the one under the /.well-known/ path)
95102
[Error] The file has expired 42 days ago ({$this->expires})
96103
[Error] The file is invalid
104+
[Info] Parsing security.txt for non-verbose.example
105+
[Info] Using https://non-verbose.example/.well-known/security.txt
106+
[Error] The Expires field must always be present (How to fix: Add an Expires field with a date and time in the future formatted according to the Internet profile of ISO 8601 as defined in RFC 3339, e.g. 2027-04-14T23:59:59+00:00)
107+
[Warning] security.txt not found at the top-level path (How to fix: Redirect the top-level file to the one under the /.well-known/ path)
108+
[Error] The file is invalid
97109
EOT;
98110
Assert::same($expected . "\n", $output);
99111
Assert::same(CheckExitStatus::Error->value, $this->exitStatus);

0 commit comments

Comments
 (0)