Skip to content

Comments

Added RFC 3403-compliant guard clauses to DnsNAPTRRecordData constructor#51

Open
zbalkan wants to merge 5 commits intoTechnitiumSoftware:masterfrom
zbalkan:fix/Add-RFC-3403-compliant-guard-clauses-to-DnsNAPTRRecordData-constructor
Open

Added RFC 3403-compliant guard clauses to DnsNAPTRRecordData constructor#51
zbalkan wants to merge 5 commits intoTechnitiumSoftware:masterfrom
zbalkan:fix/Add-RFC-3403-compliant-guard-clauses-to-DnsNAPTRRecordData-constructor

Conversation

@zbalkan
Copy link
Contributor

@zbalkan zbalkan commented Feb 3, 2026

Solves #45

P.S: Renamed branch and reopened PR.

Signed-off-by: Zafer Balkan <zafer@zaferbalkan.com>
@zbalkan zbalkan force-pushed the fix/Add-RFC-3403-compliant-guard-clauses-to-DnsNAPTRRecordData-constructor branch from 27c6b6f to 98a3ba5 Compare February 3, 2026 17:12
Copilot AI review requested due to automatic review settings February 9, 2026 09:29
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to enforce RFC 3403 structural constraints at construction time for DnsNAPTRRecordData, preventing creation of syntactically invalid NAPTR records (per issue #45).

Changes:

  • Added null guard clauses for NAPTR <character-string> fields (flags, services, regexp, replacement).
  • Added validation for RFC 3403 constraints (REGEXP/REPLACEMENT exclusivity, FLAGS character set, basic SERVICES sanity).
  • Added a helper to validate regexp via .NET Regex compilation (plus new System.Text.RegularExpressions import).
Comments suppressed due to low confidence (1)

TechnitiumLibrary.Net/Dns/ResourceRecords/DnsNAPTRRecordData.cs:123

  • IsDomainNameValid(replacement, true) is called even when replacement contains a trailing dot; since IsDomainNameValid rejects empty labels, this will throw for any FQDN that ends with .. This further confirms the trailing-dot requirement above is incompatible with the validator. Normalize replacement (e.g., trim a terminal dot) before validation, or remove the trailing-dot requirement entirely to match other RR constructors.
            //  DNS <character-string> constraints 
            if (DnsClient.IsDomainNameUnicode(replacement))
                replacement = DnsClient.ConvertDomainNameToAscii(replacement);

            DnsClient.IsDomainNameValid(replacement, true);


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

@zbalkan zbalkan force-pushed the fix/Add-RFC-3403-compliant-guard-clauses-to-DnsNAPTRRecordData-constructor branch from a531352 to 6dcaff3 Compare February 9, 2026 12:25
nameof(flags));
}

// RFC 3403: SERVICES is a DNS <character-string>
Copy link
Member

Choose a reason for hiding this comment

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

can contain arbitrary chars ans since the SERVICES parameter leaves it up to the application, validation for this param should be removed.

nameof(services));
}

// RFC 3403: REPLACEMENT must be a fully qualified domain name
Copy link
Member

Choose a reason for hiding this comment

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

validation for REPLACEMENT is already in place done using DnsClient.IsDomainNameValid. So this is can be removed. This library internally does not allow domain names to end with . char.

@ShreyasZare
Copy link
Member

Thanks for the PR. Added couple of comments for change. Rest changes look good.

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.

2 participants