Repository navigation
fix: keep URL passwords and query strings out of rejected path reasons - #7565
Draft
dwsmith1983 wants to merge 30 commits into
Draft
dwsmith1983 wants to merge 30 commits into
dwsmith1983 wants to merge 30 commits into
Conversation
… logs The native Azure store printed whole URLs in its errors and debug lines, and handed them to object_store, whose own errors quote the URL. A password in the userinfo or a SAS token in the query could end up in task errors and logs. object_store now gets a copy without the password, query and fragment, keeping the container it reads from the username. Messages name the URL by scheme, container and host only.
… driver messages object_store_authority kept an ABFS URL's password, so it reached the store cache key, the registration URL, debug lines and the deregister_store error. object_store_backend parsed the raw URL and normalize_object_store_url echoed input it could not parse. On the driver, AbfsAuthResolver put the URI into forwarded errors and a warning, and the mixed-store decline reason and objectStoreKey kept the raw authority. All of these now name a URL by scheme, container and host only. The URL helpers move to objectstore/url_redaction.rs, NativeConfig gains a matching displayAuthority, objectStoreKey matches the native key again, and the resolver scrubs the URI's password and query from Hadoop messages it forwards. Credentials only come from configuration, so URLs that differ only by password share a store.
The message no longer needs the host, so the redacted helper that built it is removed.
Drop comments that restate the code or a test's name, and shorten the rest to why each part of a URL is kept or removed.
Native now builds the key callback path from the authority without its password, so the retriever registered under the listed path was never found. Register and look up retrievers by one key: S3 aliases folded to s3a, and for abfs/abfss the scheme lowercased and the password dropped by the same rule as native. The missing-retriever error prints that key.
Secrets were replaced one after another, so a URI password inside a configured credential was masked first and the rest of the credential stayed in the message. Match every secret in the same text and replace each run of masked characters with one marker.
The object store key is built per file during fallback checks and again per split in partition packing. Return the raw authority as is when it has no `@`, or no `:` before the last `@`, instead of rebuilding the same string.
…layAuthority nativeFallbackReason stripped the URL password with its own copy of the logic in NativeConfig.authorityWithoutPassword. Use the shared helpers so the resolver renders a location one way in every message: lower-case scheme, no password, path or query.
The ABFS resolver skipped secrets under eight characters, so a short password in the URI reached the driver warning when Hadoop named the container-scoped keystore alias it could not recover. URI secrets are now hidden at any length; the floor applies only to configured values. The password is read from the raw authority, split as Hadoop splits the container, so a registry-based authority (an underscore in the host) no longer loses it.
When object_store cannot open a path, the scan falls back to Spark with a reason that names the path. The native Parquet scan printed the whole URI there, and the native Delta scan masked the user info but kept the query, so a password, a SAS token or a signature reached the plan and the fallback report. NativeConfig.displayPath shows a URI without its password, query or fragment and masks the username as ***, keeping an ABFS username, which names the container. It reads the user info from the raw authority, so registry based hosts are covered. A URI with nothing to hide reads as before, and an opaque one shows only its scheme. Both reasons use it.
This was referenced Nov 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #7128.
This is the last of four pull requests for #7128, each covering one area. They build on #7127, which adds the URL redaction helpers they use, so #7127 merges first. Suggested order after it:
They target
main, so until #7127 merges each one also shows its commits.Rationale for this change
When object_store cannot open a path, the scan falls back to Spark with a reason that names the path. The native Parquet scan printed the whole URI there, and the native Delta scan masked the user info but kept the query, so a password or a SAS token or signature showed up in the plan and the fallback report.
What changes are included in this PR?
NativeConfig.displayPathshows a URI without its password, query or fragment and masks a username as***, as Delta's existing reasons do. An ABFS username names the container, so it stays. The user info is read from the raw authority, so a host Java treats as registry based, such as one with an underscore, is covered too.file:/paths and the%0Athat explains why the path was rejected. An opaque URI shows only its scheme.branch-1.2prints the whole URI in the native Parquet reason, so that part is a candidate for a backport. A?that Hadoop folded into a path is part of the path and stays, as in the other three pull requests.How are these changes tested?
NativeConfigSuitecovers each shape with a password, a username and a signature query, the ABFS container, a registry based host, an opaque URI, and paths that must stay as they are.CometMultiStoreScanSuitechecks the Parquet fallback reason end to end for an S3 and an ABFS root, andDeltaScanContribSuitechecks the Delta reason drops the query and fragment. Each new test fails on the base branch. The suites pass on Spark 3.5, 4.0 and 4.1.