[MSHARED-609] Partially revert MSHARED-429

[jira-importer]

Michael Osipov opened MSHARED-609 and commented

MSHARED-429 introduced handling of hostnames endling with .local though they are invalid in the way they are used.

Copied from the ticket:
I'd seriously like to revert this partially for 3.0:

• Your DNS setup is simply broken. .local is a reserved TLD for mDNS resolution. This is not meant to be used in private networks. Doing so breaks Avahi on Linux/FreeBSD, Bonjour on macOS and everything else using zeroconf. You should register a domain name and use subdomains on your private network (https://de.wikipedia.org/wiki/Zeroconf#Multicast_DNS).
• It does not accept full 16-bit unsigned integer
• You always have to update with the newest pattern in Commons Validator

Local hostnames (unqualified) can be validated by passing an option/flag to the validator. The rest of the patch, missing TLDs, etc. are already in Commons Validator 1.5.1.

We should not encourage bad setups.

---

Affects: maven-reporting-impl 2.4

Issue Links:

• MSHARED-611 Drop any href validation and pass as-is
  ("is part of")

• VALIDATOR-412 No options for Pseudo TLD

• VALIDATOR-411 UrlValidator accepts ports above max limit of 16-bit unsigned integer
  ("requires")

• MSHARED-429 Supposedly invalid URLs are rendered as plain text
  ("supercedes")

[jira-importer]

Jan Schultze commented

I would like to point out, that the ??bad setup?? is a bad network setup and has nothing to do with the Maven setup. In most organizations developers cannot change the network setup and they might be forced (as my team is) to use a URL ending in .local if such a setup is in place. Disallowing this does not encourage network operators to do anything as they do not consult the Maven source code for best practices in networking. However, it effectively discourages some from using Maven sites as the behavior of rendering "invalid" urls. Instead of linking to them they are displayed directly and this appears as bug that makes the sites cumbersome to use. I consider this to be a seriously bad trade-off.

If for some reason maven users should truly be discouraged from using URLs ending in .local as a project.url, than Maven should warn about such usage while building projects and not when a dependent project builds its site.

It accepts ports > 16-bit unsigned integer

Actually it does not even accept ports greater than 59999. But that is only me and my unit tests - do you have an example URL?

You always have to update with the newest pattern in Commons Validator

I do not understand what you mean by pattern. The Regexps describing the valid authority? I consider the rules for hostnames, ports etc. as very stable (though not immutable).

Local hostnames (unqualified) can be validated by passing an option/flag to the validator.

Already in place:

new UrlValidator( SCHEMES, authorityValidator, UrlValidator.ALLOW_LOCAL_URLS );

[jira-importer]

Michael Osipov commented

• Exactly, I guess you have raised this with your network admin and he was too stupid to understand his fault, didn't he? Is your IP range in 169.254.0.0/16? I do not expect net ops to read source code, but rather RFCs and documentations.
• Refined my statement, It blocks every port above 60000 but below 16-bit unsigned integer. A bug.

I am not judging your code as a contribution, but you have rather fixed the wrong end, Maven only. I would rather consider this being a UrlValidator flag:

new UrlValidator( UrlValidator.ALLOW_LOCAL_URLS | UrlValidator.ALLOW_MDNS_HOSTNAMES ); // or ALLOW_LINK_LOCAL_URLS, etc.

Beneath, DomainValidator gets isValidLinkLocal...(String) and your are done.

This would be reasonable.

FWIW, I can commit a patch to Commons Validator if you provide one.

[jira-importer]

Jan Schultze commented

Agreed. The port range should be extended.

And yes, my patch did fix the wrong end. However, that was a conscious decision based on the following reasoning. I wanted to change as little as possible in order to avoid creating problems for other people. I did not and do not know who consumes UrlValidator. Given the recent explosion of TLDs it seemed (and that is only my personal impression) odd to validate TLDs, especially with a hard coded set of more than 1000 names. Also, the deprecated "non-routines" UrlValidator originally used in Maven tried to "validate" TLDs by restricting them to a maximum of 4 characters. That's what actually prevented .local from being used in Maven sites. Concerning domain validation in general I felt that depending on the use case a DNS lookup validating the fully qualified name or a purely syntactical analysis should be employed. So I decided that I do not understand the component and cannot fix it because I do not know if it is really broken or working as intended. However, given its behavior it was used in the wrong way in Maven. So I decided to fix the wrong end.

Do you know if there is a good reason for this kind of "static" TLD validation in DomainValidator?

Your suggested solution would change the behavior of isValid(String) or need a new flag to allow link-local (in addition to the UrlValidator.ALLOW_MDNS_HOSTNAME flag). What would you prefer?

Concerning Maven - I personally think that URL validation for project URLs is not necessary at all. If it is, it should be validated upon build time with a meaningful warning for the producer not the consumer.

[jira-importer]

Michael Osipov commented

Do you know if there is a good reason for this kind of "static" TLD validation in DomainValidator?

I think yes, because any type of TLD is validated with an isValid...Tld(String) method.

Your suggested solution would change the behavior of isValid(String) and need a new flag to allow link-local (in addition to the UrlValidator.ALLOW_MDNS_HOSTNAME flag). What would you prefer?

This should not be a default option, but a boolean flag to the #getInstance() method with boolean allowLinkLocal. #isValid() would work accordingly with the internal flag. Since UrlValidator uses DomainValidator, it think it should require ALLOW_LINK_LOCAL_URLS becaues mDNS is the technology behind and not the hostname style.

Apple provides good documentation about it as well as the RFC 6762, Section 3.

Concerning Maven - I personally think that URL validation for project URLs is not necessary at all. If it is, it should be validated upon build time with a meaningful warning for the producer not the consumer.

I fully agree here. I am inclined to remove it altogether because people could use SFTP, SSH, S3, FTPS, SMB and all of them are valid, but considered invalid by us -- which is wrong. There is no bulletproof way for us to tell wether a URL is correct or not. We could pass it at most to URI and see the result. Moreover, a href does not necessarily has to be a URL at all. I will create a new ticket for removing the validation as a whole.

[jira-importer]

Michael Osipov commented

Implicitly fixed with r1781702. Links are now passed as-is.