alvkeke commented 2 years ago

Crashed on FIOBJ_TYPE_IS(fiobj_null(), FIOBJ_T_NUMBER)

Description

Hi, I found that program will crash at FIOBJ_TYPE_IS(obj, FIOBJ_T_NUMBER) if obj is FIOBJ_T_NULL. but it will only crash when checking if it's FIOBJ_T_NUMBER, it can safely pass the checking when I pass FIOBJ_T_STRING or other types in FIOBJ_TYPE_IS()

I don't know if this is expected, but because of this, I need to do more checking before I check if an obj is FIOBJ_T_NUMBER while I can do only one check if I want to know is an obj FIOBJ_T_STRING

if (!FIOBJ_IS_NULL(obj) && FIOBJ_TYPE_IS(obj, FIOBJ_T_NUMBER)
    do_something();

if (FIOBJ_TYPE_IS(obj, FIOBJ_T_STRING)
    do_something();

could you please take a look?

steps to repro:

I found this issue when I was checking the object type from a hash that parsed out from a JSON.

FIOBJ obj = fiobj_hash_get(req->params, o_key_to_null_or_notexist);
FIOBJ_TYPE_IS(obj, FIOBJ_T_NUMBER);

this issue can also be seen by codes below:

FIOBJ obj = fiobj_null();
FIOBJ_TYPE_IS(obj, FIOBJ_T_NUMBER);

Version

printf("%s\n", FIO_VERSION_STRING);

outputs:

0.7.4

boazsegev commented 2 years ago

Hi @alvkeke ,

Thank you for opening this issue. You are right, FIOBJ_T_NUMBER is a special check since memory allocation for many (but not all) numbers can be optimized away.

I didn't notice this issue before as I was usually treating numbers and strings the same way (using the fiobj_obj2num function to get the data) and using a switch(FIOBJ_TYPE(obj)) {...} statement.

The issue is cause by the line here:

https://github.com/boazsegev/facil.io/blob/7b8e5fcf5e945d07df92aca7728909c5887a80fc/lib/facil/fiobj/fiobject.h#L269-L273

A couple more tests should be used before memory redirection, such as:

FIO_INLINE size_t fiobj_type_is(FIOBJ o, fiobj_type_enum type) {
  switch (type) {
  case FIOBJ_T_NUMBER:
    return (o & FIOBJECT_NUMBER_FLAG) ||
           (o && !(o & FIOBJECT_PRIMITIVE_FLAG) &&
            ((fiobj_type_enum *)o)[0] == FIOBJ_T_NUMBER);

As a workaround, I yours is a good workaround.

You can directly paste the update into the code you're using. I will eventually commit a patch.

alvkeke commented 2 years ago

Hi boazsegev ,

Thanks for your reply and patch, your patch works fine. I like your project very much, thank you for bring such a good work to us.

regards, alvkeke

boazsegev / facil.io