fix: make "file://" fail url validation

[bfabio]

"file://" is not a valid URL since it has no path.

Fixes Or Enhances

Makes "file://" fail the url validation instead of passing it.

This is akin to "http://" already failing as expected in tests.

Make sure that you've checked the boxes below before you submit PR:

• Tests exist or have been written that cover this particular change.

@go-playground/validator-maintainers

[coveralls]

coverage: 73.677% (-0.009%) from 73.686%
when pulling 66619d5 on bfabio:url_file
into 3fd4678 on go-playground:master.

[deankarn]

Thanks for the PR, will try and take a look this weekend.

I think there might be some other issue if file URI paths are passing the URL validation at all.

URL is a subset of URI which are specifically for resources hosted online, whereas file paths are local and are URI and not URLs.

[bfabio]

Hi @deankarn, AFAIK they are also URLs: https://url.spec.whatwg.org/#scheme-relative-file-url-string

TIL that they can have a host as in file://localhost/etc/passwd and file:///etc/passwd is just /etc/passwd with an empty host.

[deankarn]

Why not let it fall through to url.Parse()? This should achieve the same no?

[bfabio]

That's because for file:// we care only about u.Path, whereas the code below doesn't check for it and we'd end up hitting the last return (true)

[deankarn]

I’ll have to look again because it should fail the host being blank and return false

[bfabio]

The Host can be there even in file://, such as file://localhost/path/to/file.txt. We have a test case for that.

[deankarn]

part of the file checks still fall through to the url parsing logic which feels strange the logic being split, WDYT about consolidating like this and only having url.Parse?

// isURL is the validation function for validating if the current field's value is a valid URL.
func isURL(fl FieldLevel) bool {
	field := fl.Field()

	//var err error
	switch field.Kind() {
	case reflect.String:

		s := strings.ToLower(field.String())

		if len(s) == 0 {
			return false
		}

		url, err := url.Parse(s)
		if err != nil || url.Scheme == "" {
			return false
		}

		isFileScheme := url.Scheme == "file"

		if (isFileScheme && (len(url.Path) == 0 || url.Path == "/")) || (!isFileScheme && len(url.Host) == 0 && len(url.Fragment) == 0 && len(url.Opaque) == 0) {
			return false
		}

		return true
	}

	panic(fmt.Sprintf("Bad field type %s", field.Type()))
}

[bfabio]

That looks cleaner. Why not url.Path == "" though (and, in general, using len() instead of comparing to the empty string)?

[deankarn]

generally number equality checks should be faster that string equality is the only reason .. there may be some optimizations under the hood that make it close to the same but still number should be faster.

[bfabio]

@deankarn TIL! Thanks