Skip to content

Convert Basic00 to be a general specification#942

Merged
matsduf merged 18 commits into
zonemaster:developfrom
matsduf:update-basic00
Sep 13, 2022
Merged

Convert Basic00 to be a general specification#942
matsduf merged 18 commits into
zonemaster:developfrom
matsduf:update-basic00

Conversation

@matsduf

@matsduf matsduf commented May 4, 2021

Copy link
Copy Markdown
Contributor
  • Formats specification after new template.
  • Adds explicit normalization of the domain name to be tested.
  • Adds explicit handling of IDN names.

Issue #614 has requirements on Basic00 that are not met in this PR. See comment in the issue.

Updates 2022-07-11:

As agreed, test case Basic00 is removed and turned into a general specification. That transformation has been done with the following two commits:

  • 8a8bc90 renames the file
  • f734a6f updates the document to be in the new role. These updates do not change any logic.

* Formats specification after new template.
* Adds explicit normalization of the domain name to be tested.
* Adds explicit handling of IDN names.
Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
matsduf added 2 commits July 13, 2021 12:08
* Applies new template.
* Removes handling of leading or trailing space.
* Added description of handling of underscore.
* Added links to Unicode database for characters
* Updates wording, no other update of logic.
@matsduf matsduf requested a review from hannaeko July 20, 2021 15:53
@matsduf

matsduf commented Jul 20, 2021

Copy link
Copy Markdown
Contributor Author

@blacksponge, please review again. I have applied the draft template on the test case in this PR.

@hannaeko hannaeko 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.

just a few remaining typos, otherwise looks good to me

Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
@matsduf matsduf requested a review from hannaeko July 21, 2021 14:50

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Beside my comments on some typos, I think this specifications looks good.

Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
Comment thread docs/specifications/tests/Basic-TP/basic00.md Outdated
@matsduf matsduf requested a review from a user July 29, 2021 08:03
ghost
ghost previously approved these changes Jul 29, 2021
hannaeko
hannaeko previously approved these changes Jul 29, 2021
@matsduf

matsduf commented Aug 31, 2021

Copy link
Copy Markdown
Contributor Author

@mattias-p and @vlevigneron, can you review this?

Comment thread docs/specifications/tests/RequirementsAndNormalizationOfDomainNames.md Outdated
Comment thread docs/specifications/tests/RequirementsAndNormalizationOfDomainNames.md Outdated
Comment thread docs/specifications/tests/RequirementsAndNormalizationOfDomainNames.md Outdated
Comment on lines +93 to +96
[RFC 1123][RFC 1123#2.1], section 2.1, specifies that a domain name label
may not start or end with a HYPHEN-MINUS ("-"), only digit or letter. This
restriction on HYPHEN-MINUS is disregarded in this specification and is assumed
to be handled in test case [Syntax02].

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we plan to maybe migrate the SYNTAX02 into BASIC00? I think it would make sense since it can be seen as part of checking that the input domain is valid.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I do not think we should.

Comment thread docs/specifications/tests/RequirementsAndNormalizationOfDomainNames.md Outdated
Comment thread docs/specifications/tests/RequirementsAndNormalizationOfDomainNames.md Outdated
Comment thread docs/specifications/tests/RequirementsAndNormalizationOfDomainNames.md Outdated
Comment thread docs/specifications/tests/RequirementsAndNormalizationOfDomainNames.md Outdated
Comment thread docs/specifications/tests/RequirementsAndNormalizationOfDomainNames.md Outdated
see the [Severity Level Definitions] document.

The argument names in the Arguments column lists the arguments used in the
message. The argument names are defined in the [argument list].

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looking at the logentry_args.md document, it looks like the dlabel is not under the "Defined arguments" section. Maybe update the document as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Chose to define "label" (not "dlabel") in PR zonemaster/zonemaster-engine#1126.

By commit 78732ed to this PR, "dlabel" is replaced by "label".

* Adds explicit NFC normalization.
* Adds explicit control of empty domain.
* Updates handling of LATIN CAPITAL LETTER I WITH DOT ABOVE.
* Corrected message tags.
* Editorial updates from review comments.
Comment thread docs/specifications/tests/RequirementsAndNormalizationOfDomainNames.md Outdated
Comment thread docs/specifications/tests/RequirementsAndNormalizationOfDomainNames.md Outdated
@matsduf matsduf requested a review from hannaeko September 2, 2022 20:59
@matsduf

matsduf commented Sep 2, 2022

Copy link
Copy Markdown
Contributor Author

@PNAX, please re-review.

@hannaeko hannaeko 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.

Looks good to me, thank you for all the edits!

@matsduf

matsduf commented Sep 13, 2022

Copy link
Copy Markdown
Contributor Author

A first step to implementation is found in zonemaster/zonemaster-engine#1040

@matsduf

matsduf commented Sep 13, 2022

Copy link
Copy Markdown
Contributor Author

@blacksponge, can I merge this? Will you continue on zonemaster/zonemaster-engine#1040? By this update, current Basic00 goes away.

@hannaeko

Copy link
Copy Markdown
Contributor

Yes you can, the implementation at zonemaster/zonemaster-engine#1040 should be up to date with this specification, I will mark the PR as ready so you review it again

@matsduf matsduf merged commit fccb390 into zonemaster:develop Sep 13, 2022
@matsduf matsduf deleted the update-basic00 branch September 13, 2022 14:34
matsduf added a commit to matsduf/zonemaster that referenced this pull request Dec 20, 2022
@matsduf matsduf mentioned this pull request Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-TestCase Area: Test case specification or implementation of test case

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants