Skip to content

PHPC-742: Regex constructor should default flags arg to empty string #388

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

Merged
merged 1 commit into from
Sep 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 16 additions & 12 deletions src/BSON/Regex.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,20 @@ static bool php_phongo_regex_init(php_phongo_regex_t *intern, const char *patter
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Pattern cannot contain null bytes");
return false;
}

if (strlen(flags) != flags_len) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Flags cannot contain null bytes");
return false;
}

intern->pattern = estrndup(pattern, pattern_len);
intern->pattern_len = pattern_len;
intern->flags = estrndup(flags, flags_len);
intern->flags_len = flags_len;

if (flags) {
if (strlen(flags) != flags_len) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Flags cannot contain null bytes");
return false;
}
intern->flags = estrndup(flags, flags_len);
intern->flags_len = flags_len;
} else {
intern->flags = estrdup("");
intern->flags_len = 0;
}

return true;
}
Expand Down Expand Up @@ -92,22 +96,22 @@ static bool php_phongo_regex_init_from_hash(php_phongo_regex_t *intern, HashTabl
return false;
}

/* {{{ proto void Regex::__construct(string $pattern, string $flags)
/* {{{ proto void Regex::__construct(string $pattern [, string $flags])
Constructs a new BSON regular expression type. */
PHP_METHOD(Regex, __construct)
{
php_phongo_regex_t *intern;
zend_error_handling error_handling;
char *pattern;
phongo_zpp_char_len pattern_len;
char *flags;
phongo_zpp_char_len flags_len;
char *flags = NULL;
phongo_zpp_char_len flags_len = 0;


zend_replace_error_handling(EH_THROW, phongo_exception_from_phongo_domain(PHONGO_ERROR_INVALID_ARGUMENT), &error_handling TSRMLS_CC);
intern = Z_REGEX_OBJ_P(getThis());

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ss", &pattern, &pattern_len, &flags, &flags_len) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|s", &pattern, &pattern_len, &flags, &flags_len) == FAILURE) {
zend_restore_error_handling(&error_handling TSRMLS_CC);
return;
}
Expand Down
36 changes: 36 additions & 0 deletions tests/bson/bson-regex-003.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
--TEST--
MongoDB\BSON\Regex with flags omitted
--FILE--
<?php

require_once __DIR__ . '/../utils/tools.php';

$regexp = new MongoDB\BSON\Regex("regexp");
printf("Pattern: %s\n", $regexp->getPattern());
printf("Flags: %s\n", $regexp->getFlags());
printf("String representation: %s\n", $regexp);

$tests = array(
array("regex" => $regexp),
);

foreach($tests as $n => $test) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you have intention of adding additional test cases? If not, we can do away with this foreach.

Copy link
Contributor Author

@derickr derickr Sep 6, 2016

Choose a reason for hiding this comment

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

It's a copy of #001, which had the same thing. I didn't want to deviate (or change an existing test).

Copy link
Member

Choose a reason for hiding this comment

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

If you're merging this as-is, don't object if I go back and refactor this at some point. It's dead code that doesn't add any value to the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't mind!

$s = fromPHP($test);
echo "Test#{$n} ", $json = toJSON($s), "\n";
$bson = fromJSON($json);
$testagain = toPHP($bson);
var_dump(toJSON(fromPHP($test)), toJSON(fromPHP($testagain)));
var_dump((object)$test == (object)$testagain);
}
?>
===DONE===
<?php exit(0); ?>
--EXPECTF--
Pattern: regexp
Flags:
String representation: /regexp/
Test#0 { "regex" : { "$regex" : "regexp", "$options" : "" } }
string(54) "{ "regex" : { "$regex" : "regexp", "$options" : "" } }"
string(54) "{ "regex" : { "$regex" : "regexp", "$options" : "" } }"
bool(true)
===DONE===
20 changes: 20 additions & 0 deletions tests/bson/bson-regex-004.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
--TEST--
MongoDB\BSON\Regex debug handler with flags omitted
--FILE--
<?php

$regex = new MongoDB\BSON\Regex('regexp');

var_dump($regex);

?>
===DONE===
<?php exit(0); ?>
--EXPECTF--
object(MongoDB\BSON\Regex)#%d (%d) {
["pattern"]=>
string(6) "regexp"
["flags"]=>
string(0) ""
}
===DONE===
27 changes: 27 additions & 0 deletions tests/bson/bson-regex-serialization-002.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
MongoDB\BSON\Regex serialization with flags omitted
--FILE--
<?php

var_dump($regex = new MongoDB\BSON\Regex('regexp'));
var_dump($s = serialize($regex));
var_dump(unserialize($s));

?>
===DONE===
<?php exit(0); ?>
--EXPECTF--
object(MongoDB\BSON\Regex)#%d (%d) {
["pattern"]=>
string(6) "regexp"
["flags"]=>
string(0) ""
}
string(76) "O:18:"MongoDB\BSON\Regex":2:{s:7:"pattern";s:6:"regexp";s:5:"flags";s:0:"";}"
object(MongoDB\BSON\Regex)#%d (%d) {
["pattern"]=>
string(6) "regexp"
["flags"]=>
string(0) ""
}
===DONE===