[
https://issues.apache.org/jira/browse/AVRO-1743?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Dmitry Spikhalskiy updated AVRO-1743:
-------------------------------------
Description:
BlockingBinaryEncoder, which extends BufferedBinaryEncoder should override
"public void writeFixed(ByteBuffer bytes) throws IOException" method.
Now if we use BlockingBinaryEncoder - all writeFixed(ByteBuffer bytes) are
addressed by BufferedBinaryEncoder. As a result, if "!bytes.hasArray() &&
bytes.remaining() > bulkLimit", then in flushBuffer() we flush empty buffer
from BufferedBinaryEncoder and don't flush actual buffer from
BlockingBinaryEncoder.
I prepared localized unit tests to replicate bugs here:
https://github.com/Spikhalskiy/avro-blockingbinaryencoder-error
Bug could appears in silently incorrect serialization (We will read another
object) or in deserialization errors. Both replicated in provided tests.
Looks like BlockingBinaryEncoder which extends BufferedBinaryEncoder is
error-prone approach and mistake in class hierarchy. We mostly override
everything from BufferedBinaryEncoder, creating unused buffers and fields (like
double pos, buf, etc), and it's already not first bug relating to "somebody
forget to override method in BlockingBinaryEncoder from BufferedBinaryEncoder"
(ex. https://issues.apache.org/jira/browse/AVRO-88). So, this classes should be
separated at all or have common interface, or at least work with same buffer
and pos instances. But BlockingBinaryEncoder shouldn't inherit method
implementations, which work with another buffer object.
was:
BlockingBinaryEncoder, which extends BufferedBinaryEncoder should override
"public void writeFixed(ByteBuffer bytes) throws IOException" method.
Now if we use BlockingBinaryEncoder - all writeFixed(ByteBuffer bytes) are
addressed by BufferedBinaryEncoder. As a result, if "!bytes.hasArray() &&
bytes.remaining() > bulkLimit", then in flushBuffer() we flush empty buffer
from BufferedBinaryEncoder and don't flush actual buffer from
BlockingBinaryEncoder.
I prepared localized unit tests to replicate bugs here:
https://github.com/Spikhalskiy/avro-blockingbinaryencoder-error
Looks like BlockingBinaryEncoder which extends BufferedBinaryEncoder is
error-prone approach and mistake in class hierarchy. We mostly override
everything from BufferedBinaryEncoder, creating unused buffers and fields (like
double pos, buf, etc), and it's already not first bug relating to "somebody
forget to override method in BlockingBinaryEncoder from BufferedBinaryEncoder"
(ex. https://issues.apache.org/jira/browse/AVRO-88). So, this classes should be
separated at all or have common interface, or at least work with same buffer
and pos instances. But BlockingBinaryEncoder shouldn't inherit method
implementations, which work with another buffer object.
> BlockingBinaryEncoder should override writeFixed(ByteBuffer bytes)
> ------------------------------------------------------------------
>
> Key: AVRO-1743
> URL: https://issues.apache.org/jira/browse/AVRO-1743
> Project: Avro
> Issue Type: Bug
> Components: java
> Affects Versions: 1.7.7
> Environment: All environments
> Reporter: Dmitry Spikhalskiy
>
> BlockingBinaryEncoder, which extends BufferedBinaryEncoder should override
> "public void writeFixed(ByteBuffer bytes) throws IOException" method.
> Now if we use BlockingBinaryEncoder - all writeFixed(ByteBuffer bytes) are
> addressed by BufferedBinaryEncoder. As a result, if "!bytes.hasArray() &&
> bytes.remaining() > bulkLimit", then in flushBuffer() we flush empty buffer
> from BufferedBinaryEncoder and don't flush actual buffer from
> BlockingBinaryEncoder.
> I prepared localized unit tests to replicate bugs here:
> https://github.com/Spikhalskiy/avro-blockingbinaryencoder-error
> Bug could appears in silently incorrect serialization (We will read another
> object) or in deserialization errors. Both replicated in provided tests.
> Looks like BlockingBinaryEncoder which extends BufferedBinaryEncoder is
> error-prone approach and mistake in class hierarchy. We mostly override
> everything from BufferedBinaryEncoder, creating unused buffers and fields
> (like double pos, buf, etc), and it's already not first bug relating to
> "somebody forget to override method in BlockingBinaryEncoder from
> BufferedBinaryEncoder" (ex. https://issues.apache.org/jira/browse/AVRO-88).
> So, this classes should be separated at all or have common interface, or at
> least work with same buffer and pos instances. But BlockingBinaryEncoder
> shouldn't inherit method implementations, which work with another buffer
> object.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
