Comcast/sirius

Prevent modification of internal Sirius copy of a value byte array

smuir opened this issue · 2 comments

The current implementation of Sirius passes a byte array as the value argument when calling the RequestHandler handlePut method. After the call to handlePut returns, Sirius proceeds to use that array as-is in subsequent operations, including sending it as the value to other Sirius cluster nodes. If the callee manipulated the array those other nodes will see the modified value. This causes very confusing behaviour.

Sirius should prevent modification of the value array from having any impact on subsequent operations. The best way to do so is TBD, but SHOULD (in the RFC sense) be more than just an API-level statement that the caller shouldn't modify the array. Given very limited read-only abstractions (anything other than ByteBuffer?), the best solution is probably to copy the array before passing it. Since the value array is typically quite small this should have negligible impact. [Aside: if one cared about giant values it might make sense to add an API that uses ByteBuffers in order to also potentially leverage direct mapped arrays]

There seems to be a fairly trivial solution to this problem, i.e., using clone() on the array value, but that fails the unit tests, and I'm not sure if that is intentional. Is there a reason not to just clone() the array?

Passing a copy in should be fine, my guess is initially it was an oversight, and the reason the tests are failing is due to the way matchers were used. Should be able to change the matcher in the "happy path" test case to capture and make sure the array contents are equal but references are different. In the failure case an any should be fine. Thoughts?