WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 2 years ago

#40353 assigned defect (bug)

Site URL and Home URL inputs are not properly validating

Reported by: subrataemfluence Owned by: loru88
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Options, Meta APIs Keywords: good-first-bug has-patch
Focuses: administration Cc:
PR Number:

Description

In wp-admin/options-general.php > General settings the URLs not properly validating. I tried the following with WordPress address (URL) input:

http://local.mysite. com
http://local.my?site.com
http://local.my*site.com

In all three cases WP saves the entry and then the page breaks! A proper handling is required.

Attachments (6)

40353.patch (1.1 KB) - added by thamaraiselvam 3 years ago.
This validates URLs more accurate
40353.2.patch (1.7 KB) - added by shulard 3 years ago.
Validate URL using parse_url and create a new is_wordpress_url function.
#40353.3.patch (5.4 KB) - added by loru88 3 years ago.
validate url using a simple filterable regex
40353.4.patch (9.8 KB) - added by loru88 3 years ago.
fixed previous patch and added tests
40353.3.patch (1.1 KB) - added by umangvaghela123 2 years ago.
Fix #40353 issue.
40353.5.patch (1.0 KB) - added by umangvaghela123 2 years ago.

Download all attachments as: .zip

Change History (23)

#1 @swissspidy
3 years ago

  • Component changed from Administration to Options, Meta APIs
  • Version 4.7.3 deleted

#2 @welcher
3 years ago

  • Keywords good-first-bug needs-patch added

#3 @thamaraiselvam
3 years ago

In general, URIs as defined by * RFC 3986 (see * Section 2: Characters) may contain any of the following characters:

ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~:/?#[]@!$&'()*+,;=`.

@subrataemfluence From your question all tested cases are valid.

references
http://stackoverflow.com/questions/1547899/which-characters-make-a-url-invalid

@thamaraiselvam
3 years ago

This validates URLs more accurate

#4 @rachelbaker
3 years ago

@thamaraiselvam PHP can be compiled without support for filter, so we cannot assume the filter_var() functions are available.

#5 @shulard
3 years ago

@rachelbaker you are right, but can't it be nice to use this feature when available ?

Maybe rely on parse_url can be a valid drawback ? If the url can be parsed, we can check if the scheme is http/https then accept it.

Last edited 3 years ago by shulard (previous) (diff)

@shulard
3 years ago

Validate URL using parse_url and create a new is_wordpress_url function.

@loru88
3 years ago

validate url using a simple filterable regex

#6 @loru88
3 years ago

@shulard as I know parse_url doesn't validate the url

The idea of a dedicated function is ok, but I upload a patch to validate the url against a simple regex.
It takes into consideration just http and https, a valid hostname or ip address, an optional port and an optional path.

a filter is applied to the regex in case someone needs different validation pattern.

I taked inspiration from Symfony UrlValidator but I simplified the regex:
https://github.com/symfony/validator/blob/master/Constraints/UrlValidator.php

Could be useful to add a unit test on this function.

#7 @shulard
3 years ago

You are right, parse_url doesn't validate the URL it analyze the different parts and extract them inside an array, it's why I validated array keys and values.

The regex used seems very complicated due to IPv6 support but it's more accurate regarding validation. I'll take a look to add some unit tests on your function :)

@loru88
3 years ago

fixed previous patch and added tests

@umangvaghela123
2 years ago

Fix #40353 issue.

#8 @umangvaghela123
2 years ago

Test with the previous patch and find an easy and sufficient solution.

#9 @umangvaghela123
2 years ago

  • Keywords has-patch added; needs-patch removed

#10 follow-up: @swissspidy
2 years ago

@umangvaghela123 As mentioned earlier in the ticket, filter_var() can't be used because it could be turned off in PHP 5.2.

40353.4.patch looks a bit messy because it removes existing unit tests and the regex in is_valid_wordpress_url() is way too complex for this IMHO.

This ticket was mentioned in Slack in #core by umangvaghela. View the logs.


2 years ago

#12 @DrewAPicture
2 years ago

  • Owner set to loru88
  • Status changed from new to assigned

Assigning ownership to mark the good-first-bug as "claimed".

#13 in reply to: ↑ 10 @loru88
2 years ago

Replying to swissspidy:

@umangvaghela123 As mentioned earlier in the ticket, filter_var() can't be used because it could be turned off in PHP 5.2.

40353.4.patch looks a bit messy because it removes existing unit tests and the regex in is_valid_wordpress_url() is way too complex for this IMHO.

It does not remove any existing unit test, I just take the email validation test as a template to start but I probably mess with git. I'll fix it as soon as I can.

The function "is_valid_wordpress_url" is complex because I think it could be useful for plugin and theme authors, just like any other data validation function in wordpress core.

#14 follow-up: @umangvaghela123
2 years ago

@loru88

Its is fine if we use esc_url() for validate site_url and home_url.

#15 in reply to: ↑ 14 @loru88
2 years ago

I don't think so, how would you use it?
Once I tried this

<?php

if( esc_url($url) == $url ){
  valid url
}

but it doesn't check if it is a syntactically valid URL.

and as @rachelbaker said here, we cannot user filter function

Replying to umangvaghela123:

@loru88

Its is fine if we use esc_url() for validate site_url and home_url.

Last edited 2 years ago by loru88 (previous) (diff)

#16 @umangvaghela123
2 years ago

@loru88 ,@swissspidy,@DrewAPicture  If we check with wp_http_validate_url() so we can solve issue.To use this function we are not face php varsion issue.

Last edited 2 years ago by umangvaghela123 (previous) (diff)
Note: See TracTickets for help on using tickets.