Skip to content

Commit 6bb1aa8

Browse files
MeldironMeldiron
committed
Improve code quality
1 parent 6ef67e1 commit 6bb1aa8

File tree

4 files changed

+53
-25
lines changed

4 files changed

+53
-25
lines changed

‎src/DNS/Client.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Utopia\DNS;
44

55
use Exception;
6+
use Utopia\Validator\IP;
67

78
class Client
89
{
@@ -14,6 +15,11 @@ class Client
1415

1516
public function __construct(string $server = '127.0.0.1', int $port = 53, int $timeout = 5)
1617
{
18+
$validator = new IP(IP::ALL); // IPv4 + IPv6
19+
if (!$validator->isValid($server)) {
20+
throw new Exception('Server must be an IP address.');
21+
}
22+
1723
$this->server = $server;
1824
$this->port = $port;
1925
$this->timeout = $timeout;

‎src/DNS/Validator/DNS.php‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,20 +41,6 @@ class DNS extends Validator
4141
*/
4242
public function __construct(protected string $target, protected string $type = self::RECORD_CNAME, protected string $dnsServer = self::DEFAULT_DNS_SERVER)
4343
{
44-
if (!filter_var($dnsServer, FILTER_VALIDATE_IP)) {
45-
$dns = new Client(self::DEFAULT_DNS_SERVER);
46-
$question = new Question($dnsServer, Record::TYPE_A); // TODO: ipv6 support
47-
$query = Message::query($question, recursionDesired: true);
48-
$response = $dns->query($query);
49-
$answer = $response->answers[0] ?? null;
50-
$aRecord = $answer->rdata ?? '';
51-
52-
if (empty($aRecord)) {
53-
throw new \Exception('Invalid DNS server.');
54-
}
55-
56-
$this->dnsServer = $aRecord;
57-
}
5844
}
5945

6046
/**

‎tests/e2e/DNS/ClientTest.php‎

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ final class ClientTest extends TestCase
1414

1515
public function testARecords(): void
1616
{
17-
$client = new Client('localhost', self::PORT);
17+
$client = new Client('127.0.0.1', self::PORT);
1818
$response = $client->query(Message::query(
1919
new Question('dev.appwrite.io', Record::TYPE_A)
2020
));
@@ -49,7 +49,7 @@ public function testARecords(): void
4949

5050
public function testAAAARecords(): void
5151
{
52-
$client = new Client('localhost', self::PORT);
52+
$client = new Client('127.0.0.1', self::PORT);
5353
$response = $client->query(Message::query(
5454
new Question('dev.appwrite.io', Record::TYPE_AAAA)
5555
));
@@ -79,7 +79,7 @@ public function testAAAARecords(): void
7979

8080
public function testCnameRecords(): void
8181
{
82-
$client = new Client('localhost', self::PORT);
82+
$client = new Client('127.0.0.1', self::PORT);
8383
$response = $client->query(Message::query(
8484
new Question('alias.appwrite.io', Record::TYPE_CNAME)
8585
));
@@ -102,7 +102,7 @@ public function testCnameRecords(): void
102102

103103
public function testTxtRecords(): void
104104
{
105-
$client = new Client('localhost', self::PORT);
105+
$client = new Client('127.0.0.1', self::PORT);
106106
$response = $client->query(Message::query(
107107
new Question('dev.appwrite.io', Record::TYPE_TXT)
108108
));
@@ -128,7 +128,7 @@ public function testTxtRecords(): void
128128

129129
public function testNsRecords(): void
130130
{
131-
$client = new Client('localhost', self::PORT);
131+
$client = new Client('127.0.0.1', self::PORT);
132132
$response = $client->query(Message::query(
133133
new Question('delegated.appwrite.io', Record::TYPE_NS)
134134
));
@@ -161,7 +161,7 @@ public function testNsRecords(): void
161161

162162
public function testCaaRecords(): void
163163
{
164-
$client = new Client('localhost', self::PORT);
164+
$client = new Client('127.0.0.1', self::PORT);
165165
$response = $client->query(Message::query(
166166
new Question('dev.appwrite.io', Record::TYPE_CAA)
167167
));
@@ -187,7 +187,7 @@ public function testCaaRecords(): void
187187

188188
public function testSoaRecords(): void
189189
{
190-
$client = new Client('localhost', self::PORT);
190+
$client = new Client('127.0.0.1', self::PORT);
191191
$response = $client->query(Message::query(
192192
new Question('appwrite.io', Record::TYPE_SOA)
193193
));
@@ -220,4 +220,39 @@ public function testSoaRecords(): void
220220
$this->assertStringContainsString('team.appwrite.io', $rdata);
221221
$this->assertStringContainsString('1 7200 1800 1209600 3600', $rdata);
222222
}
223+
224+
public function testInvalidServer(): void
225+
{
226+
try {
227+
new Client('not-ip-address', self::PORT);
228+
$this->fail('Expected invalid IP address exception');
229+
} catch (\Exception $e) {
230+
$this->assertEquals('Server must be an IP address.', $e->getMessage());
231+
}
232+
233+
try {
234+
new Client('ns1.digitalocean.com', self::PORT);
235+
$this->fail('Expected invalid IP address exception');
236+
} catch (\Exception $e) {
237+
$this->assertEquals('Server must be an IP address.', $e->getMessage());
238+
}
239+
240+
try {
241+
$client = new Client('172.64.52.210', self::PORT);
242+
$this->assertNotEmpty($client);
243+
$client = new Client('127.0.0.1', self::PORT);
244+
$this->assertNotEmpty($client);
245+
} catch (\Exception $e) {
246+
$this->fail('IPv4 threw unexpected error');
247+
}
248+
249+
try {
250+
$client = new Client('::1', self::PORT);
251+
$this->assertNotEmpty($client);
252+
$client = new Client('2606:4700:52::ac40:34d2', self::PORT);
253+
$this->assertNotEmpty($client);
254+
} catch (\Exception $e) {
255+
$this->fail('IPv6 threw unexpected error');
256+
}
257+
}
223258
}

‎tests/unit/DNS/Validator/DNSTest.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,9 @@ public function testAAAA(): void
7272
*/
7373
public function testCAA(): void
7474
{
75-
$certainly = new DNS('certainly.com', DNS::RECORD_CAA, 'ns1.digitalocean.com');
76-
$letsencrypt = new DNS('letsencrypt.org', DNS::RECORD_CAA, 'ns1.digitalocean.com');
75+
$digitalOceanIp = '172.64.52.210'; // ping ns1.digitalocean.com
76+
$certainly = new DNS('certainly.com', DNS::RECORD_CAA, $digitalOceanIp);
77+
$letsencrypt = new DNS('letsencrypt.org', DNS::RECORD_CAA, $digitalOceanIp);
7778

7879
// No CAA record succeeds on main domain & subdomains for any issuer
7980
$this->assertEquals($certainly->isValid('caa.appwrite.org'), true);
@@ -88,11 +89,11 @@ public function testCAA(): void
8889
$this->assertEquals($letsencrypt->isValid('certainly-full.caa.appwrite.org'), false);
8990

9091
// Custom flags&tag are not allowed if validator includes specific flags&tag
91-
$certainlyFull = new DNS('0 issue "certainly.com"', DNS::RECORD_CAA, 'ns1.digitalocean.com');
92+
$certainlyFull = new DNS('0 issue "certainly.com"', DNS::RECORD_CAA, $digitalOceanIp);
9293
$this->assertEquals($certainlyFull->isValid('certainly-full.caa.appwrite.org'), false);
9394

9495
// Custom flags&tag still allows if they match exactly
95-
$certainlyFull = new DNS('128 issuewild "certainly.com;account=123456;validationmethods=dns-01"', DNS::RECORD_CAA, 'ns1.digitalocean.com');
96+
$certainlyFull = new DNS('128 issuewild "certainly.com;account=123456;validationmethods=dns-01"', DNS::RECORD_CAA, $digitalOceanIp);
9697
$this->assertEquals($certainlyFull->isValid('certainly-full.caa.appwrite.org'), true);
9798

9899
// Certainly CAA allows Certainly, but not LetsEncrypt; Same for subdomains

0 commit comments

Comments
 (0)