Abhishek Chennaka created KUDU-3810:
---------------------------------------
Summary: ScanTokenPB.feature_flags verification does not reject
tokens using unknown features
Key: KUDU-3810
URL: https://issues.apache.org/jira/browse/KUDU-3810
Project: Kudu
Issue Type: Improvement
Reporter: Abhishek Chennaka
Assignee: Abhishek Chennaka
ScanTokenPB.feature_flags exists so that a client deserializing a scan token
produced by a newer client can detect that the token relies on a feature it
does not implement, and fail cleanly rather than silently running a scan with
different semantics. Both clients have a check intended to do this, and neither
check works.
Java side:
{{{}KuduScanToken.pbIntoScannerBuilder(){}}}:
{code:java}
Preconditions.checkArgument(
!message.getFeatureFlagsList().contains(ScanTokenPB.Feature.Unknown),
"Scan token requires an unsupported feature. This Kudu client must be
updated.");
{code}
C++ side:
{{{}scan_token-internal.cc{}}}:
{code:cpp}
for (int32_t feature : message.feature_flags()) {
if (!ScanTokenPB::Feature_IsValid(feature) || feature ==
ScanTokenPB::Unknown) {
return Status::NotSupported(
"scan token requires features not supported by this client version");
}
}
{code}
h3. Why neither fires
{{feature_flags}} is a proto2 {{repeated}} enum, and proto2 enums are closed:
the generated type is guaranteed to hold only declared values. That guarantee
is enforced at parse time, so an undeclared value is routed into the message's
unknown field set (keyed by field number 1) instead of appearing in the typed
list. Consequences:
- {{getFeatureFlagsList()}} / {{message.feature_flags()}} return a list that
looks complete and well-formed, with no indication that an element was elided.
- {{Feature.Unknown}} is present only if a producer explicitly wrote 0.
Neither writer does — {{scan_token-internal.cc}} and {{KuduScanToken}} both
write {{{}Feature::RowVisibility{}}}, the only feature currently defined.
- {{Feature_IsValid(feature)}} in the C++ loop can never return false, because
an invalid value never reached {{{}message.feature_flags(){}}}.
So both checks only ever trip on an explicit zero, which no producer emits.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
