[ AsyncMysql ] Data loss when selecting MYSQL_TYPE_LONGLONG with mapRowsTyped or vectorRowsTyped
- Dominant language
- C++
- Stars
- 18.7k
- Forks
- 3.1k
- Avg merge
- 1h 47m
- Merged PRs (30d)
- 2
Description
**Describe the bug**
Selecting a `MYSQL_TYPE_LONGLONG` from a database using a typed method like `->mapRowsTyped()` causes data loss when the value exceeds `LLONG_MAX`, because of an unchecked errno after a call to [strtoll](https://www.cplusplus.com/reference/cstdlib/strtoll/).
**Standalone code, or other way to reproduce the problem**
```HACK
$conn = await AsyncMysqlClient::connect(...);
$result = await $conn->query('SELECT 9223372036854775808');
$result->mapRows()[0];
// object(HH\Map) (1) {
// ["9223372036854775808"]=>
// string(19) "9223372036854775808" <- ...08
// }
$result->mapRowsTyped()[0];
// object(HH\Map) (1) {
// ["9223372036854775808"]=>
// int(9223372036854775807) <- ...07
// }
```
Steps to reproduce the behavior:
1. Select an unsigned integer that exceeds `LLONG_MAX`.
**Expected behavior**
We need to define some behavior which turns this silent data loss into something usable.
- Throw an exception if `ERANGE` is set by `strtoll`
- Return `MYSQL_TYPE_LONGLONG` as strings
- Return `MYSQL_TYPE_LONGLONG` as signed numbers always
(Hack code will need to do two's complement to get the unsigned value back)
**Actual behavior**
`LLONG_MAX` is returned.
**Environment**
- Operating system
> Ubuntu 20.04
- Installation method
> apt-get with dl.hhvm.com repository
- HHVM Version
```
HipHop VM 4.56.1 (rel)
Compiler: 1593547575_929404933
Repo schema: d1ae8e21bf3419a65f12a010527485564e719d07
```
**Additional context**
### CPP callgraph
HHVM_METHOD for `AsyncMysqlQueryResult->mapRowsTyped()` calls `AsyncMysqlQueryResult::buildRows()`.
https://github.com/facebook/hhvm/blob/a95e1f715406144f62ca1bd1a8fe4dd07c2f3df7/hphp/runtime/ext/async_mysql/ext_async_mysql.cpp#L1249-L1252
`AsyncMysqlQueryResult::buildRows()` calls `buildTypedValue`.
https://github.com/facebook/hhvm/blob/a95e1f715406144f62ca1bd1a8fe4dd07c2f3df7/hphp/runtime/ext/async_mysql/ext_async_mysql.cpp#L1334-L1337
`buildTypedValue` calls `mysql_makevalue` when `typed_values` is true.
https://github.com/facebook/hhvm/blob/a95e1f715406144f62ca1bd1a8fe4dd07c2f3df7/hphp/runtime/ext/async_mysql/ext_async_mysql.cpp#L1310-L1314
`mysql_makevalue` calls `String.toInt64()` when `field_type` is something intish.
https://github.com/facebook/hhvm/blob/a95e1f715406144f62ca1bd1a8fe4dd07c2f3df7/hphp/runtime/ext/mysql/mysql_common.cpp#L1167-L1175
`String.toInt64()` calls `StringData.toInt64()`
https://github.com/facebook/hhvm/blob/a95e1f715406144f62ca1bd1a8fe4dd07c2f3df7/hphp/runtime/base/type-string.h#L344
`StringData.toInt64()` calls `strtoll()`
https://github.com/facebook/hhvm/blob/a95e1f715406144f62ca1bd1a8fe4dd07c2f3df7/hphp/runtime/base/string-data.cpp#L844-L846
[strtoll](https://www.cplusplus.com/reference/cstdlib/strtoll/) has this error state which goes unchecked.
```
If the value read is out of the range of representable values by a long long int,
the function returns LLONG_MAX or LLONG_MIN (defined in ),
and errno is set to ERANGE.
```
Hack code does not have access to this `ERANGE` and will therefore not know that this error has occurred.
Contributor guide
Research direction
Start with hphp/runtime/ext/mysql/mysql_common.cpp and hphp/runtime/base/string-data.cpp, following mysql_makevalue through String.toInt64() to the strtoll call; the mapRowsTyped() entry point is in hphp/runtime/ext/async_mysql/ext_async_mysql.cpp. First resolve which overflow behavior the API should guarantee, then add coverage for an unsigned value above LLONG_MAX so typed results no longer silently become LLONG_MAX.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, mysql
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100