[jira] [Comment Edited] (CASSANDRA-17254) nodetool toppartitions can fail because ByteBuffer.array() returns more bytes than would be considered valid by UTF8Serializer.validate

2022-11-07 Thread Jordan West (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-17254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17630073#comment-17630073
 ] 

Jordan West edited comment on CASSANDRA-17254 at 11/7/22 10:38 PM:
---

If I am reading the 3.11 code right (pasted below jic), it uses {{bytesToHex}} 
for the key but just uses the {{ByteBuffer}} directly unlike my patch which 
duplicates it for the value. I am fine with the 3.11 approach as well assuming 
the comment is correct:
{code:java}
//Not duplicating the buffer for safety because AbstractSerializer and 
ByteBufferUtil.bytesToHex
//don't modify position or limit
ByteBuffer key = counter.getItem();
result.put(new CompositeDataSupport(COUNTER_COMPOSITE_TYPE, COUNTER_NAMES, new 
Object[] {
ByteBufferUtil.bytesToHex(key), // raw
counter.getCount(),  // count
counter.getError(),  // error
metadata.getKeyValidator().getString(key) })); // string {code}
 


was (Author: jrwest):
If I am reading the 3.11 code right (pasted below jic), it uses {{bytesToHex}} 
for the key but just uses the {{ByteBuffer}} directly unlike my patch which 
duplicates it. I am fine with the 3.11 approach as well assuming the comment is 
correct:


{code:java}
//Not duplicating the buffer for safety because AbstractSerializer and 
ByteBufferUtil.bytesToHex
//don't modify position or limit
ByteBuffer key = counter.getItem();
result.put(new CompositeDataSupport(COUNTER_COMPOSITE_TYPE, COUNTER_NAMES, new 
Object[] {
ByteBufferUtil.bytesToHex(key), // raw
counter.getCount(),  // count
counter.getError(),  // error
metadata.getKeyValidator().getString(key) })); // string {code}
 

