[
https://issues.apache.org/jira/browse/CASSANDRA-15790?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17102024#comment-17102024
]
David Capwell commented on CASSANDRA-15790:
-------------------------------------------
bq. instead of using null as the default value in parse, if you use "" (empty
string) you can short circuit that special case in addition to handling the no
value passed condition.
Can't short-circuit as this would log, this also confuses since users could
define "". I switched to java.lang.System#getProperty(java.lang.String) which
defaults to null.
bq. Consider trimming the string passed to NonEmptyWriteHandler.valueOf
Done
bq. Consider renaming “LOG_DATA_LOSS” to “LOG” for succinctness
We spoke in slack, I rather not as doing this causes data loss, so I care more
that you are aware of that than how succinct the name is. Renamed silent to
silent_DATA_LOSS for the same reasons.
bq. Really minor style nit: consider remaking *handle(r) to *mode or *behavior
Switched to behavior
bq. Minor style nit: define the NonEmptyWriteHandle enum closer to parse()
done.
[~jwest] commit for all feedback can be located
[here|https://github.com/dcapwell/cassandra/commit/166ab9c82401f0588959376ffcf2384f7a66f9f1]
> EmptyType doesn't override writeValue so could attempt to write bytes when
> expected not to
> ------------------------------------------------------------------------------------------
>
> Key: CASSANDRA-15790
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15790
> Project: Cassandra
> Issue Type: Bug
> Components: CQL/Semantics
> Reporter: David Capwell
> Assignee: David Capwell
> Priority: Normal
> Fix For: 3.0.x, 3.11.x, 4.0-alpha
>
>
> EmptyType.writeValues is defined here
> https://github.com/apache/cassandra/blob/e394dc0bb32f612a476269010930c617dd1ed3cb/src/java/org/apache/cassandra/db/marshal/AbstractType.java#L407-L414
> {code}
> public void writeValue(ByteBuffer value, DataOutputPlus out) throws
> IOException
> {
> assert value.hasRemaining();
> if (valueLengthIfFixed() >= 0)
> out.write(value);
> else
> ByteBufferUtil.writeWithVIntLength(value, out);
> }
> {code}
> This is fine when the value is empty as the write of empty no-ops (the
> readValue also noops since the length is 0), but if the value is not empty
> (possible during upgrades or random bugs) then this could silently cause
> corruption; ideally this should throw a exception if the ByteBuffer has data.
> This was called from
> org.apache.cassandra.db.rows.BufferCell.Serializer#serialize, here we check
> to see if data is present or not and update the flags. If data is present
> then and only then do we call type.writeValue (which requires bytes is not
> empty). The problem is that EmptyType never expects writes to happen, but it
> still writes them; and does not read them (since it says it is fixed length
> of 0, so does read(buffer, 0)).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
