Skip to content

Commit 0bdd141

Browse files
Fix ping url validation (alexjustesen#1754)
* Fix ping url validation * Fix pint issues * remove early return --------- Co-authored-by: Alex Justesen <[email protected]>
1 parent 45ef51b commit 0bdd141

File tree

3 files changed

+87
-20
lines changed

3 files changed

+87
-20
lines changed

app/Jobs/Speedtests/ExecuteOoklaSpeedtest.php

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,20 +112,19 @@ public function handle(): void
112112

113113
/**
114114
* Check for internet connection.
115+
*
116+
* @throws \Exception
115117
*/
116118
protected function checkForInternetConnection(): bool
117119
{
118120
$url = config('speedtest.ping_url');
119121

120-
// TODO: skip checking for internet connection, current validation does not take into account different host formats and ip addresses.
121-
return true;
122-
123122
// Skip checking for internet connection if ping url isn't set (disabled)
124123
if (blank($url)) {
125124
return true;
126125
}
127126

128-
if (! URL::isValidUrl($url)) {
127+
if (! $this->isValidPingUrl($url)) {
129128
$this->result->update([
130129
'data' => [
131130
'type' => 'log',
@@ -165,4 +164,20 @@ protected function checkForInternetConnection(): bool
165164

166165
return true;
167166
}
167+
168+
/**
169+
* Check if the given URL is a valid ping URL.
170+
*/
171+
public function isValidPingUrl(string $url): bool
172+
{
173+
$hasTLD = static function (string $url): bool {
174+
// this also ensures the string ends with a TLD
175+
return preg_match('/\.[a-z]{2,}$/i', $url);
176+
};
177+
178+
return (filter_var($url, FILTER_VALIDATE_URL) && $hasTLD($url))
179+
// to check for things like `google.com`, we need to add the protocol
180+
|| (filter_var('https://'.$url, FILTER_VALIDATE_URL) && $hasTLD($url))
181+
|| filter_var($url, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4 || FILTER_FLAG_IPV6) !== false;
182+
}
168183
}

tests/Unit/ExampleTest.php

Lines changed: 0 additions & 16 deletions
This file was deleted.
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php
2+
3+
namespace Tests\Unit;
4+
5+
use App\Jobs\Speedtests\ExecuteOoklaSpeedtest;
6+
use App\Models\Result;
7+
use Tests\TestCase;
8+
9+
final class ExecuteOoklaSpeedtestTest extends TestCase
10+
{
11+
/**
12+
* A basic test example.
13+
*/
14+
public function test_that_ping_urls_handled(): void
15+
{
16+
$job = new ExecuteOoklaSpeedtest(new Result);
17+
config(['speedtest' => ['ping_url' => 'google.com']]);
18+
19+
$passingCases = [
20+
// ipv6
21+
'2a00:1450:4016:80c::200e',
22+
// ipv4
23+
'1.1.1.1',
24+
// hostname
25+
'google.com',
26+
// hostname with subdomain
27+
'www.google.com',
28+
// hostname with protocol
29+
'http://google.com',
30+
'https://google.com',
31+
];
32+
33+
foreach ($passingCases as $case) {
34+
$this->assertTrue($job->isValidPingUrl($case), "$case is an invalid ping url");
35+
}
36+
37+
$failingCases = [
38+
// invalid hostname
39+
'google',
40+
// no tld
41+
'http://google',
42+
'https://google',
43+
// invalid ipv4
44+
'1.1.1',
45+
'1.1.1.1.',
46+
'1.1.1.1.1',
47+
'.1.1.1.1',
48+
// invalid ipv6
49+
'2a00:1450:4016:80c::200e::',
50+
'2a00:14504:4016:80c::200e',
51+
'2a00:1450:401680c::200e',
52+
'2v00:1450:4016:80c::200e',
53+
'2::1:1450:4016:80c::200e',
54+
// path included
55+
'https://google.com/test',
56+
// path and query included
57+
'https://google.com/test?query=1',
58+
// path, query and fragment included
59+
'https://www.google.com/test?query=1#fragment',
60+
// path, query, fragment and port included
61+
'https://google.com:8080/test?query=1#fragment',
62+
];
63+
64+
foreach ($failingCases as $case) {
65+
$this->assertFalse($job->isValidPingUrl($case), "$case is a valid ping url");
66+
}
67+
}
68+
}

0 commit comments

Comments
 (0)