Skip to content

New FILTER_VALIDATE_DOMAIN and better RFC conformance for FILTER_VALIDATE_URL #826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 27, 2014
Merged

New FILTER_VALIDATE_DOMAIN and better RFC conformance for FILTER_VALIDATE_URL #826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
728945d
FILTER_VALIDATE_DOMAIN and RFC conformance for FILTER_VALIDATE_URL
dunglas Sep 16, 2014
5ad3f87
Fix filter_list test
dunglas Nov 14, 2014
a8742d3
Fix ext/filter/tests/033.phpt
dunglas Nov 14, 2014
54f06d2
New label length test
dunglas Nov 17, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ext/filter/filter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ static const filter_list_entry filter_list[] = {
{ "float", FILTER_VALIDATE_FLOAT, php_filter_float },

{ "validate_regexp", FILTER_VALIDATE_REGEXP, php_filter_validate_regexp },
{ "validate_domain", FILTER_VALIDATE_DOMAIN, php_filter_validate_domain },
{ "validate_url", FILTER_VALIDATE_URL, php_filter_validate_url },
{ "validate_email", FILTER_VALIDATE_EMAIL, php_filter_validate_email },
{ "validate_ip", FILTER_VALIDATE_IP, php_filter_validate_ip },
Expand Down Expand Up @@ -231,6 +232,7 @@ PHP_MINIT_FUNCTION(filter)
REGISTER_LONG_CONSTANT("FILTER_VALIDATE_FLOAT", FILTER_VALIDATE_FLOAT, CONST_CS | CONST_PERSISTENT);

REGISTER_LONG_CONSTANT("FILTER_VALIDATE_REGEXP", FILTER_VALIDATE_REGEXP, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("FILTER_VALIDATE_DOMAIN", FILTER_VALIDATE_DOMAIN, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("FILTER_VALIDATE_URL", FILTER_VALIDATE_URL, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("FILTER_VALIDATE_EMAIL", FILTER_VALIDATE_EMAIL, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("FILTER_VALIDATE_IP", FILTER_VALIDATE_IP, CONST_CS | CONST_PERSISTENT);
Expand Down Expand Up @@ -278,6 +280,8 @@ PHP_MINIT_FUNCTION(filter)
REGISTER_LONG_CONSTANT("FILTER_FLAG_NO_RES_RANGE", FILTER_FLAG_NO_RES_RANGE, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("FILTER_FLAG_NO_PRIV_RANGE", FILTER_FLAG_NO_PRIV_RANGE, CONST_CS | CONST_PERSISTENT);

REGISTER_LONG_CONSTANT("FILTER_FLAG_HOSTNAME", FILTER_FLAG_HOSTNAME, CONST_CS | CONST_PERSISTENT);

sapi_register_input_filter(php_sapi_filter, php_sapi_filter_init TSRMLS_CC);

return SUCCESS;
Expand Down
5 changes: 4 additions & 1 deletion ext/filter/filter_private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
#define FILTER_FLAG_NO_RES_RANGE 0x400000
#define FILTER_FLAG_NO_PRIV_RANGE 0x800000

#define FILTER_FLAG_HOSTNAME 0x100000

#define FILTER_VALIDATE_INT 0x0101
#define FILTER_VALIDATE_BOOLEAN 0x0102
#define FILTER_VALIDATE_FLOAT 0x0103
Expand All @@ -64,7 +66,8 @@
#define FILTER_VALIDATE_EMAIL 0x0112
#define FILTER_VALIDATE_IP 0x0113
#define FILTER_VALIDATE_MAC 0x0114
#define FILTER_VALIDATE_LAST 0x0114
#define FILTER_VALIDATE_DOMAIN 0x0115
#define FILTER_VALIDATE_LAST 0x0115

#define FILTER_VALIDATE_ALL 0x0100

Expand Down
87 changes: 76 additions & 11 deletions ext/filter/logical_filters.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
+----------------------------------------------------------------------+
| Authors: Derick Rethans <derick@php.net> |
| Pierre-A. Joye <pierre@php.net> |
| Kévin Dunglas <dunglas@gmail.com> |
+----------------------------------------------------------------------+
*/

Expand Down Expand Up @@ -80,6 +81,8 @@
#define FORMAT_IPV4 4
#define FORMAT_IPV6 6

static int _php_filter_validate_ipv6(char *str, size_t str_len TSRMLS_DC);

static int php_filter_parse_int(const char *str, size_t str_len, zend_long *ret TSRMLS_DC) { /* {{{ */
zend_long ctx_value;
int sign = 0, digit = 0;
Expand Down Expand Up @@ -452,6 +455,65 @@ void php_filter_validate_regexp(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
RETURN_VALIDATION_FAILED
}
}

static int _php_filter_validate_domain(char * domain, int len, zend_long flags) /* {{{ */
{
char *e, *s, *t;
size_t l;
int hostname = flags & FILTER_FLAG_HOSTNAME;
unsigned char i = 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why start the label length at 1 if you haven't read any characters from the string yet?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the length incrementation occurs after the length check (see https://github.com/dunglas/php-src/blob/validate_url_length/ext/filter/logical_filters.c#L497-L501).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's exactly my point, your invariant choice for i is just unusual; it would be more logical to define it as the number of characters since the last period ... I know, nitpicking :P

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@datibbaw I'm ok to initialize i as 0 and move the increment operation before the check (it's your point right?) if it's already done like that in other part of the code base. Anyway, doing it like I've done avoids one unnecessary increment operation when the label is > 63 characters.

Number of ++ operations when label is 64 chars:
current implementation: 63
your suggestion: 64

It's really micro-optimization with no impact in real uses case but... :-)


s = domain;
l = len;
e = domain + l;
t = e - 1;

/* Ignore trailing dot */
if (*t == '.') {
e = t;
l--;
}

/* The total length cannot exceed 253 characters (final dot not included) */
if (l > 253) {
return 0;
}

/* First char must be alphanumeric */
if(*s == '.' || (hostname && !isalnum((int)*(unsigned char *)s))) {
return 0;
}

while (s < e) {
if (*s == '.') {
/* The first and the last character of a label must be alphanumeric */
if (*(s + 1) == '.' || (hostname && (!isalnum((int)*(unsigned char *)(s - 1)) || !isalnum((int)*(unsigned char *)(s + 1))))) {
return 0;
}

/* Reset label length counter */
i = 1;
} else {
if (i > 63 || (hostname && *s != '-' && !isalnum((int)*(unsigned char *)s))) {
return 0;
}

i++;
}

s++;
}

return 1;
}
/* }}} */

void php_filter_validate_domain(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
{
if (!_php_filter_validate_domain(Z_STRVAL_P(value), Z_STRLEN_P(value), flags)) {
RETURN_VALIDATION_FAILED
}
}
/* }}} */

void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
Expand All @@ -473,25 +535,28 @@ void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
}

if (url->scheme != NULL && (!strcasecmp(url->scheme, "http") || !strcasecmp(url->scheme, "https"))) {
char *e, *s;
char *e, *s, *t;
size_t l;

if (url->host == NULL) {
goto bad_url;
}

e = url->host + strlen(url->host);
s = url->host;

/* First char of hostname must be alphanumeric */
if(!isalnum((int)*(unsigned char *)s)) {
goto bad_url;
l = strlen(s);
e = url->host + l;
t = e - 1;

/* An IPv6 enclosed by square brackets is a valid hostname */
if (*s == '[' && *t == ']' && _php_filter_validate_ipv6((s + 1), l - 2 TSRMLS_CC)) {
php_url_free(url);
return;
}

while (s < e) {
if (!isalnum((int)*(unsigned char *)s) && *s != '-' && *s != '.') {
goto bad_url;
}
s++;
// Validate domain
if (!_php_filter_validate_domain(url->host, l, FILTER_FLAG_HOSTNAME)) {
php_url_free(url);
RETURN_VALIDATION_FAILED
}
}

Expand Down
1 change: 1 addition & 0 deletions ext/filter/php_filter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ void php_filter_int(PHP_INPUT_FILTER_PARAM_DECL);
void php_filter_boolean(PHP_INPUT_FILTER_PARAM_DECL);
void php_filter_float(PHP_INPUT_FILTER_PARAM_DECL);
void php_filter_validate_regexp(PHP_INPUT_FILTER_PARAM_DECL);
void php_filter_validate_domain(PHP_INPUT_FILTER_PARAM_DECL);
void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL);
void php_filter_validate_email(PHP_INPUT_FILTER_PARAM_DECL);
void php_filter_validate_ip(PHP_INPUT_FILTER_PARAM_DECL);
Expand Down
34 changes: 18 additions & 16 deletions ext/filter/tests/008.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ var_dump(filter_list(array()));
echo "Done\n";
?>
--EXPECTF--
array(20) {
array(21) {
[0]=>
string(3) "int"
[1]=>
Expand All @@ -21,36 +21,38 @@ array(20) {
[3]=>
string(15) "validate_regexp"
[4]=>
string(12) "validate_url"
string(15) "validate_domain"
[5]=>
string(14) "validate_email"
string(12) "validate_url"
[6]=>
string(11) "validate_ip"
string(14) "validate_email"
[7]=>
string(12) "validate_mac"
string(11) "validate_ip"
[8]=>
string(6) "string"
string(12) "validate_mac"
[9]=>
string(8) "stripped"
string(6) "string"
[10]=>
string(7) "encoded"
string(8) "stripped"
[11]=>
string(13) "special_chars"
string(7) "encoded"
[12]=>
string(18) "full_special_chars"
string(13) "special_chars"
[13]=>
string(10) "unsafe_raw"
string(18) "full_special_chars"
[14]=>
string(5) "email"
string(10) "unsafe_raw"
[15]=>
string(3) "url"
string(5) "email"
[16]=>
string(10) "number_int"
string(3) "url"
[17]=>
string(12) "number_float"
string(10) "number_int"
[18]=>
string(12) "magic_quotes"
string(12) "number_float"
[19]=>
string(12) "magic_quotes"
[20]=>
string(8) "callback"
}

Expand Down
32 changes: 32 additions & 0 deletions ext/filter/tests/015.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,22 @@ $values = Array(
'http://www.example/img/test.png',
'http://www.example/img/dir/',
'http://www.example/img/dir',
'http://www.thelongestdomainnameintheworldandthensomeandthensomemoreandmore.com/',
'http://toolongtoolongtoolongtoolongtoolongtoolongtoolongtoolongtoolongtoolong.com',
'http://eauBcFReEmjLcoZwI0RuONNnwU4H9r151juCaqTI5VeIP5jcYIqhx1lh5vV00l2rTs6y7hOp7rYw42QZiq6VIzjcYrRm8gFRMk9U9Wi1grL8Mr5kLVloYLthHgyA94QK3SaXCATklxgo6XvcbXIqAGG7U0KxTr8hJJU1p2ZQ2mXHmp4DhYP8N9SRuEKzaCPcSIcW7uj21jZqBigsLsNAXEzU8SPXZjmVQVtwQATPWeWyGW4GuJhjP4Q8o0.com',
'http://kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58.kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58.kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58.CQ1oT5Uq3jJt6Uhy3VH9u3Gi5YhfZCvZVKgLlaXNFhVKB1zJxvunR7SJa.com.',
'http://kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58R.example.com',
'http://[2001:0db8:0000:85a3:0000:0000:ac1f:8001]',
'http://[2001:db8:0:85a3:0:0:ac1f:8001]:123/me.html',
'http://[2001:db8:0:85a3::ac1f:8001]/',
'http://[::1]',
'http://cont-ains.h-yph-en-s.com',
'http://..com',
'http://a.-bc.com',
'http://ab.cd-.com',
'http://-.abc.com',
'http://abc.-.abc.com',
'http://underscore_.example.com',
'http//www.example/wrong/url/',
'http:/www.example',
'file:///tmp/test.c',
Expand Down Expand Up @@ -56,6 +72,22 @@ string(32) "http://www.example.com/index.php"
string(31) "http://www.example/img/test.png"
string(27) "http://www.example/img/dir/"
string(26) "http://www.example/img/dir"
string(79) "http://www.thelongestdomainnameintheworldandthensomeandthensomemoreandmore.com/"
bool(false)
bool(false)
string(261) "http://kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58.kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58.kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58.CQ1oT5Uq3jJt6Uhy3VH9u3Gi5YhfZCvZVKgLlaXNFhVKB1zJxvunR7SJa.com."
bool(false)
string(48) "http://[2001:0db8:0000:85a3:0000:0000:ac1f:8001]"
string(50) "http://[2001:db8:0:85a3:0:0:ac1f:8001]:123/me.html"
string(36) "http://[2001:db8:0:85a3::ac1f:8001]/"
string(12) "http://[::1]"
string(31) "http://cont-ains.h-yph-en-s.com"
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
string(18) "file:///tmp/test.c"
Expand Down
3 changes: 2 additions & 1 deletion ext/filter/tests/033.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ int 1 123
boolean 1
float 1 123
validate_regexp O'Henry
validate_domain PHP 1 foo@bar.com http://a.b.c 1.2.3.4 123 123abc<>() O'Henry 하퍼 aa:bb:cc:dd:ee:ff
validate_url http://a.b.c
validate_email foo@bar.com
validate_ip 1.2.3.4
Expand All @@ -29,4 +30,4 @@ url PHP 1 foo@bar.com http://a.b.c 1.2.3.4 123 12
number_int 1 1234 123 123
number_float 1 1234 123 123
magic_quotes PHP 1 foo@bar.com http://a.b.c 1.2.3.4 123 123abc<>() O\'Henry 하퍼 aa:bb:cc:dd:ee:ff
callback PHP 1 FOO@BAR.COM HTTP://A.B.C 1.2.3.4 123 123ABC<>() O'HENRY 하퍼 AA:BB:CC:DD:EE:FF
callback PHP 1 FOO@BAR.COM HTTP://A.B.C 1.2.3.4 123 123ABC<>() O'HENRY 하퍼 AA:BB:CC:DD:EE:FF
68 changes: 68 additions & 0 deletions ext/filter/tests/056.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
--TEST--
filter_var() and FILTER_VALIDATE_DOMAIN
--SKIPIF--
<?php if (!extension_loaded("filter")) die("skip"); ?>
--FILE--
<?php

$values = Array(
'example.com',
'www.thelongestdomainnameintheworldandthensomeandthensomemoreandmore.com',
'toolongtoolongtoolongtoolongtoolongtoolongtoolongtoolongtoolongtoolong.com',
'eauBcFReEmjLcoZwI0RuONNnwU4H9r151juCaqTI5VeIP5jcYIqhx1lh5vV00l2rTs6y7hOp7rYw42QZiq6VIzjcYrRm8gFRMk9U9Wi1grL8Mr5kLVloYLthHgyA94QK3SaXCATklxgo6XvcbXIqAGG7U0KxTr8hJJU1p2ZQ2mXHmp4DhYP8N9SRuEKzaCPcSIcW7uj21jZqBigsLsNAXEzU8SPXZjmVQVtwQATPWeWyGW4GuJhjP4Q8o0.com',
'kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58.kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58.kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58.CQ1oT5Uq3jJt6Uhy3VH9u3Gi5YhfZCvZVKgLlaXNFhVKB1zJxvunR7SJa.com.',
'cont-ains.h-yph-en-s.com',
'..com',
'ab..cc.dd',
'a.-bc.com',
'ab.cd-.com',
'-.abc.com',
'abc.-.abc.com',
'underscore_.example.com',
'',
-1,
array(),
'\r\n',
);
foreach ($values as $value) {
var_dump(filter_var($value, FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME));
}

var_dump(filter_var('_example.com', FILTER_VALIDATE_DOMAIN));
var_dump(filter_var('_example.com', FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME));
var_dump(filter_var('test_.example.com', FILTER_VALIDATE_DOMAIN));
var_dump(filter_var('test_.example.com', FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME));
var_dump(filter_var('te_st.example.com', FILTER_VALIDATE_DOMAIN));
var_dump(filter_var('te_st.example.com', FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME));
var_dump(filter_var('test._example.com', FILTER_VALIDATE_DOMAIN));
var_dump(filter_var('test._example.com', FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME));

echo "Done\n";
?>
--EXPECT--
string(11) "example.com"
string(71) "www.thelongestdomainnameintheworldandthensomeandthensomemoreandmore.com"
bool(false)
bool(false)
string(254) "kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58.kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58.kDTvHt1PPDgX5EiP2MwiXjcoWNOhhTuOVAUWJ3TmpBYCC9QoJV114LMYrV3Zl58.CQ1oT5Uq3jJt6Uhy3VH9u3Gi5YhfZCvZVKgLlaXNFhVKB1zJxvunR7SJa.com."
string(24) "cont-ains.h-yph-en-s.com"
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
string(12) "_example.com"
bool(false)
string(17) "test_.example.com"
bool(false)
string(17) "te_st.example.com"
bool(false)
string(17) "test._example.com"
bool(false)
Done