FANNG1 commented on code in PR #66805:
URL: https://github.com/apache/doris/pull/66805#discussion_r3809563946
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/LanceStorageOptions.java:
##########
@@ -17,56 +17,171 @@
package org.apache.doris.datasource.lance;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Locale;
import java.util.Map;
+import java.util.Set;
-/** Converts normalized Doris storage properties to Lance object-store
options. */
+/**
+ * Builds the Lance object-store options for one dataset.
+ *
+ * <p>Both the FE, which opens the dataset through the Lance Java SDK, and the
BE, which opens it
+ * through lance-c, consume the map produced here, so the two cannot disagree
about how a dataset
+ * is accessed.
+ *
+ * <p>Options vended by a namespace are merged in as they arrive. The Lance
Namespace specification
+ * describes {@code storage_options} as configuration "passed directly to
Lance", so the protocol
+ * defines no key vocabulary of its own and a client cannot assume one.
Re-encoding those options
+ * into a fixed set of names would silently drop everything outside it,
including credentials
+ * spelled with a different accepted alias and every non-S3 provider's keys.
+ */
public final class LanceStorageOptions {
+ private static final Logger LOG =
LogManager.getLogger(LanceStorageOptions.class);
+
+ /**
+ * Doris backend property to Lance object-store option.
+ *
+ * <p>Lance reaches S3 through object_store, which accepts both {@code
access_key_id} and
+ * {@code aws_access_key_id}. The unprefixed spelling is chosen because it
is also the field
+ * name used by the OpenDAL backend, which performs no alias normalization
at all, so these
+ * options stay correct if that backend is ever selected.
+ */
private static final Map<String, String> S3_KEYS = new HashMap<>();
static {
- S3_KEYS.put("AWS_ACCESS_KEY", "aws_access_key_id");
- S3_KEYS.put("AWS_SECRET_KEY", "aws_secret_access_key");
- S3_KEYS.put("AWS_TOKEN", "aws_session_token");
- S3_KEYS.put("AWS_ENDPOINT", "aws_endpoint");
- S3_KEYS.put("AWS_REGION", "aws_region");
+ S3_KEYS.put("AWS_ACCESS_KEY", "access_key_id");
Review Comment:
Agreed, and fixed in ed170992de8 — the emitted spelling is now
object_store's canonical one (`aws_access_key_id`, `aws_endpoint`, ...), so
`with_env_s3`'s `!self.0.contains_key(config_key.as_ref())` check is satisfied
and the environment value is skipped. `allow_http` stays unprefixed because
that *is* its canonical name (`ClientConfigKey::AllowHttp.as_ref()`).
This also corrects the rationale I had written for the unprefixed spelling.
OpenDAL does normalize aliases, through serde: `aws_access_key_id` and the rest
are declared `#[serde(alias = ...)]` on its S3 config, in both the pinned 0.56
and 0.57. So the canonical spelling is right for both backends — and the
unprefixed one was in fact the worse choice there, since an
environment-injected canonical alias lands beside it and serde rejects the
duplicate field.
One caveat worth recording: the spelling buys nothing for `allow_http`.
`StorageOptions::new` overwrites that key outright from `AWS_ALLOW_HTTP` before
`with_env_s3` runs, so a process can still override what is derived. Noted in
the javadoc.
`testEmittedOptionsUseTheSpellingThatSuppressesTheEnvironment` now pins the
emitted key set to exactly the canonical names.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/LanceStorageOptions.java:
##########
@@ -17,56 +17,171 @@
package org.apache.doris.datasource.lance;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Locale;
import java.util.Map;
+import java.util.Set;
-/** Converts normalized Doris storage properties to Lance object-store
options. */
+/**
+ * Builds the Lance object-store options for one dataset.
+ *
+ * <p>Both the FE, which opens the dataset through the Lance Java SDK, and the
BE, which opens it
+ * through lance-c, consume the map produced here, so the two cannot disagree
about how a dataset
+ * is accessed.
+ *
+ * <p>Options vended by a namespace are merged in as they arrive. The Lance
Namespace specification
+ * describes {@code storage_options} as configuration "passed directly to
Lance", so the protocol
+ * defines no key vocabulary of its own and a client cannot assume one.
Re-encoding those options
+ * into a fixed set of names would silently drop everything outside it,
including credentials
+ * spelled with a different accepted alias and every non-S3 provider's keys.
+ */
public final class LanceStorageOptions {
+ private static final Logger LOG =
LogManager.getLogger(LanceStorageOptions.class);
+
+ /**
+ * Doris backend property to Lance object-store option.
+ *
+ * <p>Lance reaches S3 through object_store, which accepts both {@code
access_key_id} and
+ * {@code aws_access_key_id}. The unprefixed spelling is chosen because it
is also the field
+ * name used by the OpenDAL backend, which performs no alias normalization
at all, so these
+ * options stay correct if that backend is ever selected.
+ */
private static final Map<String, String> S3_KEYS = new HashMap<>();
static {
- S3_KEYS.put("AWS_ACCESS_KEY", "aws_access_key_id");
- S3_KEYS.put("AWS_SECRET_KEY", "aws_secret_access_key");
- S3_KEYS.put("AWS_TOKEN", "aws_session_token");
- S3_KEYS.put("AWS_ENDPOINT", "aws_endpoint");
- S3_KEYS.put("AWS_REGION", "aws_region");
+ S3_KEYS.put("AWS_ACCESS_KEY", "access_key_id");
+ S3_KEYS.put("AWS_SECRET_KEY", "secret_access_key");
+ S3_KEYS.put("AWS_TOKEN", "session_token");
+ S3_KEYS.put("AWS_ENDPOINT", "endpoint");
+ S3_KEYS.put("AWS_REGION", "region");
}
+ /**
+ * Every spelling object_store accepts for the options above, mapped to
the one this class emits.
+ *
+ * <p>object_store resolves an alias and its canonical name to one config
key and keeps only one
+ * of the two values, chosen by hash order. So a namespace vending {@code
endpoint_url} while the
+ * catalog contributes {@code endpoint} does not override it - the two
survive as separate
+ * entries, and the FE and the BE can each end up using a different one.
Every accepted alias has
+ * to be recognized here, or that race simply moves to the spellings this
table misses.
+ */
+ private static final Map<String, String> CANONICAL_BY_ALIAS =
ImmutableMap.<String, String>builder()
+ .put("access_key_id", "access_key_id")
+ .put("aws_access_key_id", "access_key_id")
+ .put("secret_access_key", "secret_access_key")
+ .put("aws_secret_access_key", "secret_access_key")
+ .put("session_token", "session_token")
+ .put("aws_session_token", "session_token")
+ .put("aws_token", "session_token")
+ .put("token", "session_token")
+ .put("endpoint", "endpoint")
+ .put("endpoint_url", "endpoint")
+ .put("aws_endpoint", "endpoint")
+ .put("aws_endpoint_url", "endpoint")
Review Comment:
Correct, and it took two passes to get right.
ed170992de8 added `aws_endpoint_url_s3` to the alias table, folded onto the
generic endpoint entry. That was wrong: it is not an alias at all. object_store
parses it into a config key of its own and always prefers it — `let endpoint =
self.s3_endpoint.or(self.endpoint)` (builder.rs:1209) — so collapsing the two
replaced that precedence rule with map order.
e850bffed50 fixes it properly: the resolved endpoint is written under *both*
spellings. object_store reads the same value whichever key it consults, OpenDAL
ignores the S3-specific one as an unknown field, and a namespace vending both
spellings resolves to the S3-specific value, matching what object_store itself
would have done.
That commit also closes a second hole implied here: because the map never
contained the S3-specific spelling, `with_env_s3` was free to fill it from the
environment, and object_store would then *prefer* it over the catalog's
endpoint. A stray `AWS_ENDPOINT_URL_S3` in the FE or BE process silently
redirected every request. Emitting both keys occupies both against the
environment.
Covered by `testEveryAcceptedAliasCollapsesOntoOneValue`,
`testS3SpecificEndpointWinsOverAGenericOneVendedBesideIt` and
`testAllowHttpFollowsTheS3SpecificEndpoint`.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/LanceStorageOptions.java:
##########
@@ -17,56 +17,171 @@
package org.apache.doris.datasource.lance;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Locale;
import java.util.Map;
+import java.util.Set;
-/** Converts normalized Doris storage properties to Lance object-store
options. */
+/**
+ * Builds the Lance object-store options for one dataset.
+ *
+ * <p>Both the FE, which opens the dataset through the Lance Java SDK, and the
BE, which opens it
+ * through lance-c, consume the map produced here, so the two cannot disagree
about how a dataset
+ * is accessed.
+ *
+ * <p>Options vended by a namespace are merged in as they arrive. The Lance
Namespace specification
+ * describes {@code storage_options} as configuration "passed directly to
Lance", so the protocol
+ * defines no key vocabulary of its own and a client cannot assume one.
Re-encoding those options
+ * into a fixed set of names would silently drop everything outside it,
including credentials
+ * spelled with a different accepted alias and every non-S3 provider's keys.
+ */
public final class LanceStorageOptions {
+ private static final Logger LOG =
LogManager.getLogger(LanceStorageOptions.class);
+
+ /**
+ * Doris backend property to Lance object-store option.
+ *
+ * <p>Lance reaches S3 through object_store, which accepts both {@code
access_key_id} and
+ * {@code aws_access_key_id}. The unprefixed spelling is chosen because it
is also the field
+ * name used by the OpenDAL backend, which performs no alias normalization
at all, so these
+ * options stay correct if that backend is ever selected.
+ */
private static final Map<String, String> S3_KEYS = new HashMap<>();
static {
- S3_KEYS.put("AWS_ACCESS_KEY", "aws_access_key_id");
- S3_KEYS.put("AWS_SECRET_KEY", "aws_secret_access_key");
- S3_KEYS.put("AWS_TOKEN", "aws_session_token");
- S3_KEYS.put("AWS_ENDPOINT", "aws_endpoint");
- S3_KEYS.put("AWS_REGION", "aws_region");
+ S3_KEYS.put("AWS_ACCESS_KEY", "access_key_id");
+ S3_KEYS.put("AWS_SECRET_KEY", "secret_access_key");
+ S3_KEYS.put("AWS_TOKEN", "session_token");
+ S3_KEYS.put("AWS_ENDPOINT", "endpoint");
+ S3_KEYS.put("AWS_REGION", "region");
}
+ /**
+ * Every spelling object_store accepts for the options above, mapped to
the one this class emits.
+ *
+ * <p>object_store resolves an alias and its canonical name to one config
key and keeps only one
+ * of the two values, chosen by hash order. So a namespace vending {@code
endpoint_url} while the
+ * catalog contributes {@code endpoint} does not override it - the two
survive as separate
+ * entries, and the FE and the BE can each end up using a different one.
Every accepted alias has
+ * to be recognized here, or that race simply moves to the spellings this
table misses.
+ */
+ private static final Map<String, String> CANONICAL_BY_ALIAS =
ImmutableMap.<String, String>builder()
+ .put("access_key_id", "access_key_id")
+ .put("aws_access_key_id", "access_key_id")
+ .put("secret_access_key", "secret_access_key")
+ .put("aws_secret_access_key", "secret_access_key")
+ .put("session_token", "session_token")
+ .put("aws_session_token", "session_token")
+ .put("aws_token", "session_token")
+ .put("token", "session_token")
+ .put("endpoint", "endpoint")
+ .put("endpoint_url", "endpoint")
+ .put("aws_endpoint", "endpoint")
+ .put("aws_endpoint_url", "endpoint")
+ .put("region", "region")
+ .put("aws_region", "region")
+ .put("virtual_hosted_style_request",
"virtual_hosted_style_request")
+ .put("aws_virtual_hosted_style_request",
"virtual_hosted_style_request")
Review Comment:
Agreed, fixed in ed170992de8. `enable_virtual_host_style` now resolves onto
the same entry as `virtual_hosted_style_request` /
`aws_virtual_hosted_style_request`, which OpenDAL's S3 config declares as serde
aliases of it (opendal-service-s3 0.56 `config.rs:173-177`, unchanged in 0.57).
Two of the three in one map is a duplicate field, and
`Operator::from_iter::<S3>` fails before anything is read.
Normalizing also makes the option effective on the object_store path, which
does not recognize that spelling at all.
Covered by `testOpendalVirtualHostStyleSpellingCollapsesToo`.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/lance/LanceStorageOptions.java:
##########
@@ -17,56 +17,171 @@
package org.apache.doris.datasource.lance;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Locale;
import java.util.Map;
+import java.util.Set;
-/** Converts normalized Doris storage properties to Lance object-store
options. */
+/**
+ * Builds the Lance object-store options for one dataset.
+ *
+ * <p>Both the FE, which opens the dataset through the Lance Java SDK, and the
BE, which opens it
+ * through lance-c, consume the map produced here, so the two cannot disagree
about how a dataset
+ * is accessed.
+ *
+ * <p>Options vended by a namespace are merged in as they arrive. The Lance
Namespace specification
+ * describes {@code storage_options} as configuration "passed directly to
Lance", so the protocol
+ * defines no key vocabulary of its own and a client cannot assume one.
Re-encoding those options
+ * into a fixed set of names would silently drop everything outside it,
including credentials
+ * spelled with a different accepted alias and every non-S3 provider's keys.
+ */
public final class LanceStorageOptions {
+ private static final Logger LOG =
LogManager.getLogger(LanceStorageOptions.class);
+
+ /**
+ * Doris backend property to Lance object-store option.
+ *
+ * <p>Lance reaches S3 through object_store, which accepts both {@code
access_key_id} and
+ * {@code aws_access_key_id}. The unprefixed spelling is chosen because it
is also the field
+ * name used by the OpenDAL backend, which performs no alias normalization
at all, so these
+ * options stay correct if that backend is ever selected.
+ */
private static final Map<String, String> S3_KEYS = new HashMap<>();
static {
- S3_KEYS.put("AWS_ACCESS_KEY", "aws_access_key_id");
- S3_KEYS.put("AWS_SECRET_KEY", "aws_secret_access_key");
- S3_KEYS.put("AWS_TOKEN", "aws_session_token");
- S3_KEYS.put("AWS_ENDPOINT", "aws_endpoint");
- S3_KEYS.put("AWS_REGION", "aws_region");
+ S3_KEYS.put("AWS_ACCESS_KEY", "access_key_id");
+ S3_KEYS.put("AWS_SECRET_KEY", "secret_access_key");
+ S3_KEYS.put("AWS_TOKEN", "session_token");
+ S3_KEYS.put("AWS_ENDPOINT", "endpoint");
+ S3_KEYS.put("AWS_REGION", "region");
}
+ /**
+ * Every spelling object_store accepts for the options above, mapped to
the one this class emits.
+ *
+ * <p>object_store resolves an alias and its canonical name to one config
key and keeps only one
+ * of the two values, chosen by hash order. So a namespace vending {@code
endpoint_url} while the
+ * catalog contributes {@code endpoint} does not override it - the two
survive as separate
+ * entries, and the FE and the BE can each end up using a different one.
Every accepted alias has
+ * to be recognized here, or that race simply moves to the spellings this
table misses.
+ */
+ private static final Map<String, String> CANONICAL_BY_ALIAS =
ImmutableMap.<String, String>builder()
+ .put("access_key_id", "access_key_id")
+ .put("aws_access_key_id", "access_key_id")
+ .put("secret_access_key", "secret_access_key")
+ .put("aws_secret_access_key", "secret_access_key")
+ .put("session_token", "session_token")
+ .put("aws_session_token", "session_token")
+ .put("aws_token", "session_token")
+ .put("token", "session_token")
+ .put("endpoint", "endpoint")
+ .put("endpoint_url", "endpoint")
+ .put("aws_endpoint", "endpoint")
+ .put("aws_endpoint_url", "endpoint")
+ .put("region", "region")
+ .put("aws_region", "region")
+ .put("virtual_hosted_style_request",
"virtual_hosted_style_request")
+ .put("aws_virtual_hosted_style_request",
"virtual_hosted_style_request")
+ .put("allow_http", "allow_http")
+ .put("aws_allow_http", "allow_http")
+ .build();
+
+ /**
+ * Aliases that supersede the catalog's value but keep the spelling the
namespace used.
+ *
+ * <p>{@code token} means an S3 session token to object_store's S3 parser
but a bearer token to
+ * its Azure one, and this class does not know which provider a dataset
uses. Renaming it would
+ * corrupt the Azure reading, so it is only used to decide which catalog
entry it replaces.
+ */
+ private static final Set<String> AMBIGUOUS_ALIASES =
ImmutableSet.of("token");
+
+ /**
+ * Options a namespace may not override, because they decide which data is
read rather than how
+ * it is accessed. Lance protects the same keys in the options it accepts
from a namespace.
+ */
+ private static final Set<String> PROTECTED_KEYS = ImmutableSet.of(
+ "bucket", "aws_bucket", "aws_bucket_name", "bucket_name", "root");
+
private LanceStorageOptions() {
}
- public static Map<String, String> forJavaSdk(Map<String, String>
backendProperties) {
+ /** Converts normalized Doris storage properties to Lance object-store
options. */
+ public static Map<String, String> toLanceOptions(Map<String, String>
backendProperties) {
Map<String, String> result = new HashMap<>();
S3_KEYS.forEach((dorisKey, lanceKey) -> putIfNotEmpty(result, lanceKey,
backendProperties.get(dorisKey)));
- String endpoint = backendProperties.get("AWS_ENDPOINT");
- if (endpoint != null && endpoint.startsWith("http://";)) {
- result.put("allow_http", "true");
- }
String usePathStyle = backendProperties.get("use_path_style");
if (usePathStyle != null && !usePathStyle.isEmpty()) {
- result.put("aws_virtual_hosted_style_request",
+ result.put("virtual_hosted_style_request",
String.valueOf(!Boolean.parseBoolean(usePathStyle)));
}
- return result;
+ return withDerivedAllowHttp(result);
}
- /** Merge Lance storage options returned by a namespace into properties
understood by Doris BE. */
- public static Map<String, String> forBackend(Map<String, String>
staticBackendProperties,
- Map<String, String> lanceStorageOptions) {
- Map<String, String> result = new HashMap<>(staticBackendProperties);
- if (lanceStorageOptions == null || lanceStorageOptions.isEmpty()) {
+ /**
+ * Merges the options a namespace vended for one table over the catalog's
own options.
+ *
+ * <p>Options a namespace may not override are dropped; everything else
replaces the catalog
+ * value, since the namespace decides how the table it just described is
reached.
+ */
+ public static Map<String, String> mergeVended(Map<String, String>
lanceOptions,
+ Map<String, String> vendedOptions) {
+ Map<String, String> result = new HashMap<>(lanceOptions);
+ if (vendedOptions == null || vendedOptions.isEmpty()) {
return result;
}
- S3_KEYS.forEach((dorisKey, lanceKey) -> putIfNotEmpty(result, dorisKey,
- lanceStorageOptions.get(lanceKey)));
- String virtualHostedStyle =
lanceStorageOptions.get("aws_virtual_hosted_style_request");
- if (virtualHostedStyle != null && !virtualHostedStyle.isEmpty()) {
- result.put("use_path_style",
String.valueOf(!Boolean.parseBoolean(virtualHostedStyle)));
+ Map<String, String> accepted = new HashMap<>();
+ Set<String> superseded = new HashSet<>();
+ vendedOptions.forEach((key, value) -> {
+ if (key == null || value == null || value.isEmpty()) {
+ return;
+ }
+ String lowerCased = key.toLowerCase(Locale.ROOT);
+ if (PROTECTED_KEYS.contains(lowerCased)) {
+ LOG.warn("Ignoring Lance storage option '{}' vended by the
namespace because it "
+ + "would change which data is read", key);
+ return;
+ }
+ String canonical = CANONICAL_BY_ALIAS.get(lowerCased);
+ if (canonical != null) {
+ superseded.add(canonical);
+ }
+ accepted.put(canonical != null &&
!AMBIGUOUS_ALIASES.contains(lowerCased)
Review Comment:
The version difference is real — I checked both lockfiles. lance-c 0.1.6 and
the Lance Java SDK 9.1.0-beta.3 pin the same object_store (0.13.2); only
OpenDAL differs, 0.56 against 0.57, and `skip_signature` is indeed a field only
0.57 has.
I do not think it is P1, or a defect in this PR:
- The default path is unaffected. Both sides run the same object_store; the
divergence is confined to the OpenDAL backend, which a namespace has to opt
into by vending `use_opendal=true`.
- "Align the provider versions" is not something this path can do — the pins
belong to lance-c and to the Lance Java SDK.
- Normalizing or rejecting options against a "common vocabulary" would mean
re-encoding server-supplied options into a fixed key set, which is precisely
the behaviour this PR removes, and the Lance Namespace spec describes
`storage_options` as passed directly to Lance.
What I have done is stop the PR from over-claiming. The class javadoc and
the description now say the FE and the BE agree on the option *map*, not on the
library that interprets it, and the residual skew is recorded under "Not in
scope".
##########
be/src/format_v2/table/lance_reader.cpp:
##########
@@ -894,36 +894,22 @@ Status
LanceTableReader::_fill_block_from_arrow(LanceBatch* batch, Block* block,
return Status::OK();
}
+// The FE sends these already in Lance's own vocabulary, merged from the
catalog properties and
+// from whatever the namespace vended. Re-encoding them here would drop every
option this list did
+// not anticipate, so they are handed to lance-c as they arrive.
std::vector<std::string> LanceTableReader::_storage_options(
const TFileScanRangeParams* scan_params) {
- if (scan_params == nullptr || !scan_params->__isset.properties) {
+ if (scan_params == nullptr || !scan_params->__isset.lance_storage_options)
{
return {};
}
- static constexpr std::array<std::pair<std::string_view, std::string_view>,
5> kStorageKeys = {
- {{"AWS_ACCESS_KEY", "aws_access_key_id"},
- {"AWS_SECRET_KEY", "aws_secret_access_key"},
- {"AWS_TOKEN", "aws_session_token"},
- {"AWS_ENDPOINT", "aws_endpoint"},
- {"AWS_REGION", "aws_region"}}};
std::vector<std::string> options;
- options.reserve(kStorageKeys.size() * 2);
- for (const auto& [doris_key, lance_key] : kStorageKeys) {
- const auto it = scan_params->properties.find(std::string(doris_key));
- if (it != scan_params->properties.end() && !it->second.empty()) {
- options.emplace_back(lance_key);
- options.emplace_back(it->second);
- }
- }
- const auto endpoint = scan_params->properties.find("AWS_ENDPOINT");
- if (endpoint != scan_params->properties.end() &&
endpoint->second.rfind("http://";, 0) == 0) {
- options.emplace_back("allow_http");
- options.emplace_back("true");
- }
- const auto path_style = scan_params->properties.find("use_path_style");
- if (path_style != scan_params->properties.end() &&
!path_style->second.empty()) {
- const bool use_path_style = path_style->second == "true" ||
path_style->second == "1";
- options.emplace_back("aws_virtual_hosted_style_request");
- options.emplace_back(use_path_style ? "false" : "true");
+ options.reserve(scan_params->lance_storage_options.size() * 2);
+ for (const auto& [key, value] : scan_params->lance_storage_options) {
+ if (value.empty()) {
+ continue;
+ }
+ options.emplace_back(key);
Review Comment:
The mechanism is real: JSON can escape U+0000 and Jackson accepts it,
Thrift's Java binding encodes U+0000 as a real `0x00`, and lance-c parses these
with `CStr::from_ptr` — so the FE and the BE would match on different keys.
Both ends now reject it (ed170992de8): `mergeVended` drops any key or value
containing a NUL, and `_storage_options` re-checks defensively for an FE that
predates the check. The offending key is named in the log line, with the NULs
rendered.
Two qualifications, though.
The severity assumes a hostile namespace server, and that is not a boundary
this check defends. The namespace already decides the dataset URI, so
`PROTECTED_KEYS` guards against confusion, not against an attacker.
And the concrete example does not hold. On the object_store path the bucket
is re-derived from the dataset URL in `build()`, so a NUL-truncated `bucket`
changes nothing; on the OpenDAL path lance overwrites `bucket` from the URL as
well (aws.rs:110). The reachable variant is a NUL-truncated `root` on the
OpenDAL path, where lance sets `root` only when the URL carries a non-empty
prefix (aws.rs:113-115). The guard is worth having — for that case.
--
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]
