Add test for validation for regex flags #41
Conversation
src/DB_Command.php
Outdated
| @@ -820,6 +820,10 @@ public function search( $args, $assoc_args ) { | |||
|
|
|||
| if ( ( $regex = \WP_CLI\Utils\get_flag_value( $assoc_args, 'regex', false ) ) ) { | |||
| $regex_flags = \WP_CLI\Utils\get_flag_value( $assoc_args, 'regex-flags', false ); | |||
| // http://php.net/manual/en/reference.pcre.pattern.modifiers.php | |||
| if ( $regex_flags && ! preg_match( '/^(?!.*(.).*\1)[imsxeADSUXJu]+$/', $regex_flags ) ) { | |||
There was a problem hiding this comment.
Sorry, not a fan of this check, either here or in search-replace. The check below if ( false === @preg_match( $search_regex, '' ) ) { at https://github.com/wp-cli/db-command/blob/master/src/DB_Command.php#L845 traps bad modifiers. Also this check doesn't allow for spaces and newlines which are allowed (surprisingly). Also it doesn't allow for PHP version differences. Also PHP doesn't care about duplicates (and spaces and newlines can be duplicated anyway).
There was a problem hiding this comment.
Oh it looks true ..., but if we pass the incorrect regex-flags, the table will be broken.
$ wp search-replace '(Hello)\s(world)' '$2, $1' --regex --regex-flags='kppr'
PHP Warning: preg_replace(): Unknown modifier 'k' in phar:///usr/local/bin/wp/vendor/wp-cli/search-replace-command/src/WP_CLI/SearchReplacer.php on line 86
Warning: preg_replace(): Unknown modifier 'k' in phar:///usr/local/bin/wp/vendor/wp-cli/search-replace-command/src/WP_CLI/SearchReplacer.php on line 86
...
...
$ wp plugin list
Error: One or more database tables are unavailable. The database may need to be repaired.
I am thinking we need a validation for --regex-flags definitely. Do you have any idea?
There was a problem hiding this comment.
I think using the same check in search-replace as at https://github.com/wp-cli/db-command/blob/master/src/DB_Command.php#L845 should be good enough.
features/db-search.feature
Outdated
| @@ -701,6 +701,9 @@ Feature: Search through the database | |||
| When I run `wp db search 'unfindable' --regex` | |||
| Then STDOUT should be empty | |||
|
|
|||
| When I try `wp db search 'unfindable' --regex --regex-flags='abcd'` | |||
| Then the return code should be 1 | |||
There was a problem hiding this comment.
A And STDERR should be: step would be good.
gitlost
changed the title
|
Ah, I have understood what you are saying. It is working as expected, thanks!
And I will update search-replace too. 😄 |
|
Sorry, should have waited to merge. (Was too eager to get my fix for #43 in!) |
Add test for validation for regex flags
Related: wp-cli/search-replace-command#28