> nodetool toppartitions can fail because ByteBuffer.array() returns more bytes 
> than would be considered valid by UTF8Serializer.validate
> ---
>
> Key: CASSANDRA-17254
> URL: https://issues.apache.org/jira/browse/CASSANDRA-17254
> Project: Cassandra
>  Issue Type: Bug
>  Components: Tool/nodetool
>Reporter: Jordan West
>Assignee: Jordan West
>Priority: Normal
> Fix For: 3.0.x, 3.11.x
>
>
> The error below is caused by the use of 
> [{{ByteBuffer.array()}}|https://github.com/apache/cassandra/blob/cassandra-3.0/src/java/org/apache/cassandra/db/ColumnFamilyStore.java#L1628].
>  Doing so not only makes the hex key potentially incorrect but causes invalid 
> data to be passed to {{AbstractType.getString}} and ultimately 
> {{UTF8Validator.validate}}. 
> {code}
> error: String didn't validate.
> -- StackTrace --
> org.apache.cassandra.serializers.MarshalException: String didn't validate.
>   at 
> org.apache.cassandra.serializers.UTF8Serializer.validate(UTF8Serializer.java:35)
>   at 
> org.apache.cassandra.db.marshal.AbstractType.getString(AbstractType.java:129)
>   at 
> org.apache.cassandra.db.ColumnFamilyStore.finishLocalSampling(ColumnFamilyStore.java:1633)
>   at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>   at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>   at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org



[jira] [Comment Edited] (CASSANDRA-17254) nodetool toppartitions can fail because ByteBuffer.array() returns more bytes than would be considered valid by UTF8Serializer.validate

2022-11-07 Thread Brandon Williams (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-17254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17630066#comment-17630066
 ] 

Brandon Williams edited comment on CASSANDRA-17254 at 11/7/22 10:26 PM:


bq. I was thinking maybe an in-JVM dtest but the key has to be a certain format 
to trigger the issue. On the other hand, the usage of the ByteBuffer API here 
is definitely wrong. Thoughts?

You're right, that's a lot of effort to trigger it.  The usage is definitely 
wrong though, that person didn't know what they were doing (it was me.)

3.11 doesn't seem to have this problem and uses ByteBufferUtil.bytesToHex 
instead, any reason not to just backport that solution to 3.0?


was (Author: brandon.williams):
.bq I was thinking maybe an in-JVM dtest but the key has to be a certain format 
to trigger the issue. On the other hand, the usage of the ByteBuffer API here 
is definitely wrong. Thoughts?

You're right, that's a lot of effort to trigger it.  The usage is definitely 
wrong though, that person didn't know what they were doing (it was me.)

3.11 doesn't seem to have this problem and uses ByteBufferUtil.bytesToHex 
instead, any reason not to just backport that solution to 3.0?

> nodetool toppartitions can fail because ByteBuffer.array() returns more bytes 
> than would be considered valid by UTF8Serializer.validate
> ---
>
> Key: CASSANDRA-17254
> URL: https://issues.apache.org/jira/browse/CASSANDRA-17254
> Project: Cassandra
>  Issue Type: Bug
>  Components: Tool/nodetool
>Reporter: Jordan West
>Assignee: Jordan West
>Priority: Normal
> Fix For: 3.0.x, 3.11.x
>
>
> The error below is caused by the use of 
> [{{ByteBuffer.array()}}|https://github.com/apache/cassandra/blob/cassandra-3.0/src/java/org/apache/cassandra/db/ColumnFamilyStore.java#L1628].
>  Doing so not only makes the hex key potentially incorrect but causes invalid 
> data to be passed to {{AbstractType.getString}} and ultimately 
> {{UTF8Validator.validate}}. 
> {code}
> error: String didn't validate.
> -- StackTrace --
> org.apache.cassandra.serializers.MarshalException: String didn't validate.
>   at 
> org.apache.cassandra.serializers.UTF8Serializer.validate(UTF8Serializer.java:35)
>   at 
> org.apache.cassandra.db.marshal.AbstractType.getString(AbstractType.java:129)
>   at 
> org.apache.cassandra.db.ColumnFamilyStore.finishLocalSampling(ColumnFamilyStore.java:1633)
>   at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>   at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>   at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org



[jira] [Comment Edited] (CASSANDRA-17254) nodetool toppartitions can fail because ByteBuffer.array() returns more bytes than would be considered valid by UTF8Serializer.validate

2022-11-07 Thread Cheng Wang (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-17254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17630057#comment-17630057
 ] 

Cheng Wang edited comment on CASSANDRA-17254 at 11/7/22 10:13 PM:
--

+1. Approved. 


was (Author: JIRAUSER291462):
+1. Approached. 

> nodetool toppartitions can fail because ByteBuffer.array() returns more bytes 
> than would be considered valid by UTF8Serializer.validate
> ---
>
> Key: CASSANDRA-17254
> URL: https://issues.apache.org/jira/browse/CASSANDRA-17254
> Project: Cassandra
>  Issue Type: Bug
>  Components: Tool/nodetool
>Reporter: Jordan West
>Assignee: Jordan West
>Priority: Normal
> Fix For: 3.0.x, 3.11.x
>
>
> The error below is caused by the use of 
> [{{ByteBuffer.array()}}|https://github.com/apache/cassandra/blob/cassandra-3.0/src/java/org/apache/cassandra/db/ColumnFamilyStore.java#L1628].
>  Doing so not only makes the hex key potentially incorrect but causes invalid 
> data to be passed to {{AbstractType.getString}} and ultimately 
> {{UTF8Validator.validate}}. 
> {code}
> error: String didn't validate.
> -- StackTrace --
> org.apache.cassandra.serializers.MarshalException: String didn't validate.
>   at 
> org.apache.cassandra.serializers.UTF8Serializer.validate(UTF8Serializer.java:35)
>   at 
> org.apache.cassandra.db.marshal.AbstractType.getString(AbstractType.java:129)
>   at 
> org.apache.cassandra.db.ColumnFamilyStore.finishLocalSampling(ColumnFamilyStore.java:1633)
>   at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>   at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>   at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org