-
Notifications
You must be signed in to change notification settings - Fork 4
AHN-2781: fix rdap structure #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
670378c
f9e2f77
7a2d07e
8c4312d
95d41a1
c24a1f4
17a9b65
62975df
11bd7d9
cc3f9ca
3cb0727
cb1a4e7
8066685
1892d83
e0b0fcd
2959ffd
55405f1
fb79e55
4d6c5d8
e39e981
611d80c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,8 @@ | |
|
|
||
| final class Domain extends Common | ||
| { | ||
| use TopMostEntityTrait; | ||
|
|
||
| /** | ||
| * @var DomainName | ||
| */ | ||
|
|
@@ -58,11 +60,16 @@ final class Domain extends Common | |
| */ | ||
| private $network; | ||
|
|
||
| /** @var array */ | ||
| private $redacted; | ||
|
|
||
| public function __construct(DomainName $ldhName) | ||
| { | ||
| parent::__construct(ObjectClassName::DOMAIN()); | ||
|
|
||
| $this->ldhName = $ldhName->toLDH(); | ||
|
|
||
| $this->secureDNS = new SecureDNS(); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -123,7 +130,11 @@ public function getSecureDNS(): ?SecureDNS | |
| */ | ||
| public function getEntities(): ?array | ||
| { | ||
| return $this->entities; | ||
| if (empty($this->entities['entities'])) { | ||
| return $this->entities ?? []; | ||
| } | ||
|
|
||
| return $this->entities['entities']; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -207,6 +218,7 @@ public function addEntity(Entity $entity): Domain | |
| if ($this->entities === null) { | ||
| $this->entities = []; | ||
| } | ||
|
|
||
| $this->entities[] = $entity; | ||
|
|
||
| return $this; | ||
|
|
@@ -222,4 +234,34 @@ public function setNetwork(IPNetwork $network): Domain | |
|
|
||
| return $this; | ||
| } | ||
|
|
||
| public function getRedacted(): array | ||
| { | ||
| if (empty($this->redacted)) { | ||
| $this->setDefaultRedactedRules(); | ||
| } | ||
|
|
||
| return $this->redacted; | ||
| } | ||
|
|
||
| private function setDefaultRedactedRules(): void | ||
| { | ||
| $this->redacted = []; | ||
| foreach (($this->getEntities() ?? []) as $k => $v) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove unused variable The loop variable Apply this diff: - foreach (($this->getEntities() ?? []) as $k => $v) {
+ foreach (array_keys($this->getEntities() ?? []) as $k) {Based on static analysis. 🧰 Tools🪛 PHPMD (2.15.0)250-250: Avoid unused local variables such as '$v'. (undefined) (UnusedLocalVariable) 🤖 Prompt for AI Agents |
||
| if ($v->getHandle() !== null) { | ||
| continue; | ||
| } | ||
| $this->redacted[] = [ | ||
| 'name' => [ | ||
| 'type' => 'Registry registrant ID', | ||
| ], | ||
| 'prePath' => "$.entities[{$k}].handle", | ||
| 'pathLang' => "jsonpath", | ||
| 'method' => 'removal', | ||
| 'reason' => [ | ||
| 'description' => 'Server policy', | ||
| ] | ||
| ]; | ||
| } | ||
| } | ||
|
Comment on lines
+247
to
+266
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify entity ordering assumption. The redaction rules assume entities are ordered as registrant (index 0), admin (1), tech (2), billing (3). The Run the following script to verify how entities are added and if ordering is enforced: 🏁 Script executed: #!/bin/bash
# Description: Check if entity ordering is enforced or documented
# Search for code that adds entities to Domain objects
rg -nP --type=php -C5 '->addEntity\(' | grep -A5 -B5 'Domain'
# Look for role-based entity ordering logic or documentation
rg -nP --type=php -C3 'registrant|admin|tech|billing' | grep -i 'entit'
# Check for any role constants or enums
ast-grep --pattern 'class Role {
$$$
}'Length of output: 2016 Now let me search for more details on the specific codebase structure: Use role-based JSONPath filters instead of hardcoded positional indices for entity redaction rules. The RFC 9537 standard explicitly warns against removing array elements by position where that position determines semantic meaning, and recommends using role-based JSONPath filters like The current code assumes entities are ordered as registrant (index 0), admin (1), tech (2), billing (3), but entity ordering in RDAP responses varies across servers and is not guaranteed. This will cause redaction rules to target incorrect entities when the server returns entities in a different order. Replace the hardcoded indices with role-based filters to ensure the rules target the correct entities regardless of their position in the array. 🤖 Prompt for AI Agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -91,19 +91,20 @@ public function setHandle(?string $handle): void | |||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * @return VCard[]|null | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| public function getVcardArray(): ?array | ||||||||||||||||||||||||||||||||||||
| public function getVcardArray() | ||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||
| return $this->vcardArray; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * @param VCard $vcard | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| public function addVcard(VCard $vcard): void | ||||||||||||||||||||||||||||||||||||
| public function addVcard( $vcard): void | ||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||
| if (empty($this->vcardArray)) { | ||||||||||||||||||||||||||||||||||||
| $this->vcardArray = []; | ||||||||||||||||||||||||||||||||||||
| $this->vcardArray = ['vcard']; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| $this->vcardArray[] = $vcard; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+102
to
109
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Removed type hints and confusing data structure initialization. Several critical issues in this method:
Apply this diff to restore type safety and clarify the data structure: /**
* @param VCard $vcard
*/
- public function addVcard( $vcard): void
+ public function addVcard(VCard $vcard): void
{
if (empty($this->vcardArray)) {
- $this->vcardArray = ['vcard'];
+ $this->vcardArray = [];
}
-
$this->vcardArray[] = $vcard;
}If the 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| <?php | ||
| declare(strict_types=1); | ||
|
|
||
| namespace hiqdev\rdap\core\Domain\Entity; | ||
|
|
||
| /** | ||
| * Trait TopMostEntityTrait | ||
| * | ||
| * @author Dmytro Naumenko <d.naumenko.a@gmail.com> | ||
| */ | ||
| trait TopMostEntityTrait | ||
| { | ||
| /** | ||
| * @var string[] The data structure named "rdapConformance" is an array of strings, | ||
| * each providing a hint as to the specifications used in the construction of the response. | ||
| * This data structure appears only in the topmost JSON object of a response. | ||
| */ | ||
| private $rdapConformance = [ | ||
| 'rdap_level_0', | ||
| 'icann_rdap_technical_implementation_guide_1', | ||
| 'icann_rdap_response_profile_1', | ||
| 'redacted' | ||
| ]; | ||
|
|
||
| /** | ||
| * @return string[] | ||
| */ | ||
| public function getRdapConformance(): array | ||
| { | ||
| return $this->rdapConformance; | ||
| } | ||
|
|
||
| /** | ||
| * @param string[] $rdapConformance | ||
| */ | ||
| public function setRdapConformance(array $rdapConformance): void | ||
| { | ||
| $this->rdapConformance = $rdapConformance; | ||
| } | ||
|
|
||
| /** | ||
| * @param string $rdapConformance | ||
| */ | ||
| public function addRdapConformance(string $rdapConformance): void | ||
| { | ||
| $this->rdapConformance[] = $rdapConformance; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,130 @@ | ||||||||||||||||||||||
| <?php | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| namespace hiqdev\rdap\core\Domain\Entity; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| final class VCard implements \JsonSerializable | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| const VERSION = '4.0'; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** @var array */ | ||||||||||||||||||||||
| private $availableProperties = [ | ||||||||||||||||||||||
| 'version' => 'text', | ||||||||||||||||||||||
| 'fn' => 'text', | ||||||||||||||||||||||
| 'n' => 'text', | ||||||||||||||||||||||
| 'tel' => 'text', | ||||||||||||||||||||||
| 'email' => 'text', | ||||||||||||||||||||||
| 'org' => 'text', | ||||||||||||||||||||||
| 'adr' => 'text', | ||||||||||||||||||||||
| 'url' => 'uri', | ||||||||||||||||||||||
| ]; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** @var array */ | ||||||||||||||||||||||
| private $vCard; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function __construct(array $vCard = []) | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| $this->setVersion(self::VERSION); | ||||||||||||||||||||||
| array_merge($this->vCard, $vCard); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+24
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Constructor fails to merge provided array. The Apply this diff: public function __construct(array $vCard = [])
{
$this->setVersion(self::VERSION);
- array_merge($this->vCard, $vCard);
+ $this->vCard = array_merge($this->vCard, $vCard);
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function jsonSerialize() | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| return array_values($this->vCard); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function setVersion(string $version): VCard | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| return $this->setPropery('version', $version); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function setFullName(string $name, array $properties = []): VCard | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| return $this->setPropery('fn', $name, $properties); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function setName(string $name, array $properties = []): VCard | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| return $this->setPropery('n', $name, $properties); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function setEmail(string $email, array $properties = []): VCard | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| return $this->setPropery('email', $email, $properties); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function setOrg(?string $org = null, array $properties = []): VCard | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| if (empty($org)) { | ||||||||||||||||||||||
| return $this; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return $this->setPropery('org', $org, $properties); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function setCompany(?string $company = null, array $properties = []): VCard | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| return $this->setOrg($company, $properties); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function setUrl(string $url, array $properties = []): VCard | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| return $this->setPropery('url', $url, $properties); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function setTel(string $tel, array $properties = []): VCard | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| if (empty($properties) || empty($properties['type'])) { | ||||||||||||||||||||||
| $properties['type'] = 'voice'; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return $this->setPropery('tel', $tel, $properties); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function setAddress( | ||||||||||||||||||||||
| string $street, | ||||||||||||||||||||||
| string $city, | ||||||||||||||||||||||
| ?string $state = '', | ||||||||||||||||||||||
| string $zip, | ||||||||||||||||||||||
| ?string $country = null, | ||||||||||||||||||||||
| string $ext = '', | ||||||||||||||||||||||
| string $name = '', | ||||||||||||||||||||||
| array $properties = [] | ||||||||||||||||||||||
| ): VCard { | ||||||||||||||||||||||
| $properties['cc'] = strtoupper($properties['cc'] ?? $country); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| $this->vCard['adr'] = [ | ||||||||||||||||||||||
| 'adr', | ||||||||||||||||||||||
| $properties, | ||||||||||||||||||||||
| $this->availableProperties['adr'], | ||||||||||||||||||||||
| [ | ||||||||||||||||||||||
| $name, | ||||||||||||||||||||||
| $ext, | ||||||||||||||||||||||
| $street, | ||||||||||||||||||||||
| $city, | ||||||||||||||||||||||
| $state, | ||||||||||||||||||||||
| $zip, | ||||||||||||||||||||||
| '', | ||||||||||||||||||||||
| ], | ||||||||||||||||||||||
| ]; | ||||||||||||||||||||||
| return $this; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * @return array | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| public function getVCard(): array | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| return $this->vCard; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| protected function setPropery(string $property, string $value, array $properties = [], string $type = 'text'): VCard | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| $this->vCard[$property] = [ | ||||||||||||||||||||||
| $property, | ||||||||||||||||||||||
| empty($properties) ? new \stdClass() : $properties, | ||||||||||||||||||||||
| $this->availableProperties[$property] ?? $type, | ||||||||||||||||||||||
| $value | ||||||||||||||||||||||
| ]; | ||||||||||||||||||||||
| return $this; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+120
to
+129
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Undefined property reference and typo in method name. Line 127 references After fixing the property name at line 10, apply this diff: - protected function setPropery(string $property, string $value, array $properties = [], string $type = 'text'): VCard
+ protected function setProperty(string $property, string $value, array $properties = [], string $type = 'text'): VCard
{
$this->vCard[$property] = [
$property,
empty($properties) ? new \stdClass() : $properties,
$this->availableProperties[$property] ?? $type,
$value
];
return $this;
}And update all calls to
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| } | ||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,7 +33,7 @@ final class Event | |||||||||||||||||
| /** | ||||||||||||||||||
| * @var Link[] | ||||||||||||||||||
| */ | ||||||||||||||||||
| private $links = []; | ||||||||||||||||||
| private $links; | ||||||||||||||||||
|
Comment on lines
34
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update the docblock to reflect the nullable type. The docblock declares Apply this diff to update the docblock: /**
- * @var Link[]
+ * @var Link[]|null
*/
private $links;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| private function __construct(EventAction $eventAction, DateTimeImmutable $eventDate) | ||||||||||||||||||
| { | ||||||||||||||||||
|
|
@@ -88,8 +88,8 @@ public function getEventDate(): DateTimeImmutable | |||||||||||||||||
| /** | ||||||||||||||||||
| * @return Link[] | ||||||||||||||||||
| */ | ||||||||||||||||||
| public function getLinks(): array | ||||||||||||||||||
| public function getLinks(): ?array | ||||||||||||||||||
| { | ||||||||||||||||||
| return $this->links; | ||||||||||||||||||
| return $this->links ?? null; | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify eager SecureDNS initialization.
The constructor now always creates a
SecureDNS()instance with default values (delegationSigned=false). Since the property is typed asSecureDNS|null, this changes the behavior to never return null fromgetSecureDNS().Confirm this is intentional—previously the property could be null until explicitly set via
setSecureDNS().🏁 Script executed:
Length of output: 4596
Length of output: 2882
Length of output: 4060
Update return type annotation to reflect non-nullable contract.
The constructor now eagerly initializes
SecureDNSwith default values, ensuringgetSecureDNS()never returns null. However, the return type annotation remains?SecureDNS(line 123 insrc/Domain/Entity/Domain.php), which is now inaccurate.Change line 123 from:
to:
This reflects the actual contract that the property is always initialized after construction.
🤖 Prompt for AI Agents