Re: Assertions as binary data validation checks in deserialization
We have a lot of such assertions. They are created for developers, not for users. We cannot cover all such places with checks for corrupted data or so, as it will decrease performance significantly. Hence, proposed fix doesn't make sense to me. чт, 27 июля 2017 г. в 23:43, Andrey Kuznetsov : > May I change this line while working on existing ticket, unless someone > else objects? > > 27 июля 2017 г. 23:37 пользователь "Valentin Kulichenko" < > valentin.kuliche...@gmail.com> написал: > > Makes sense to me. Feel free to create a ticket unless someone else has any > objection. However, I think that we should revise other code for such > places then. Fixing only this one line doesn't change a lot. > > -Val >
Re: Assertions as binary data validation checks in deserialization
May I change this line while working on existing ticket, unless someone else objects? 27 июля 2017 г. 23:37 пользователь "Valentin Kulichenko" < valentin.kuliche...@gmail.com> написал: Makes sense to me. Feel free to create a ticket unless someone else has any objection. However, I think that we should revise other code for such places then. Fixing only this one line doesn't change a lot. -Val
Re: Assertions as binary data validation checks in deserialization
Makes sense to me. Feel free to create a ticket unless someone else has any objection. However, I think that we should revise other code for such places then. Fixing only this one line doesn't change a lot. -Val On Thu, Jul 27, 2017 at 12:55 PM, Andrey Kuznetsov wrote: > Indeed, "let it crash" approach is better than unclear error in some > indeterminate place later. Here we depend on data from "inpredictable" > source, so assertions are not suitable. > > 27 июля 2017 г. 22:35 пользователь "Valentin Kulichenko" < > valentin.kuliche...@gmail.com> написал: > > Do you suggest to throw an exception instead of assertions? > > -Val >
Re: Assertions as binary data validation checks in deserialization
Indeed, "let it crash" approach is better than unclear error in some indeterminate place later. Here we depend on data from "inpredictable" source, so assertions are not suitable. 27 июля 2017 г. 22:35 пользователь "Valentin Kulichenko" < valentin.kuliche...@gmail.com> написал: Do you suggest to throw an exception instead of assertions? -Val
Re: Assertions as binary data validation checks in deserialization
Do you suggest to throw an exception instead of assertions? -Val On Thu, Jul 27, 2017 at 11:52 AM, Andrey Kuznetsov wrote: > Valentin, > > I meant the behaviour of this code when corrupted data from the network are > being deserialized. Assertion is no-op in production, and we silently > ignore binary format violation. > > 27 июля 2017 г. 21:09 пользователь "Valentin Kulichenko" < > valentin.kuliche...@gmail.com> написал: > > > Andrey, > > > > How will it corrupt the data? Assertions only reads the array, not > updates > > it, right? > > > > -Val > > > > On Thu, Jul 27, 2017 at 8:54 AM, Andrey Kuznetsov > > wrote: > > > > > Hi Igniters, > > > > > > While examining BinaryObjectImpl code I found this curious line in > > typeId() > > > method: > > > > > > assert arr[off] == GridBinaryMarshaller.STRING : arr[off]; > > > > > > Is it OK to check external binary data with assertions? > > > I think it can lead to undefined behaviour on corrupt data from the > wire. > > > > > > -- > > > Best regards, > > > Andrey Kuznetsov. > > > > > >
Re: Assertions as binary data validation checks in deserialization
Valentin, I meant the behaviour of this code when corrupted data from the network are being deserialized. Assertion is no-op in production, and we silently ignore binary format violation. 27 июля 2017 г. 21:09 пользователь "Valentin Kulichenko" < valentin.kuliche...@gmail.com> написал: > Andrey, > > How will it corrupt the data? Assertions only reads the array, not updates > it, right? > > -Val > > On Thu, Jul 27, 2017 at 8:54 AM, Andrey Kuznetsov > wrote: > > > Hi Igniters, > > > > While examining BinaryObjectImpl code I found this curious line in > typeId() > > method: > > > > assert arr[off] == GridBinaryMarshaller.STRING : arr[off]; > > > > Is it OK to check external binary data with assertions? > > I think it can lead to undefined behaviour on corrupt data from the wire. > > > > -- > > Best regards, > > Andrey Kuznetsov. > > >
Re: Assertions as binary data validation checks in deserialization
Andrey, How will it corrupt the data? Assertions only reads the array, not updates it, right? -Val On Thu, Jul 27, 2017 at 8:54 AM, Andrey Kuznetsov wrote: > Hi Igniters, > > While examining BinaryObjectImpl code I found this curious line in typeId() > method: > > assert arr[off] == GridBinaryMarshaller.STRING : arr[off]; > > Is it OK to check external binary data with assertions? > I think it can lead to undefined behaviour on corrupt data from the wire. > > -- > Best regards, > Andrey Kuznetsov. >
Assertions as binary data validation checks in deserialization
Hi Igniters, While examining BinaryObjectImpl code I found this curious line in typeId() method: assert arr[off] == GridBinaryMarshaller.STRING : arr[off]; Is it OK to check external binary data with assertions? I think it can lead to undefined behaviour on corrupt data from the wire. -- Best regards, Andrey Kuznetsov.
