ion-c icon indicating copy to clipboard operation
ion-c copied to clipboard

ion_reader_read_string and ion_reader_next access to the same address pointer.

Open cheqianh opened this issue 3 years ago • 0 comments

Description:

This issue relates to Pull Request #237, Text/binary reader value changes when read multi-field structs.

Had a brief look. The root is because ion_reader_read_string() and ion_reader_next() both access a string with the same address.

Sample test given by requester:

The test request give:

// What's going on here? If I write a struct {field1:value,field2:value}, then run through to grab the offsets of each
// field, save them for later, then re-seek to them and hydrate, what I expect is:
//
// field1: value
// field2: value
//
// but with the text reader, what I end up with is:
//
// field1: value
// field2: field2
//
// This currently works correctly in the binary reader.
TEST_P(TextAndBinary, ReaderPopulatesStructFieldsOnSeek) {
    // Write!

    hWRITER writer = NULL;
    ION_STREAM *ion_stream = NULL;
    ION_STRING field1, field2, value;
    BYTE *data;
    SIZE data_length;

    // {field1:value,field2:value}
    ion_string_from_cstr("field1", &field1);
    ion_string_from_cstr("field2", &field2);
    ion_string_from_cstr("value", &value);
    ION_ASSERT_OK(ion_test_new_writer(&writer, &ion_stream, is_binary));
    ION_ASSERT_OK(ion_writer_start_container(writer, tid_STRUCT));
    ION_ASSERT_OK(ion_writer_write_field_name(writer, &field1));
    ION_ASSERT_OK(ion_writer_write_symbol(writer, &value));
    ION_ASSERT_OK(ion_writer_write_field_name(writer, &field2));
    ION_ASSERT_OK(ion_writer_write_symbol(writer, &value));
    ION_ASSERT_OK(ion_writer_finish_container(writer));
    ION_ASSERT_OK(ion_test_writer_get_bytes(writer, ion_stream, &data, &data_length));

    // Read!

    hREADER reader = NULL;
    ION_TYPE type;
    POSITION pos_field1, pos_field2;
    ION_STRING read_field1, read_val1, read_field2, read_val2;

    // We use this reader to capture offsets
    ION_ASSERT_OK(ion_test_new_reader(data, data_length, &reader));

    // Assemble: Take one pass through the document to capture value offsets and field names

    ION_ASSERT_OK(ion_reader_next(reader, &type));

    ION_ASSERT_OK(ion_reader_step_in(reader));

    ION_ASSERT_OK(ion_reader_next(reader, &type));
    ASSERT_EQ(tid_SYMBOL, type);

    // Real code would probably capture these, but it doesn't affect the issue
    //ION_ASSERT_OK(ion_reader_get_field_name(reader, &read_field1));
    ION_ASSERT_OK(ion_reader_get_value_offset(reader, &pos_field1));

    ION_ASSERT_OK(ion_reader_next(reader, &type));
    ASSERT_EQ(tid_SYMBOL, type);

    // Real code would probably capture these, but it doesn't affect the issue
    //ION_ASSERT_OK(ion_reader_get_field_name(reader, &read_field2));
    ION_ASSERT_OK(ion_reader_get_value_offset(reader, &pos_field2));

    ION_ASSERT_OK(ion_reader_step_out(reader));

    // Act: Move back to the original offsets and then re-assemble values

    // Seek to first field
    ION_ASSERT_OK(ion_reader_seek(reader, pos_field1, -1));
    ION_ASSERT_OK(ion_reader_next(reader, &type));
    ASSERT_EQ(tid_SYMBOL, type);

    // Read field value
    ION_ASSERT_OK(ion_reader_read_string(reader, &read_val1));

    // Seek to second field
    ION_ASSERT_OK(ion_reader_seek(reader, pos_field2, -1));
    ION_ASSERT_OK(ion_reader_next(reader, &type));
    ASSERT_EQ(tid_SYMBOL, type);

    // Read field value
    ION_ASSERT_OK(ion_reader_read_string(reader, &read_val2));

    ION_ASSERT_OK(ion_reader_close(reader));

    // Assert:

    // Easy assertions: there's only one value, "value," and we should have read it both times
    assertStringsEqual((char *)value.value, (char *)read_val1.value, read_val1.length);
    assertStringsEqual((char *)value.value, (char *)read_val2.value, read_val2.length);
}

cheqianh avatar Apr 30 '21 19:04 cheqianh