apache / apache/datafusion

ObjectStoreRegistry key strips URL userinfo, causing ABFS containers on the same account to collide

Open
#23,926 2 comments 0 reactions 1 assignee Claimed by @847850277 View on GitHub
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

## Describe the bug

`get_url_key` in `datafusion-execution/src/object_store.rs` builds the registry key from `Position::BeforeHost..Position::AfterPort`, which excludes URL userinfo:

```rust
/// Get the key of a url for object store registration.
/// The credential info will be removed
fn get_url_key(url: &Url) -> String {
format!(
"{}://{}",
url.scheme(),
&url[url::Position::BeforeHost..url::Position::AfterPort],
)
}
```

The doc comment describes this as removing "credential info," which is accurate for HTTP-style basic auth (`user:pass@host`). It is not accurate for ABFS. Azure Data Lake Storage Gen2 URLs encode the storage container as the URL userinfo:

```
abfss://@.dfs.core.windows.net/
```

`object_store::azure::MicrosoftAzureBuilder::parse_url` reads the container from the userinfo and binds it into the resulting store instance. The resource `Path` is then container-relative. So `abfss://c1@acct.dfs.core.windows.net/...` and `abfss://c2@acct.dfs.core.windows.net/...` are two different stores, but `get_url_key` reduces both to `abfss://acct.dfs.core.windows.net`.

The consequence within a single `RuntimeEnv`: registering the `c2` store overwrites the `c1` store, and any subsequent lookup for `c1` returns the `c2` store, which reads from the wrong container.

## To Reproduce

Register two Azure stores for different containers on the same account against one `RuntimeEnv`:

```rust
let rt = RuntimeEnv::default();
let url_c1 = Url::parse("abfss://c1@acct.dfs.core.windows.net/").unwrap();
let url_c2 = Url::parse("abfss://c2@acct.dfs.core.windows.net/").unwrap();
rt.register_object_store(&url_c1, store_c1);
rt.register_object_store(&url_c2, store_c2);
// rt.object_store(&url_c1) now returns store_c2
```

## Expected behavior

Two Azure containers on the same account should be addressable as two distinct stores from one `RuntimeEnv`.

## Additional context

This was noticed while reviewing a Comet fix for the same class of bug in a Comet-owned process-wide store cache (apache/datafusion-comet#4993 / apache/datafusion-comet#5053). Comet dodges the DataFusion-layer issue by constructing a fresh `RuntimeEnv` per Parquet file read, so today the DF layer never sees two ABFS containers registered on one env. That workaround is fragile — any refactor that shares a `RuntimeEnv` across container reads reintroduces the bug at the DF layer.

Two possible directions, roughly in preference order:

1. **Scheme-aware key derivation.** For schemes where userinfo is a namespace rather than a credential (`abfs`, `abfss`), include the userinfo in the key. For other schemes, keep the current behavior so existing basic-auth callers are unaffected.
2. **Include userinfo unconditionally.** Simpler and arguably more correct — treating userinfo as identity rather than credential in a *cache key* is safe (the key is not logged, and two callers with different credentials for the same host arguably *should* get different stores anyway). This is a semantic change and would need a migration note.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.