zonemaster-engine
zonemaster-engine copied to clipboard
Separate functions to trim whitespace and normalize domain name
Purpose
This PR separates the normalization of what's logically part of domain names from the white space stripping that is also specified in RequirementsAndNormalizationOfDomainNames.md to make it possible to apply them individually.
Context
This PR fixes item 3 in zonemaster/zonemaster-cli#364.
zonemaster/zonemaster-backend#1143 should be merged together with this one.
This is technically a major change because normalize_name() is part of the public API. However, even though we've included an implementation in prior releases, we've never actually called it (except from unit tests).
Changes
- This PR splits out trim_space() from normalize_name().
- As a drive-by change it also improves the quality of the associated unit test.
How to test this PR
Zonemaster CLI should present the following behaviors:
$ zonemaster-cli --show-testcase --no-ipv6 --test basic/basic01 'example .com'
Domain name has an ASCII label ("example ") with a character not permitted.
$ zonemaster-cli --show-testcase --no-ipv6 --test basic/basic01 'example '
Domain name has an ASCII label ("example ") with a character not permitted.
$ zonemaster-cli --show-testcase --no-ipv6 --test basic/basic01 ' '
Domain name has an ASCII label (" ") with a character not permitted.
$ zonemaster-cli --raw --level info --no-ipv6 --test basic01 zonemaster.fr --ns 'd.nic.fr. /194.0.9.1'
Invalid name in --ns argument:
Domain name has an ASCII label (" ") with a character not permitted.
$ zonemaster-cli --raw --level info --no-ipv6 --test basic01 zonemaster.fr --ns 'd.nic.fr./ 194.0.9.1'
Invalid IP address in --ns argument:
Invalid chars in IP 194.0.9.1
@matsduf has expressed that he wants to update RequirementsAndNormalizationOfDomainNames.md before this is merged.
@matsduf Regarding your comment above, as you remember, we discussed this topic at a work group meeting a while ago. I believe it was after your comment, but it seems we forgot to note our conclusions here. Instead of leaving (seemingly) unanswered questions hanging, it seems appropriate to summarize some clarifications along the lines of our discussion as I remember it.
- The test case specifications should not specify the trimming of surrounding space as part of the definition of any of its data types (e.g. domain names). It is questionable if the test case specifications need to specify space trimming at all. But if they do, they should specify a common trimming algorithm that can be applied (or avoided) at the discretion of user interface implementations to conform whatever expectations apply to that particular kind of user interface.
- GUI should trim spaces from values in any text field (e.g. domain name fields) before doing anything with them (e.g. before sending them to Backend). This is what you'd expect from a web application and we don't have any reasons to deviate.
- RPCAPI should not trim space from any of its arguments. It exposes an application programming interface and clients are expected to say what they mean and not be sloppy in their requests.
-
Neither
zmb
norzmtest
should trim space from any of its arguments. You wouldn't expect a command line tool to do that. - Additionally,
zmb
should definitely not trim space from any of its arguments because we need it to be able to test how Backend responds to requests both with and without surrounding space in all request arguments.
@matsduf, @marc-vanderwal, @tgreenx, @MichaelTimbert do you guys agree with the above?
@mattias-p, please see https://github.com/zonemaster/zonemaster/pull/1260.
@mattias-p, you do not trimming to be part of the normalization specification (you write "test case specification" but I think you mean the normalization document), but you want the normalization to happen in GUI. Where do you want that to be specified or do you prefer it to be unspecified.
I am not sure that I agree with your bullets, but I will not totally object. A requirement is that operations happening are documented and specified.