PHPC-741: Consistent exceptions for BSON init methods

[jmikola]

https://jira.mongodb.org/browse/PHPC-741

Due to overlap, I've also included the PRs for disallowing null bytes in Javascript's code and Regex's pattern and flags fields:

• https://jira.mongodb.org/browse/PHPC-721
• https://jira.mongodb.org/browse/PHPC-724

[jmikola]

The original STRTOLL() macro we had uses three possible functions:

• _atoi64() on Windows, which supports error reporting (may not detect extra data)
• atoll() if HAVE_ATOLL is defined, which doesn't support error reporting at all
• strtoll() otherwise, which supports error reporting and endptr for detecting extra data

Support for atoll() appears to be a concession for HP-UX's broken strtoll() implementation (see: PHP bug #33433). If we care about HP-UX portability, I suggest we use bson_ascii_strtoll(), which is a strtoll() polyfill in libbson. It does have an issue with detecting out-of-range integers (see: CDRIVER-1377), but I think this is an improvement over the current use of atoll() and reporting no errors at all.

[jmikola]

This message was previously only used in the constructor; however, the exception may now be thrown from __set_state() or unserialize(). What do you think about putting the class name in the message?

I've started doing this with other new messages (e.g. Javascript now allowing null bytes in its code argument) for added clarity, but did not want to change existing messages before discussing.

[jmikola]

To clarify, my main concern was with unserialize(), as I expected that would be the only function in the stack trace. After further testing, it appears that the appropriate __wakeup() method is included in the backtrace as "[internal function]". I tested this by unserializing a Javascript object with an invalid Regex in its scope document, just to see how nesting is handled.

<?php

unserialize('O:23:"MongoDB\BSON\Javascript":2:{s:4:"code";s:33:"function foo(bar) { return bar; }";s:5:"scope";O:8:"stdClass":1:{s:1:"r";O:18:"MongoDB\BSON\Regex":2:{s:7:"pattern";s:6:"regexp";s:5:"flags";s:1:"'. "\0" .'";}}}');

Output:

Fatal error: Uncaught exception 'MongoDB\Driver\Exception\InvalidArgumentException' with message 'MongoDB\BSON\Regex flags should not contain null bytes' in tests/bson/bson-regex-serialization_error-002.php:15
Stack trace:
#0 [internal function]: MongoDB\BSON\Regex->__wakeup()
#1 tests/bson/bson-regex-serialization_error-002.php(15): unserialize('O:23:"MongoDB\\B...')
#2 {main}
  thrown in tests/bson/bson-regex-serialization_error-002.php on line 15

Given this behavior, I think we're OK with not having the class name in every message.

[jmikola]

This is the same error message used in HHVM (see: here), but I think there's room for improvement. We should at least show the offending value.

Let's discuss and create a new ticket if needed.

[jmikola]

A number of the exception messages for PHPC-741 involved reporting zend_class_entry->name. On PHP 7, this required accessing zend_string->val. Digging a little deeper, I found that we were inconsistently using either the phongo_str() macro or using #if blocks to conditionally use ZSTR_VAL() for PHP 7. Simply backporting ZSTR_VAL() to PHP 5.x allowed me to remove our custom macro and delete those #if blocks.

[derickr]

LGTM