Skip to content

PHPC-475: toJSON and fromJSON should throw on error #130

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 2 commits into from
Oct 30, 2015
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
10 changes: 8 additions & 2 deletions src/bson.c
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,7 @@ PHP_FUNCTION(toJSON)
char *data;
int data_len;
const bson_t *b;
bool eof = false;
bson_reader_t *reader;

(void)return_value_ptr; (void)this_ptr; (void)return_value_used; /* We don't use these */
Expand All @@ -1114,8 +1115,13 @@ PHP_FUNCTION(toJSON)
RETVAL_STRINGL(str, str_len, 1);
bson_free(str);
} else {
RETURN_NULL();
phongo_throw_exception(PHONGO_ERROR_UNEXPECTED_VALUE TSRMLS_CC, "Could not read document from BSON reader");
}

if (bson_reader_read(reader, &eof) || !eof) {
phongo_throw_exception(PHONGO_ERROR_UNEXPECTED_VALUE TSRMLS_CC, "Reading document did not exhaust input buffer");
Copy link
Contributor

Choose a reason for hiding this comment

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

This will yield in two exceptions being thrown because the former has no "return"

}

bson_reader_destroy(reader);
}
/* }}} */
Expand All @@ -1139,7 +1145,7 @@ PHP_FUNCTION(fromJSON)
RETVAL_STRINGL((const char *) bson_get_data(&b), b.len, 1);
bson_destroy(&b);
} else {
RETURN_NULL();
phongo_throw_exception(PHONGO_ERROR_UNEXPECTED_VALUE TSRMLS_CC, "%s", error.message ? error.message : "Error parsing JSON");
}
}
/* }}} */
Expand Down
39 changes: 39 additions & 0 deletions tests/bson/bson-fromJSON-001.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
--TEST--
BSON\fromJSON(): Decoding JSON
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"?>
--FILE--
<?php

require_once __DIR__ . "/../utils/basic.inc";

$tests = [
'{}',
'{ "foo": "bar" }',
'{ "foo": [ 1, 2, 3 ]}',
'{ "foo": { "bar": 1 }}',
];

foreach ($tests as $json) {
printf("Test %s\n", $json);
$bson = fromJSON($json);
hex_dump($bson);
}

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
Test {}
0 : 05 00 00 00 00 [.....]
Test { "foo": "bar" }
0 : 12 00 00 00 02 66 6f 6f 00 04 00 00 00 62 61 72 [.....foo.....bar]
10 : 00 00 [..]
Test { "foo": [ 1, 2, 3 ]}
0 : 24 00 00 00 04 66 6f 6f 00 1a 00 00 00 10 30 00 [$....foo......0.]
10 : 01 00 00 00 10 31 00 02 00 00 00 10 32 00 03 00 [.....1......2...]
20 : 00 00 00 00 [....]
Test { "foo": { "bar": 1 }}
0 : 18 00 00 00 03 66 6f 6f 00 0e 00 00 00 10 62 61 [.....foo......ba]
10 : 72 00 01 00 00 00 00 00 [r.......]
===DONE===
56 changes: 56 additions & 0 deletions tests/bson/bson-fromJSON-002.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
--TEST--
BSON\fromJSON(): Decoding extended JSON types
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"?>
--FILE--
<?php

require_once __DIR__ . "/../utils/basic.inc";

$tests = [
'{ "_id": { "$oid": "56315a7c6118fd1b920270b1" }}',
'{ "binary": { "$binary": "Zm9v", "$type": "00" }}',
'{ "date": { "$date": "2015-10-28T00:00:00Z" }}',
'{ "timestamp": { "$timestamp": { "t": 1446084619, "i": 0 }}}',
'{ "regex": { "$regex": "pattern", "$options": "i" }}',
'{ "undef": { "$undefined": true }}',
'{ "minkey": { "$minKey": 1 }}',
'{ "maxkey": { "$maxKey": 1 }}',
'{ "long": { "$numberLong": "1234" }}',
];

