file:/// URI validation seems incorrect?

[alcohol]

Currently, the URI validator does not support file:///path for a local path. It does seem to support file://path; but this is technically wrong (though a common mistake): https://en.wikipedia.org/wiki/File_URI_scheme

If host is omitted, it is taken to be "localhost", the machine from which the URL is being interpreted. Note that when omitting host, the slash is not omitted (while "file:///piro.txt" is valid, "file://simpen.txt" is not, although some interpreters manage to handle the latter).

See also https://en.wikipedia.org/wiki/File_URI_scheme#Number_of_slash_characters

Since this is a validation library and not just an interpreter, would it not be preferable to enforce the specification?

[DannyvdSluijs]

Hi @alcohol

it seems like a legit case. I’ve been working on the validations based on the JSON schema test cases. So it would be very likely that there was no such test case and that is how this could have been overlooked.

If you could make a PR I’m available to help with the review and merging/releasing.

[alcohol]

Will do; though with all the festivities right now it might be a few days before I will have some spare time available.

[alcohol]

So I added the correct/incorrect formats to the dataprovider; but I am not sure how to proceed now. While the format/specification is rather clear, I think it will be quite difficult to actually validate it because it is more about assumptions than actual wrong or right values. In particular, the file://path/file.txt case -- where path should be interpreted as a hostname but we cannot know if that is actually the case here or not.

See alcohol@312de34 for context

[alcohol]

So I found https://www.rfc-editor.org/rfc/rfc3986#appendix-B which seems a lot simpler than the current regular expression in use. Alternatively, one could consider breaking down the BNF into a regular expression (I did this for composer/spdx-licenses for example), but looking at https://www.rfc-editor.org/rfc/rfc3986#appendix-A this one is going to be quite complex in that regard..

[DannyvdSluijs]

@alcohol im trying to see how to help you out but it is a bit unclear to me. Is it you’re questioning the root cause, the change that are needed or if we should improve this at all?

I think the best way forward is to see if we can make the tests green. Which might be a tweak to the current regex or a replacement all together. I believe the current regex came into place to pass the JSON schema tests.

[alcohol]

Mostly the change that is needed. It should be improved because, in the current state of the validator, actual valid content is considered invalid.

However, as I mentioned, the proposed regular expression from the RFC is much simpler than the one in use. There are a lot of additional checks done in the isValid method also that are not necessarily part of the specification. For example, newsgroup name or email address validation.

To preserve that functionality, I think we merely need to focus on the part that isn't working properly right now, which is the file:///path/file.txt and file:/path/file.txt input.

[alcohol]

The smallest change I could imagine unfortunately breaks another test. See the latest commit in the PR. Any thoughts as to whether this is acceptable or not?

[alcohol]

This change made me realize that while maybe never used before, it is now also possible to validate a data:/ URI because those also only use one /: https://en.wikipedia.org/wiki/Data_URI_scheme

[alcohol]

Or wait, those are covered by the non hierarchical branch. I forgot for a moment that we are dealing with more than one regex here. I'm going to see if I need to revise things.

[alcohol]

Okay so maybe treating file as non-hierarchical is the easiest/simplest solution. Not sure if that is technically correct, but it works 🤷

[alcohol]

Sorry for the numerous replies. It was a bit like rubber 🦆ing. I think my PR contains the most sensible approach to fixing this now.