Skip to content
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.

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

Validation Class - corresponding about the escaped separator. #1203

Merged
merged 1 commit into from
Sep 6, 2018

Conversation

ytetsuro
Copy link
Contributor

@ytetsuro ytetsuro commented Sep 6, 2018

When writing the following rules, this program works as follows.

$ this-> validate ([
    'date' => 'required|regex_match[/^[0-9]{4}[\-\.\/][0-9]{2}[\-\.\/][0-9]{2}/]|max_length[10]'
]);

validation rule is regex_match[/^[0-9]{4}[\-\.\/].

but, expected validation rule is regex_match[/^[0-9]{4}[\-\.\/][0-9]{2}[\-\.\/][0-9]{2}/].

Signed-off-by: ytetsuro [email protected]

detail: #1202 (comment)

@lonnieezell
Copy link
Member

I'll be honest - off the top of my head, I couldn't tell you how that particular preg_match regex works lol. All tests are passing, though, so I'll roll with it.

@lonnieezell lonnieezell merged commit 76e5559 into codeigniter4:develop Sep 6, 2018
@ytetsuro
Copy link
Contributor Author

ytetsuro commented Sep 6, 2018

Sorry...

How particular preg_match regex works.

find non escaped separator and ].
Matches whichever \ has an even number before the combination of separator and ].

example:

string mached
/] 🙆‍♂️
\/] 🙅‍♂️
\\/] 🙆‍♂️
\\\/] 🙅‍♂️

@lonnieezell
Copy link
Member

@ytetsuro I'm running into an issue when running migrations that seems to be caused by this. Unfortunately, the only error message it gives me is fairly generic:

preg_match(): Compilation failed: reference to non-existent subpattern at offset 20
/Users/kilishan/WebSites/CodeIgniter4/system/Validation/Validation.php - 723

Any ideas?

@ytetsuro
Copy link
Contributor Author

ytetsuro commented Sep 13, 2018

@lonnieezell

Oh... I'm very very very sorry....

It can be solved by changing it as follows.
The intention of this change is to escape anything except those that can not be used as a delimiter.

+				$separator = (preg_match('/[^a-z0-9\\\s]/i', $separator)) ? '\\'.$separator : $separator;
-				if (preg_match('/(?<!\\\\)(?:\\\\\\\\)*\\'.$separator.'\]/', $rules, $matches, PREG_OFFSET_CAPTURE))
+				if (preg_match('/(?<!\\\\)(?:\\\\\\\\)*'.$separator.'\]/', $rules, $matches, PREG_OFFSET_CAPTURE))

However, I noticed when writing this diff, but since the PHP regular expression delimiter has to support cases such as () and [], I think that the processing between these delimiters is not sufficient.

http://php.net/manual/en/regexp.reference.delimiters.php

Shall i create this diff PR?

@lonnieezell
Copy link
Member

@ytetsuro yes, please. A PR would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants