Rewrite Zone09 fully after specification update by marc-vanderwal · Pull Request #1543 · zonemaster/zonemaster-engine

marc-vanderwal · 2026-07-20T07:38:32Z

Purpose

This PR is a complete rewrite of the Zone09 test case, after its specification and test scenarios were updated.

The previous implementation had two main issues: firstly, MX RRsets were not sorted before comparison; secondly, MX RRsets that only differ by TTL values were not deemed equal, although we wanted them to compare equal. Now, only the preference and exchange fields of the RDATA are used for sorting and comparison.

Test scenarios were updated as well, so this commit also updates the corresponding unit tests. Said unit tests are ported to the DSL for good measure. When relevant, even the messages’ arguments are tested.

Context

See:

MX records returned in random order causes warning ? (Zone09) zonemaster#1515 (issue pinpointing a deficiency in Zone09)
Updates Zone09 zonemaster#1516 (specification rewrite, also addressing said deficiency)
Updates Zone09 scenarios to match updated specification zonemaster#1517 (test scenario update)

Changes

Complete rewrite of Zone09 implementation
Complete rewrite of Zone09 unit tests, leveraging the DSL
Require Perl 5.26 in lib/Zonemaster/Engine/Test/Zone.pm so that lexical subroutines (state sub) can be used inside methods
Also refactor Zone11 a little bit as a drive-by change

How to test this PR

Unit tests should still pass.

matsduf · 2026-07-20T07:49:32Z

Is the failing unit test due to error in a hint file for the scenarios?

marc-vanderwal · 2026-07-20T07:54:47Z

It’s an obsolete unit test file I forgot to delete. Previously, when different root hints are to be used for a scenario, that scenario had to go in a different .t file. This isn’t necessary anymore with the DSL-based tests.

matsduf · 2026-07-20T10:20:24Z

⚠️ Draft PR – Specification and test scenarios should be stable enough but aren’t merged yet.

Is this still a draft PR?

marc-vanderwal · 2026-07-20T10:32:12Z

⚠️ Draft PR – Specification and test scenarios should be stable enough but aren’t merged yet.

Is this still a draft PR?

It’s ready for review, but it shouldn’t be merged yet. I just hope that the specifications and the scenarios are stable enough so that this code is stable too.

matsduf · 2026-07-20T11:24:15Z

It’s ready for review, but it shouldn’t be merged yet. I just hope that the specifications and the scenarios are stable enough so that this code is stable too.

We should have an additional review on both PRs. I will soon review this PRs.

…ne#1543

matsduf

I have tested the scenarios with zonemaster-cli while updating the test-zones-output.md file. I have also inspected the normal output (non-raw). Everything looks fine and as expected.

matsduf · 2026-07-23T07:51:24Z

zonemaster-cli   --test zone09 --hints hintfile.zone --level info mx-data.zone09.xa

Seconds Level    Message
======= ======== =======
   0.00 INFO     Using version v9.0.0 of the Zonemaster engine.
   0.07 INFO     The MX RDATA in the MX RRset, "10 mail.mx-data.zone09.xa.", as returned by name servers "ns1.mx-data.zone09.xa/127.19.9.31;ns1.mx-data.zone09.xa/fda1:b2:c3:0:127:19:9:31;ns2.mx-data.zone09.xa/127.19.9.32;ns2.mx-data.zone09.xa/fda1:b2:c3:0:127:19:9:32".

The mail exchange domain name has a final dot '.' in the presentation, but in name/IP pairs the final dot is removed. It looks better and it is easier to read if the final dot of a domain name is always removed (unless it is the root node).

marc-vanderwal · 2026-07-23T09:35:44Z

The mail exchange domain name has a final dot '.' in the presentation, but in name/IP pairs the final dot is removed. It looks better and it is easier to read if the final dot of a domain name is always removed (unless it is the root node).

Good point. I’ve fixed that.

matsduf · 2026-07-23T10:30:44Z

Yesterday I ran the same zonemaster-cli commands for the scenarios, but without --raw to inspect the msgids. That is when I saw the final dots. I think we have them from other test cases too.

