Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ion_reader_read_string and ion_reader_next access to the same address pointer. #239

Open
cheqianh opened this issue Apr 30, 2021 · 0 comments

Comments

@cheqianh
Copy link
Contributor

cheqianh commented Apr 30, 2021

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);
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant