Reject invalid non-null tokens beginning with n #93
Conversation
tobil4sk
force-pushed
the
fix/reject-non-null-n-tokens
branch
from
afb69f1 to
f00ee09
Compare
|
I'm thinking about waiting until the next release of simdjson before getting this merged. Alternatively commenting out the broken tests, and then we can enable them in the PR for the next simdjson release. |
Tokens that begin with `n` should not be treated as `null`.
In 0f609dd, a regression was introduced where the parser would read any token that started with n as null. The `.type()` method returns `json_type::null` simply if the token begins with `n`, and it does not check whether the token is actually valid. Adding an `is_null()` call here ensures this and returns an error if the token starts with `n` but is not `null`. Calling `.value()` on the returned value will raise it as an exception if `is_null()` returned an error. Currently with simdjson 3.10.1, there is a bug where calling `is_null` on a document object lacks this behaviour, so currently this patch only fixes the problem for non-scalar documents.
tobil4sk
force-pushed
the
fix/reject-non-null-n-tokens
branch
from
f00ee09 to
e6ff957
Compare
I've gone and commented those out, so the tests are passing for now. If you want to merge this now, I'll open a new PR so we don't forget about re-enabling them when the next simdjson version is released. |
Sounds good to me! The only thing is whether or not to cut a release of lua-simdjson in the meanwhile. |
FourierTransformer
merged commit 4a65dc5
into
FourierTransformer:master
|
Thanks!
On second thought, it might be worth adding a workaround for now so that lua-simdjson still works properly and we don't have to wait until the next simdjson release. See #94 |
In 0f609dd, a regression was introduced where the parser would read any token that started with n as null.
The
.type()method returnsjson_type::nullsimply if the token begins withn, and it does not check whether the token is actually valid.Adding an
is_null()call here ensures this and returns an error if the token starts withnbut is notnull. Calling.value()on the returned value will raise it as an exception ifis_null()returned an error.Currently with simdjson 3.10.1, there is a bug where calling
is_nullon a document object lacks this behaviour, so currently this patch only fixes the problem for non-scalar documents. See simdjson/simdjson#2258