Skip to content

Commit 4084186

Browse files
committed
Formalize UrlType as http/https only
This was always the intent, but we accidentally let through things that are not web, such as `file://`
1 parent c44c7d2 commit 4084186

File tree

2 files changed

+94
-1
lines changed

2 files changed

+94
-1
lines changed

src/Api/Scalar/UrlType.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,19 @@
66

77
final class UrlType extends AbstractStringBasedType
88
{
9+
/**
10+
* @var string
11+
*/
12+
public $description = 'An absolute web URL that must start with `http` or `https`.';
13+
914
/**
1015
* Validate an URL
1116
*
1217
* @param mixed $value
1318
*/
1419
protected function isValid($value): bool
1520
{
16-
return is_string($value) && filter_var($value, FILTER_VALIDATE_URL);
21+
// Here we use a naive pattern that should ideally be kept in sync with Natural validator
22+
return is_string($value) && preg_match('~^https?://(?:[^.\s]+\.)+[^.\s]+$~', $value);
1723
}
1824
}

tests/Api/Scalar/UrlTypeTest.php

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace EcodevTests\Felix\Api\Scalar;
6+
7+
use Ecodev\Felix\Api\Scalar\UrlType;
8+
use GraphQL\Language\AST\StringValueNode;
9+
use PHPUnit\Framework\TestCase;
10+
11+
final class UrlTypeTest extends TestCase
12+
{
13+
/**
14+
* @dataProvider providerUrls
15+
*/
16+
public function testSerialize(?string $input, bool $isValid): void
17+
{
18+
$type = new UrlType();
19+
$actual = $type->serialize($input);
20+
self::assertSame($input, $actual);
21+
}
22+
23+
/**
24+
* @dataProvider providerUrls
25+
*/
26+
public function testParseValue(?string $input, bool $isValid): void
27+
{
28+
$type = new UrlType();
29+
30+
if (!$isValid) {
31+
$this->expectExceptionMessage('Query error: Not a valid Url');
32+
}
33+
34+
$actual = $type->parseValue($input);
35+
36+
self::assertSame($input, $actual);
37+
}
38+
39+
/**
40+
* @dataProvider providerUrls
41+
*/
42+
public function testParseLiteral(?string $input, bool $isValid): void
43+
{
44+
$type = new UrlType();
45+
$ast = new StringValueNode(['value' => $input]);
46+
47+
if (!$isValid) {
48+
$this->expectExceptionMessage('Query error: Not a valid Url');
49+
}
50+
51+
$actual = $type->parseLiteral($ast);
52+
53+
self::assertSame($input, $actual);
54+
}
55+
56+
public function providerUrls(): array
57+
{
58+
return [
59+
['http://www.example.com', true],
60+
['https://www.example.com', true],
61+
['http://example.com', true],
62+
['http://www.example.com/path', true],
63+
['http://www.example.com/path#frag', true],
64+
['http://www.example.com/path?param=1', true],
65+
['http://www.example.com/path?param=1#fra', true],
66+
['http://t.co', true],
67+
['http://www.t.co', true],
68+
['http://a-b.c.t.co', true],
69+
['http://aa.com', true],
70+
['http://www.example', true], // this is indeed valid because `example` could be a TLD
71+
['https://example.com:4200/subscribe', true],
72+
['https://example-.com', true], // this is not conform to rfc1738, but we tolerate it for simplicity sake
73+
74+
['www.example.com', false],
75+
['example.com', false],
76+
['www.example', false],
77+
['http://example', false],
78+
['www.example#.com', false],
79+
['www.t.co', false],
80+
['file:///C:/folder/file.pdf', false],
81+
82+
['', false],
83+
[null, false],
84+
[' ', false],
85+
];
86+
}
87+
}

0 commit comments

Comments
 (0)