See scenarios in zonemaster/zonemaster#1517. Scenarios MIXED-CASE-RDATA-1 and MIXED-CASE-RDATA-2 do not pass.

matsduf

I find a deviation:

Scenario name	Mandatory message tags	Forbidden message tags
MIXED-CASE-RDATA-2	Z09_MX_DATA	2)

zonemaster-cli --raw  --test zone09 --hints hintfile.zone --level info mixed-case-rdata-2.zone09.xa
   0.00 INFO     GLOBAL_VERSION  version=v9.0.0
   0.07 INFO     Z09_MX_DATA  mxrdata_list=20 mail2.mixed-case-rdata-2.zone09.xa;20 mail2.mixed-case-rdata-2.zone09.xa; ns_list=ns1.mixed-case-rdata-2.zone09.xa/127.19.9.31;ns1.mixed-case-rdata-2.zone09.xa/fda1:b2:c3:0:127:19:9:31
   0.07 INFO     Z09_MX_DATA  mxrdata_list=20 mail2.mixed-case-rdata-2.zone09.xa; ns_list=ns2.mixed-case-rdata-2.zone09.xa/127.19.9.32;ns2.mixed-case-rdata-2.zone09.xa/fda1:b2:c3:0:127:19:9:32
   0.07 WARNING  Z09_INCONSISTENT_MX_DATA

matsduf · 2026-09-24T16:59:25Z

+scenario 'MIXED-CASE-RDATA-2' => sub {
+    expect Z09_MX_DATA => {
+        ns_list => 'ns1.mixed-case-rdata-2.zone09.xa/127.19.9.31;'
+                 . 'ns1.mixed-case-rdata-2.zone09.xa/fda1:b2:c3:0:127:19:9:31;'
+                 . 'ns2.mixed-case-rdata-2.zone09.xa/127.19.9.32;'
+                 . 'ns2.mixed-case-rdata-2.zone09.xa/fda1:b2:c3:0:127:19:9:32',
+        mxrdata_list => '20 mail2.mixed-case-rdata-2.zone09.xa'
+    };
+    forbid_others;
+};


This looks correct, but why did it pass?

matsduf · 2026-09-24T17:00:51Z

I did run the new code since MIXED-CASE-RDATA-1 passed now.

marc-vanderwal · 2026-09-25T06:36:55Z

I tried rerecording the data file and now the MIXED-CASE-RDATA-2 scenario fails with the same error as what you got:

    #   Failed test 'Messages of tag 'Z09_MX_DATA' exist with specified arguments'
    #   at t/Test-zone09.t line 177.
    # Looked for a message whose tag is 'Z09_MX_DATA'
    #   where 'mxrdata_list' equals '20 mail2.mixed-case-rdata-2.zone09.xa'
    #     and 'ns_list' equals 'ns1.mixed-case-rdata-2.zone09.xa/127.19.9.31;ns1.mixed-case-rdata-2.zone09.xa/fda1:b2:c3:0:127:19:9:31;ns2.mixed-case-rdata-2.zone09.xa/127.19.9.32;ns2.mixed-case-rdata-2.zone09.xa/fda1:b2:c3:0:127:19:9:32'
    #     and contains no argument other than those listed above
    # Here are all messages that unsuccessfully matched:
    #   Z09_MX_DATA mxrdata_list=20 mail2.mixed-case-rdata-2.zone09.xa;20 mail2.mixed-case-rdata-2.zone09.xa; ns_list=ns1.mixed-case-rdata-2.zone09.xa/127.19.9.31;ns1.mixed-case-rdata-2.zone09.xa/fda1:b2:c3:0:127:19:9:31
    #   Z09_MX_DATA mxrdata_list=20 mail2.mixed-case-rdata-2.zone09.xa; ns_list=ns2.mixed-case-rdata-2.zone09.xa/127.19.9.32;ns2.mixed-case-rdata-2.zone09.xa/fda1:b2:c3:0:127:19:9:32

    #   Failed test 'Tag 'Z09_INCONSISTENT_MX_DATA' is not outputted'
    #   at t/Test-zone09.t line 184.
    # Tag 'Z09_INCONSISTENT_MX_DATA' shouldn't have been outputted, but it was
