Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

- Fixes case issue when comparing domain name in Zone10 #737

Merged
merged 1 commit into from
May 20, 2020
Merged

- Fixes case issue when comparing domain name in Zone10 #737

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/Zonemaster/Engine/Test/Zone.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use 5.014002;
use strict;
use warnings;

use version; our $VERSION = version->declare( "v1.0.11" );
use version; our $VERSION = version->declare( "v1.0.12" );

use Zonemaster::Engine;

Expand Down Expand Up @@ -699,14 +699,14 @@ sub zone10 {
}
);
}
elsif ( $soa[0]->owner ne $name->fqdn ) {
elsif ( lc( $soa[0]->owner ) ne lc( $name->fqdn ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

I note that lc can use either ASCII rules or Unicode rules depending on context. Are we sure that either ASCII rules will always be applied here, or that LATIN SMALL LETTER I is lowercase of LATIN CAPITAL LETTER I even in Turkish locales?

Also, case insensitivity only applies to ASCII labels, right? lc operates character by character, so this will fail to catch some errors for some binary labels. But maybe we don't care about binary labels?

Copy link
Contributor

@matsduf matsduf May 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

No, in Turkish locale lower case of LATIN CAPITAL LETTER I is LATIN SMALL LETTER DOTLESS I.

It is little bit unclear, but in practice we disregard binary labels. It is also unclear about IDN labels, but if they were entered as U-labels they are already converted to A-labels when we come here and can here be treated as ASCII labels.

It is good that you raise the issue, and it should be handled, but this fix is to the better rather than the worse.

push @results,
info(
WRONG_SOA => {
ns => $ns->name->string,
address => $ns->address->short,
owner => $soa[0]->owner,
name => $name->fqdn,
owner => lc( $soa[0]->owner ),
name => lc( $name->fqdn ),
}
);
}
Expand Down