Skip to content

INTGRTNS-500-Fix-external-nameserver-ip-resolution-in-createNameserversArray#545

Open
avishka-ra wants to merge 2 commits into
masterfrom
INTGRTNS-500-Fix-external-nameserver-ip-resolution-in-createNameserversArray
Open

INTGRTNS-500-Fix-external-nameserver-ip-resolution-in-createNameserversArray#545
avishka-ra wants to merge 2 commits into
masterfrom
INTGRTNS-500-Fix-external-nameserver-ip-resolution-in-createNameserversArray

Conversation

@avishka-ra

@avishka-ra avishka-ra commented May 15, 2026

Copy link
Copy Markdown
Collaborator

What this fixes

Closes #525.

createNameserversArray() was attempting to resolve an IP for every nameserver before including it in the request. For external nameservers (Cloudflare, Sectigo, OP's own NS, etc.) this is unnecessary — the OP API only requires a hostname for these. The IP lookup would silently drop any NS it couldn't resolve, causing operations to fail or proceed with an incomplete nameserver set.

What changed

The function now checks whether each NS is a glue record (i.e. the NS hostname is a child of the domain being processed — ns1.abc.com for abc.com) before doing any IP resolution.

  • Glue records: IP resolution is still performed — first from params, then searchNsRequest, then gethostbyname(). If no IP can be found, a clear exception is thrown.
  • External nameservers: IP lookup is skipped entirely. Only the hostname is sent to the OP API.

Affects domain registration, transfer, and save nameservers — all three call this method.

Copilot AI review requested due to automatic review settings May 15, 2026 01:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes APITools::createNameserversArray() so that IP resolution is only required for true glue records (NS hostnames that are a child of the domain being processed). External nameservers are now passed to the OpenProvider API with only their hostname, instead of being silently dropped when no IP could be resolved. The change affects domain registration, transfer, and "save nameservers" flows.

Changes:

  • Add a glue-record check (str_ends_with against sld.tld) and short-circuit external NS entries to be sent without IP resolution.
  • Keep the existing IP-resolution chain (params → searchNsRequest/getNameserverListgethostbyname) only for glue records, and throw/aggregate clearer errors when resolution fails.
  • Update user-facing error messages and replace the $invalidNameServer flag with an $invalidGlueNameServers list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread modules/registrars/openprovider/OpenProvider/API/APITools.php
Comment thread modules/registrars/openprovider/OpenProvider/API/APITools.php Outdated
Comment thread modules/registrars/openprovider/OpenProvider/API/APITools.php Outdated
Comment thread modules/registrars/openprovider/OpenProvider/API/APITools.php
Comment thread modules/registrars/openprovider/OpenProvider/API/APITools.php
Comment thread modules/registrars/openprovider/OpenProvider/API/APITools.php Outdated
@avishka-ra avishka-ra force-pushed the INTGRTNS-500-Fix-external-nameserver-ip-resolution-in-createNameserversArray branch from a1410ca to 21b9b5b Compare May 15, 2026 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] transferDomain silently replaces external nameservers with module defaults — APITools::createNameserversArray regression

3 participants