#   Failed scenario 'MIXED-CASE-RDATA-2'
#   at t/Test-zone09.t line 185.

But when I examine the packets in t/Test-zone09.data file for that scenario, the MX hostnames in the RRsets are stored in lowercase even though they originally were in mixed case.

$ ./util/data2dig t/Test-zone09.data | less

[...]

;; Query to ns1.arpa-email-domain.zone09.arpa (127.19.9.31) over UDP
;; ->>HEADER<<- opcode: QUERY, rcode: NOERROR
;; flags: ; QUERY: 1, ANSWER: 0, AUTHORITY: 0, ADDITIONAL: 0
;; QUESTION SECTION:
;; mixed-case-rdata-2.zone09.xa.        IN      MX
;; MSG SIZE  sent: 46

;; ->>HEADER<<- opcode: QUERY, rcode: NOERROR, id: 0
;; flags: qr aa ; QUERY: 1, ANSWER: 2, AUTHORITY: 0, ADDITIONAL: 0
;; QUESTION SECTION:
;; mixed-case-rdata-2.zone09.xa.        IN      MX

;; ANSWER SECTION:
mixed-case-rdata-2.zone09.xa.   600     IN      MX      20 mail2.mixed-case-rdata-2.zone09.xa.
mixed-case-rdata-2.zone09.xa.   600     IN      MX      20 mail2.mixed-case-rdata-2.zone09.xa.

;; AUTHORITY SECTION:

;; ADDITIONAL SECTION:

;; Query time: 0 msec
;; SERVER: 127.19.9.31
;; WHEN: Fri Sep 25 08:28:24 2026
;; MSG SIZE  rcvd: 84

So that’s why the unit tests pass when we are not rerecording the data, while they fail when we are rerecording the packets. That’s a bug and I’ll create an issue. Meanwhile, the workaround is to test manually like you did.

matsduf · 2026-09-25T07:42:10Z

There is a bug in the implementation not removing the duplicate. Will you fix that?

marc-vanderwal · 2026-09-25T07:57:42Z

Yes, I’ve spotted it and I’m committing the fix right now.

Rewrite the implementation of Zone09 completely, after the specification was updated. The previous implementation had two main issues: firstly, MX RRsets were not sorted before comparison; secondly, MX RRsets that only differ by TTL values were not deemed equal, although we wanted them to compare equal. Now, only the preference and exchange fields of the RDATA are used for sorting and comparison. Test scenarios were updated as well, so this commit also updates the corresponding unit tests. Said unit tests are ported to the DSL for good measure. When relevant, even the messages’ arguments are tested.

The rewriting of Zone09 also involved a migration from old Methods to MethodsV2, leading to a few more DNS queries made to authoritative servers in parent zones. The corresponding packets do not exist in the corresponding t/Test-zone.data file, so running this unit test fails. It cannot easily be rerecorded either and the .t file barely exercised the code anyway, so we can afford not to run Zone09 in that legacy test.

Use the newly-introduced _is_non_mail_domain() method and rewrite the logic of Zone11 in a way that is equivalent to the current logic, while involving fewer nested ifs.

marc-vanderwal · 2026-09-25T08:00:26Z

The remaining failing scenario (MIXED-CASE-RDATA-2) is marked as not testable because of #1554. But if you run it manually with zonemaster-cli, it should work now.

matsduf

Now all scenarios pass, see zonemaster/zonemaster#1517

marc-vanderwal · 2026-09-25T13:26:50Z

Good! Merging this PR, then.

marc-vanderwal added this to the v2026.1.1 milestone Jul 20, 2026

marc-vanderwal requested review from MichaelTimbert, matsduf, mattias-p and tgreenx July 20, 2026 07:38

marc-vanderwal added V-Patch Versioning: The change gives an update of patch in version. RC-Fixes Release category: Fixes. labels Jul 20, 2026

marc-vanderwal marked this pull request as draft July 20, 2026 07:53

matsduf reviewed Jul 20, 2026

View reviewed changes

Comment thread lib/Zonemaster/Engine/Test/Zone.pm

