CodeIgniter4: valid_url does not work correctly

Hello!

Url validation doesn’t work, if I pass a string like this asdf it passes the validation which should be wrong

my validation is as follows:

'rules' => 'permit_empty|valid_url|max_length[512]'

Hugs!

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 15 (14 by maintainers)

Commits related to this issue

Most upvoted comments

I’m reopening this issue because I think there are some things that need to be addressed.

  1. RFC 2396 is obsolete. The new doc is RFC 3986
  2. The spec differentiates between “URI” (Identifier) and “URL” (Location). A URL:

The term “Uniform Resource Locator” (URL) refers to the subset of URIs that, in addition to identifying a resource, provide a means of locating the resource by describing its primary access mechanism (e.g., its network “location”).

  1. The section “Syntax Components” describes the parts of the URI. Of note, it requires a scheme (e.g. “https”) but not a path:

The scheme and path components are required, though the path may be empty (no characters)

  1. Our current validation rule very clearly allows the opposite in order to validate “common” URLs like “CodeIgniter.com”. See also the validation rules tests for some wonky passes: https://github.com/codeigniter4/CodeIgniter4/blob/d9864e33758aca9aca9dc44c121e402e2d51136f/tests/system/Validation/FormatRulesTest.php#L88

Here’s my opinion:

a) We need valid_url_strict or something that has a lower tolerance for actual URLs. b) I think the current validator should not pass simple strings like “asdf”, but in order to prevent a breaking change we should still accept “CodeIgniter.com”. Probably could check for a scheme and pass to a) if found, use a simple regex otherwise.

Just to note, Laravel and Symfony use a super convoluted regex to check validity (https://github.com/laravel/framework/blob/cb398188729f51e63d6fd0dc1ed9009fcf76b404/src/Illuminate/Validation/Concerns/ValidatesAttributes.php#L1732).

In my opinion. valid_url should only allow URL to pass inspection. Such asdf , localhost, codeigniter.com string, it not a URL. It 's domain.

I think I would only mention that if the protocol part in the address is omitted it will be added automatically before doing a check. This is just more accurate and nobody will have doubts about how things are working.

About URI::validate() - it’s a good idea. If I remember it correctly we’re using parse_url() in URI class.

On seriously malformed URLs, parse_url() may return FALSE.

I wonder how many false-positive results we can get.

Not a bug, sorry - asdf is a valid hostname.

Please take a look at RFC 2396 - these are the rules behind FILTER_VALIDATE_URL.

But we’re open to adding a new rule that will be more “restrictive”.