go icon indicating copy to clipboard operation
go copied to clipboard

Iterator Reset/ResetBytes Should Clear Error/Attachment

Open kevinconaway opened this issue 5 years ago • 1 comments

We reuse iterator instances but not via Pools since we reuse them per thread.

If the iterator.Error value is not cleared between usages, it will cause undefined parsing behavior as various calls (like unreadByte) do not occur if iterator.Error has a value.

I believe that iterator.ResetBytes and iterator.Reset should clear the Error and Attachment fields, just like pool.ReturnIterator does.

I'm happy to submit a PR for this if you think it worthwhile

kevinconaway avatar Oct 09 '20 15:10 kevinconaway

I stumbled upon this bug too and went ahead and wrote a failing unit test.

func TestIterator_Reset(t *testing.T) {
	type Data struct {
		A int `json:"a"`
	}
	data := Data{}

	iter := ParseString(ConfigDefault, `{"a":1}`)
	require.NoError(t, iter.Error)

	// read
	iter.ReadVal(&data)
	require.Equal(t, 1, data.A)

	// emulate a loop that reads multiple values by just calling `WhatIsNext`
	require.Equal(t, InvalidValue, iter.WhatIsNext())

	// reuse
	iter.Reset(strings.NewReader(`{"a":2}`))
	data = Data{}
	iter.ReadVal(&data)
	require.NoError(t, iter.Error)
	require.Equal(t, 2, data.A)
}

This prints:

jsoniter.Data.readFieldHash: expect ", but found a, error found in #3 byte of ...|{"a":2}|..., bigger context ...|{"a":2}|...

If you remove the iter.WhatIsNext() call it passes. It's because it ends up with an EOF here but it doesn't reset it here

papanikge avatar Jun 21 '24 18:06 papanikge