marc-vanderwal force-pushed the bugfix/zone09-false-warnings branch from b86c93f to b4b29bc Compare July 20, 2026 08:27

marc-vanderwal marked this pull request as ready for review July 20, 2026 08:31

marc-vanderwal force-pushed the bugfix/zone09-false-warnings branch from b4b29bc to 00c3a82 Compare July 20, 2026 09:46

matsduf added a commit to matsduf/zonemaster that referenced this pull request Jul 22, 2026

Updated based on updated implementation in zonemaster/zonemaster-engi…

8556ddf

…ne#1543

matsduf mentioned this pull request Jul 22, 2026

Updates Zone09 scenarios to match updated specification zonemaster/zonemaster#1517

Merged

matsduf previously approved these changes Jul 22, 2026

View reviewed changes

marc-vanderwal dismissed matsduf’s stale review via 0e676b4 July 23, 2026 09:11

matsduf previously approved these changes Sep 17, 2026

View reviewed changes

tgreenx reviewed Sep 22, 2026

View reviewed changes

Comment thread lib/Zonemaster/Engine/Test/Zone.pm

Comment thread lib/Zonemaster/Engine/Test/Zone.pm

Comment thread lib/Zonemaster/Engine/Test/Zone.pm

Comment thread lib/Zonemaster/Engine/Test/Zone.pm

tgreenx reviewed Sep 22, 2026

View reviewed changes

Comment thread lib/Zonemaster/Engine/Test/Zone.pm

marc-vanderwal force-pushed the bugfix/zone09-false-warnings branch from 0e676b4 to 83bb597 Compare September 24, 2026 15:24

marc-vanderwal requested a review from tgreenx September 24, 2026 15:28

marc-vanderwal requested a review from matsduf September 24, 2026 15:28

tgreenx previously approved these changes Sep 24, 2026

View reviewed changes

matsduf requested changes Sep 24, 2026

View reviewed changes

matsduf reviewed Sep 24, 2026

View reviewed changes

marc-vanderwal mentioned this pull request Sep 25, 2026

Packets recorded in data files may not reflect the packets actually received in wire format #1554

Open

marc-vanderwal added 3 commits September 25, 2026 09:58

Refactor Zone11 a little bit

7271ac8

Use the newly-introduced _is_non_mail_domain() method and rewrite the logic of Zone11 in a way that is equivalent to the current logic, while involving fewer nested ifs.

marc-vanderwal dismissed tgreenx’s stale review via 7271ac8 September 25, 2026 07:59

marc-vanderwal force-pushed the bugfix/zone09-false-warnings branch from 83bb597 to 7271ac8 Compare September 25, 2026 07:59

marc-vanderwal requested a review from matsduf September 25, 2026 08:02

matsduf approved these changes Sep 25, 2026

View reviewed changes

marc-vanderwal merged commit b6e0b3a into zonemaster:develop Sep 25, 2026
4 checks passed

Conversation

marc-vanderwal commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Context

Changes

How to test this PR

Uh oh!

matsduf commented Jul 20, 2026

Uh oh!

marc-vanderwal commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

matsduf commented Jul 20, 2026

Uh oh!

marc-vanderwal commented Jul 20, 2026

Uh oh!

matsduf commented Jul 20, 2026

Uh oh!

matsduf left a comment

Choose a reason for hiding this comment

Uh oh!

matsduf commented Jul 23, 2026

Uh oh!

marc-vanderwal commented Jul 23, 2026

Uh oh!

matsduf commented Jul 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matsduf left a comment

Choose a reason for hiding this comment

Uh oh!

matsduf Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

matsduf commented Sep 24, 2026

Uh oh!

marc-vanderwal commented Sep 25, 2026

Uh oh!

matsduf commented Sep 25, 2026

Uh oh!

marc-vanderwal commented Sep 25, 2026

Uh oh!

marc-vanderwal commented Sep 25, 2026

Uh oh!

matsduf left a comment

Choose a reason for hiding this comment

Uh oh!

marc-vanderwal commented Sep 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marc-vanderwal commented Jul 20, 2026 •

edited

Loading

marc-vanderwal commented Jul 20, 2026 •

edited

Loading