amazon-ion / ion-c

A C implementation of Amazon Ion.
https://amazon-ion.github.io/ion-docs/
Apache License 2.0
166 stars 43 forks source link

ion text reader behavior with null.bool types #330

Closed ezabes closed 9 months ago

ezabes commented 1 year ago

I believe the correct behavior for null.bool types handled by the ion text reader should throw an IERR_NULL_VALUE instead of throwing an IERR_INVALID_STATE error.

Symptoms: 1) All other null.* types return IERR_NULL_VALUE. 2) Binary reader throws IERR_NULL_VALUE in similar circumstance.

Below is a workable fix. Add the following code on line 1333 before the else clause in ionc/ion_reader_text.c

else if ((text->_value_sub_type->flags & FCF_IS_NULL) !=0) {
          FAILWITH(IERR_NULL_VALUE);
}

Code below recreates issue.

#include <stdlib.h>
#include "ionc/ion.h"

#define ION_OK(x) if (x) { printf("Error: %s\n", ion_error_to_str(x)); return x; }

#define ION_ASSERT_TYPE(type, x) if ((x) != (type)) { ION_OK(IERR_INVALID_STATE); }

int main(int argc, char **argv) {
  const char* ion_text = "null.bool";

    hREADER reader;
    ION_READER_OPTIONS options;

    memset(&options, 0, sizeof(ION_READER_OPTIONS));

    ION_OK(ion_reader_open_buffer(&reader,
                                  (BYTE *)ion_text,
                                  (SIZE)strlen(ion_text),
                                  &options));

    ION_TYPE ion_type;
    BOOL value_ion_bool;

    // This line retrieves the ion type
    ION_OK(ion_reader_next(reader, &ion_type));

    // This line shows that the type is a tid_BOOL
    ION_ASSERT_TYPE(ion_type, tid_BOOL);

    // This call shows that the error code is not IERR_NULL_VALUE
    // but is instead IERR_INVALID_STATE
    ION_OK(ion_reader_read_bool(reader, &value_ion_bool));

    return 0;
}
nirosys commented 1 year ago

Thank you for the issue ezabes! We haven't had a chance to look into this yet, but will hopefully follow up in the next couple days.

nirosys commented 9 months ago

The fix has just been merged. Thank you again for bringing this to our attention!