Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 38 additions & 7 deletions src/PurplePixie/PhpDns/DNSQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ class DNSQuery

private bool $connectionException = false;

private bool $responseException = false;

public function __construct(string $server, int $port = 53, int $timeout = 60, bool $udp = true, bool $debug = false, bool $binarydebug = false)
{
$this->server = $server;
Expand Down Expand Up @@ -188,7 +190,7 @@ private function clearError(): void

/**
* @return array
* @throws Exceptions\InvalidQueryTypeId
* @throws Exceptions\InvalidQueryTypeId|Exceptions\InvalidResponse
*/
private function readRecord(): array
{
Expand All @@ -198,13 +200,32 @@ private function readRecord(): array

$ans_header_bin = $this->readResponse(10); // 10 byte header

$ans_header = unpack('ntype/nclass/Nttl/nlength', $ans_header_bin);
$ans_header = @unpack('ntype/nclass/Nttl/nlength', $ans_header_bin);
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the @ operator to suppress errors is generally discouraged as it hides potential issues and makes debugging harder. Consider removing the @ operator and handling the false return value explicitly, which you're already doing in the subsequent if statement.

Suggested change
$ans_header = @unpack('ntype/nclass/Nttl/nlength', $ans_header_bin);
$ans_header = unpack('ntype/nclass/Nttl/nlength', $ans_header_bin);

Copilot uses AI. Check for mistakes.

if (is_array($ans_header)) $this->debug(
'Record Type ' . $ans_header['type'] . ' Class ' . $ans_header['class'] .
' TTL ' . $ans_header['ttl'] . ' Length ' . $ans_header['length']
);
else $this->debug("Error unpacking answer header, no array returned.");
if ($ans_header === null || $ans_header === false || !is_array($ans_header)) // the unpack has failed - we assume an invalid return
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition $ans_header === null is redundant. The unpack() function in PHP only returns false on failure, never null. The check should be simplified to if ($ans_header === false).

Suggested change
if ($ans_header === null || $ans_header === false || !is_array($ans_header)) // the unpack has failed - we assume an invalid return
if ($ans_header === false) // the unpack has failed - we assume an invalid return

Copilot uses AI. Check for mistakes.
{
$this->debug("Error unpacking answer header, no array returned.");

if ($this->responseException)
throw new Exceptions\InvalidResponse("Answer header invalid format or empty");

return [
'header' => [],
'typeid' => null,
'typename' => "",
'data' => "",
'domain' => "",
'string' => "Error unpacking answer header",
'extras' => "",
];
}
else
{
$this->debug(
'Record Type ' . $ans_header['type'] . ' Class ' . $ans_header['class'] .
' TTL ' . $ans_header['ttl'] . ' Length ' . $ans_header['length']
);
}
Comment on lines +222 to +228
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else block is unnecessary since the if block returns an array. The code in the else block (lines 224-227) should be moved after the if block and the else removed to reduce nesting and improve readability.

Copilot uses AI. Check for mistakes.

$typeId = $ans_header['type'];

Expand Down Expand Up @@ -767,4 +788,14 @@ public function setConnectionException(bool $value): void
{
$this->connectionException = $value;
}

public function setResponseException(bool $value): void
{
$this->responseException = $value;
}

public function getResponseException(): bool
{
return $this->responseException;
}
}
29 changes: 29 additions & 0 deletions src/PurplePixie/PhpDns/Exceptions/InvalidResponse.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

/**
* Copyright (C) 2025, David Cutting, Purplepixie Systems
*
* This is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* The software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this software. If not, see www.gnu.org/licenses
*
* For more information see www.purplepixie.org/phpdns
*/

namespace PurplePixie\PhpDns\Exceptions;

class InvalidResponse extends \Exception {

public function __construct(string $message) {
parent::__construct('Invalid response: ' . $message);
}
}