Thanks Paul for the detailed review. Please see inline
On Mon, 21 Apr 2025 at 04:15, Paul Kyzivat <pkyzivat@xxxxxxxxxxxx> wrote:
Reviewer: Paul Kyzivat
Review result: Ready with Nits
I am the assigned ARTART reviewer for this Internet-Draft.
Document: draft-ietf-dnsop-structured-dns-error-12
Reviewer: Paul Kyzivat
Review Date: 2025-04-20
IETF LC End Date: 2025-04-28
IESG Telechat date: ?
Summary: This draft is on the right track but has open issues, described
in the review.
ISSUES: 7
NITS: 1
Issues:
1) NIT: Section 4 - c: (contact)
This allows sips but not sip URIs. Sips is not widely used.
Please consider allowing sip URLs.
Allowing "sip" URI introduces security issues, "sips" offers encrypted transport for SIP messages.
2) ISSUE: Section 4 - s: (suberror)
This field lacks a specification of its type.
It appears that "suberror" here is intended to be the same as
"sub-error" in section 7 and "SubError" in section 11.3. Please use a
consistent spelling throughout. And then specify here that the type of
this field is an integer with values defined in the new IANA registry.
Thanks, updated draft to use "sub-error".
3) ISSUE: Section 8 - Extended DNS Error Code
The phrasing here, for both the section title and the content, is odd
and confusing. For clarity and consistency with section 7, I suggest a
title of "New Extended DNS Error Code Definition".
And then the body could start with: "This document defines the following
new IANA-registered Extended DNS Error Code." The existing text will
then require some tweaking to align with this rephrasing.
And then to avoid confusion, perhaps change the title of section 11.4 to
"New Extended DNS Error Code Registration".
No, the section title and its body is consistent with the sections in RFC 8914 defining Extended DNS Error Codes, please see https://datatracker.ietf.org/doc/html/rfc8914#section-4
4) ISSUE: Section 9 - Examples
I fail to see how Figure 2 represents the same content as Figure 1. If
it does, can you please explain?
The script in https://github.com/ietf-wg-dnsop/draft-ietf-dnsop-structured-dns-error/blob/main/examples/minified.json was supposed to update Figure 2, I fixed it.
5) ISSUE: Section 11.1 - New Registry for JSON Names
Some of the fields described in the text are inconsistent with the
fields contained in Table 1: "Short Description" vs. "Description", and
no text description of "Full JSON Name".
Also, is "Full JSON Name" appropriate? IIUC it has no role in JSON.
Rather, it is just a human meaningful long form of the JSON Name, or
perhaps a shorter form of the "Short Description". I suggest rethinking
what you are calling these things.
Good point, I replaced "Full JSON Name" with "Field meaning" and addressed the above comment as well.
6) ISSUE: Section 11.2 - New Registry for Contact URI Scheme
Could you please add some text describing the role and responsibilities
of the Change Controller? What sort of changes are allowed? More than
additions?
IETF review is required to update the registry, see https://datatracker.ietf.org/doc/html/rfc8126#section-4.8, change controller is IETF.
7) ISSUE: Section 11.3 - New Registry for DNS SubError Codes
I don't understand what you mean by "RFC8914 error code applicability".
First, what do you mean by "RFC8914 error code"? Do you mean the
"Extended DNS Error Codes" defined in RFC8914?
Yes, updated to use "Extended DNS Error Codes".
Next, what do you mean by "applicability"? Do you mean the "Extended DNS
Error Codes" for which the "SubError Codes" may be used?
Yes.
Please clarify these.
Also, again, could you please add some text describing the role and
responsibilities of the Change Controller? What sort of changes are
allowed? More than additions?
IETF review is required to update the registry, see https://datatracker.ietf.org/doc/html/rfc8126#section-4.8, change controller is IETF.
8) ISSUE: JSON Name
Throughout the document you use "JSON Name" to describe a specific field
in a specific JSON document format. This isn't descriptive of the
purpose of the field. I suggest changing this to something more
descriptive - perhaps "EXTRA-TEXT Field Name".
Yes, fixed.
Cheers,
-Tiru
-- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
