stevenschlansker commented on code in PR #2540:
URL: https://github.com/apache/fory/pull/2540#discussion_r2320091710
##########
java/fory-format/src/main/java/org/apache/fory/format/row/binary/writer/BinaryWriter.java:
##########
@@ -121,10 +121,10 @@ protected final void zeroOutPaddingBytes(int numBytes) {
}
/**
- * Since writer is used for one-pass writer, same field won't be writer
twice. There is no need to
- * put zero into the corresponding field when set null.
+ * Writer might recycle buffers, so implementations should clear data from
+ * a previous not-null write when setting null to avoid information leaks.
*/
- public final void setNullAt(int ordinal) {
+ public void setNullAt(int ordinal) {
Review Comment:
@chaokunyang , when you get some time, could you please recommend a fix you
like?
I think the most straightforward fix is to change all encode methods like
`byte[] encode(T)` to also use a new buffer every time, like `toRow(T)` does.
This is nice because then it is not possible to incorrectly leak information,
but that could be a performance penalty to reallocate the buffer.
Alternately, we can change the `setNullAt` contract on `BinaryRowWriter` to
also require `setOffsetAndSize(offset, 0, 0)` from the caller (and update code
generator to do this) but this is a little suspect because then the contract
for `BinaryWriter.setNullAt` is different depending on the specific subtype
(row vs array vs map) writer you use.
Or, do you have another idea to fix this? Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