foreach ($tests as $json) {
printf("Test %s\n", $json);
$bson = fromJSON($json);
hex_dump($bson);
}

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
Test { "_id": { "$oid": "56315a7c6118fd1b920270b1" }}
0 : 16 00 00 00 07 5f 69 64 00 56 31 5a 7c 61 18 fd [....._id.V1Z|a..]
10 : 1b 92 02 70 b1 00 [...p..]
Test { "binary": { "$binary": "Zm9v", "$type": "00" }}
0 : 15 00 00 00 05 62 69 6e 61 72 79 00 03 00 00 00 [.....binary.....]
10 : 00 66 6f 6f 00 [.foo.]
Test { "date": { "$date": "2015-10-28T00:00:00Z" }}
0 : 13 00 00 00 09 64 61 74 65 00 00 80 be ab 50 01 [.....date.....P.]
10 : 00 00 00 [...]
Test { "timestamp": { "$timestamp": { "t": 1446084619, "i": 0 }}}
0 : 18 00 00 00 11 74 69 6d 65 73 74 61 6d 70 00 00 [.....timestamp..]
10 : 00 00 00 0b 80 31 56 00 [.....1V.]
Test { "regex": { "$regex": "pattern", "$options": "i" }}
0 : 16 00 00 00 0b 72 65 67 65 78 00 70 61 74 74 65 [.....regex.patte]
10 : 72 6e 00 69 00 00 [rn.i..]
Test { "undef": { "$undefined": true }}
0 : 0c 00 00 00 06 75 6e 64 65 66 00 00 [.....undef..]
Test { "minkey": { "$minKey": 1 }}
0 : 0d 00 00 00 ff 6d 69 6e 6b 65 79 00 00 [.....minkey..]
Test { "maxkey": { "$maxKey": 1 }}
0 : 0d 00 00 00 7f 6d 61 78 6b 65 79 00 00 [.....maxkey..]
Test { "long": { "$numberLong": "1234" }}
0 : 13 00 00 00 12 6c 6f 6e 67 00 d2 04 00 00 00 00 [.....long.......]
10 : 00 00 00 [...]
===DONE===
23 changes: 23 additions & 0 deletions tests/bson/bson-fromJSON_error-001.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
--TEST--
BSON\fromJSON(): invalid JSON
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"?>
--FILE--
<?php

require_once __DIR__ . "/../utils/basic.inc";

echo throws(function() {
fromJSON('foo');
}, 'MongoDB\Driver\Exception\UnexpectedValueException'), "\n";

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
lexical error: invalid string in json text.
foo
(right here) ------^
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it really add the "right here" stuff? :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, yes. It comes directly from yajl. We can attempt to shorten this to a one-liner at a later point.


===DONE===
30 changes: 30 additions & 0 deletions tests/bson/bson-toJSON-001.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
--TEST--
BSON\toJSON(): Encoding JSON
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"?>
--FILE--
<?php

require_once __DIR__ . "/../utils/basic.inc";

$tests = [
[],
[ 'foo' => 'bar' ],
[ 'foo' => [ 1, 2, 3 ]],
[ 'foo' => [ 'bar' => 1 ]],
];

foreach ($tests as $value) {
$bson = fromPHP($value);
echo toJSON($bson), "\n";
}

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
{ }
{ "foo" : "bar" }
{ "foo" : [ 1, 2, 3 ] }
{ "foo" : { "bar" : 1 } }
===DONE===
39 changes: 39 additions & 0 deletions tests/bson/bson-toJSON-002.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
--TEST--
BSON\toJSON(): Encoding extended JSON types
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"?>
--FILE--
<?php
use MongoDB\BSON as BSON;

require_once __DIR__ . "/../utils/basic.inc";

$tests = [
['_id' => new BSON\ObjectId('56315a7c6118fd1b920270b1')],
['binary' => new BSON\Binary('foo', BSON\Binary::TYPE_GENERIC)],
['date' => new BSON\UTCDateTime(1445990400000)],
['timestamp' => new BSON\Timestamp(BSON\Binary::TYPE_GENERIC, 1446084619)],
['regex' => new BSON\Regex('pattern', 'i')],
['minkey' => new BSON\MinKey],
['maxkey' => new BSON\MaxKey],
['long' => 1234],
];

foreach ($tests as $value) {
$bson = fromPHP($value);
echo toJSON($bson), "\n";
}

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
{ "_id" : { "$oid" : "56315a7c6118fd1b920270b1" } }
{ "binary" : { "$type" : "00", "$binary" : "Zm9v" } }
{ "date" : { "$date" : 1445990400000 } }
{ "timestamp" : { "$timestamp" : { "t" : 1446084619, "i" : 0 } } }
{ "regex" : { "$regex" : "pattern", "$options" : "i" } }
{ "minkey" : { "$minKey" : 1 } }
{ "maxkey" : { "$maxKey" : 1 } }
{ "long" : 1234 }
===DONE===
31 changes: 31 additions & 0 deletions tests/bson/bson-toJSON_error-001.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
--TEST--
BSON\toJSON(): BSON decoding exceptions
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"?>
--FILE--
<?php
require_once __DIR__ . "/../utils/basic.inc";

/* We can't really test for bson_iter_init() failure, since bson_reader_read()
* already checks that the buffer is at least 5 bytes.
*/
$invalidBson = array(
'',
str_repeat(fromJSON('{"x": "y"}'), 2),
);

foreach ($invalidBson as $bson) {
echo throws(function() use ($bson) {
toJSON($bson);
}, 'MongoDB\Driver\Exception\UnexpectedValueException'), "\n";
}

?>
===DONE===
<?php exit(0); ?>
--EXPECTF--
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
Could not read document from BSON reader
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
Reading document did not exhaust input buffer
===DONE===