Skip to content

[Storage] Fix double encoding of path names on Datalake clients - #50083

Draft
Isabelle (ibrandes) wants to merge 2 commits into
Azure:mainfrom
ibrandes:bugfix/storage/datalakeDoubleEncodingFix
Draft

[Storage] Fix double encoding of path names on Datalake clients #50083
Isabelle (ibrandes) wants to merge 2 commits into
Azure:mainfrom
ibrandes:bugfix/storage/datalakeDoubleEncodingFix

Conversation

@ibrandes

Copy link
Copy Markdown
Member

Fix double-encoding of path names on clients returned by rename / undeletePath, and correct stale encoding javadoc

Addresses #48040

Two related fixes to azure-storage-file-datalake, both surfaced while investigating a customer question about which special characters callers must percent-encode before passing a path name to the SDK:

  1. Bug fix — the DataLakeFileClient / DataLakeDirectoryClient (and async equivalents) returned by rename, renameWithResponse, and undeletePath carried a URL-encoded path name, so subsequent Data Lake endpoint requests made through that client targeted a double-encoded path.
  2. Documentation fix — 25 javadoc sites still instructed callers to pre-encode path names. That guidance has been wrong since 12.22.0 and actively causes the double-encoding it claims to prevent.

Reason for the change

The bug

DataLakeFileClient(DataLakePathClient), DataLakeDirectoryClient(DataLakePathClient), DataLakeFileAsyncClient(DataLakePathAsyncClient) and DataLakeDirectoryAsyncClient(DataLakePathAsyncClient) each applied Utility.urlEncode(pathName) when copying the parent client's state.

This was correct at the time it was written. DataLakePathAsyncClient used to store Utility.urlDecode(pathName), so the encode simply round-tripped the name back to its stored form. Commit 4316086b1d0 (blob, 12.19.0) and b93671528c7 (datalake, 12.22.0) removed the decode — both shipped as documented breaking changes — but the matching encode in the copy constructors was left behind, turning a round-trip into a double-encode.

The resulting client was internally inconsistent, which is why this went unnoticed:

  • Blob-backed operations (getProperties, delete, …) reuse the parent's BlockBlobClient, which was built from the raw name → correct URL.
  • Data Lake-backed operations (create, setAccessControl, …) and getFileUrl() / getDirectoryUrl() use the re-encoded pathName → double-encoded URL.

Measured behaviour before the fix, renaming to dest%20%25:

Call URL
the rename request itself /fs/dest%2520%2525
renamed.getProperties() (blob endpoint) /fs/dest%2520%2525
renamed.create() (dfs endpoint) /fs/dest%252520%252525
renamed.getFileUrl() .../dest%252520%252525

The existing recorded test FileApiTest.renameUrlEncoded passes despite the bug because it only asserts getPropertiesWithResponse — the blob-endpoint half.

I verified every call site of these four constructors (rename in the four path clients, undeletePath in the two file system clients). In all six the source pathName is the raw, user-supplied name, so dropping the encode is correct everywhere.

The javadoc

DataLakePathAsyncClient stores pathName verbatim and never decodes it. Encoding happens once, at request-build time: PathsImpl declares @PathParam("path") without encoded = true, so SwaggerMethodParser applies UrlEscapers.PATH_ESCAPER. Anything a caller pre-encodes therefore gets escaped a second time — the % in %20 becomes %25.

The javadoc on DataLakeFileSystemClient.getFileClient / getDirectoryClient / createDirectory* / deleteDirectory* (and async equivalents) and on DataLakePathClientBuilder.pathName(String) still said:

If the path name contains special characters, pass in the url encoded version of the path name.

That is the pre-12.22.0 contract and is now precisely backwards.

Changes

emoved the stale Utility.urlEncode(...) from the four copy constructors: DataLakeFileClient.java, DataLakeDirectoryClient.java, DataLakeFileAsyncClient.java and DataLakeDirectoryAsyncClient.java.

Javadoc fix — 25 occurrences replaced with:

Pass the name unencoded; the client percent-encodes it when building the request URL.

12 in DataLakeFileSystemClient, 12 in DataLakeFileSystemAsyncClient, plus DataLakePathClientBuilder.pathName(String).

Deliberately not changed: DataLakePathClientBuilder.endpoint(String) javadoc, which says a path embedded in the endpoint URL must be url-encoded. That one is correct — endpoint parsing goes through BlobUrlParts.getBlobName(), which calls Utility.urlDecode (BlobUrlParts.java:152). Only the separate pathName(String) setter, which stores verbatim, was stale.

New testsDataLakePathNameEncodingTests.java (406 lines, 119 tests). Fully offline; uses a request-capturing mock HttpClient to assert what the SDK actually puts on the wire rather than what it's documented to do. Covers:

  • path names are stored verbatim and never decoded
  • raw % and # no longer throw IllegalArgumentException: Illegal hex characters in escape (%) pattern
  • the full wire-encoding table: safe set A–Z a–z 0–9 - . _ ~ ! $ & ' ( ) * + , ; = : @, encoded set (space) " # % / < > ? [ ] ^ \ { | } ` and non-ASCII
  • the Windows-reserved characters the customer asked about (" \ / : | < > * ?)
  • pre-encoded input is double-encoded (the anti-pattern)
  • x-ms-rename-source uses Utility.urlEncode while the URL path uses UrlEscapers.PATH_ESCAPER — two different but individually valid encodings
  • regression guards for this fix: the client returned by rename / undeletePath addresses the same URL from both the blob and dfs code paths, and agrees with a freshly acquired client

CHANGELOG.md — entries under 12.29.0-beta.2 → Bugs Fixed and Other Changes.

@github-actions github-actions Bot added the Storage Storage Service (Queues, Blobs, Files) label Aug 10, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
34 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a long-standing path-name encoding inconsistency in azure-storage-file-datalake where clients returned from rename / renameWithResponse / undeletePath could double-encode the path name on subsequent DFS calls, and updates stale JavaDoc that incorrectly told customers to pre-encode path names. Adds a comprehensive offline test suite that asserts the exact on-the-wire encoding behavior, plus a CHANGELOG entry.

Changes:

  • Remove stale Utility.urlEncode(...) usage from DataLakeFile* / DataLakeDirectory* copy constructors to prevent double-encoding on returned clients.
  • Update JavaDoc across DataLakeFileSystemClient / DataLakeFileSystemAsyncClient and DataLakePathClientBuilder.pathName(String) to instruct callers to pass unencoded names.
  • Add DataLakePathNameEncodingTests to lock down encoding behavior and prevent regressions; update CHANGELOG.md.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/DataLakePathNameEncodingTests.java New offline test suite asserting verbatim storage + request-time percent-encoding behavior, including rename/undelete regressions.
sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathClientBuilder.java JavaDoc update to clarify callers must pass unencoded path names.
sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeFileSystemClient.java JavaDoc updates to remove incorrect “pre-encode path” guidance.
sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeFileSystemAsyncClient.java Async JavaDoc updates mirroring sync guidance on raw path names.
sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeFileClient.java Stop re-encoding pathName when constructing returned file clients.
sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeFileAsyncClient.java Stop re-encoding pathName when constructing returned async file clients (and remove now-unused import).
sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeDirectoryClient.java Stop re-encoding pathName when constructing returned directory clients (and remove now-unused import).
sdk/storage/azure-storage-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakeDirectoryAsyncClient.java Stop re-encoding pathName when constructing returned async directory clients (and remove now-unused import).
sdk/storage/azure-storage-file-datalake/CHANGELOG.md Document the bug fix + JavaDoc correction under 12.29.0-beta.2 (Unreleased).
Suppressed comments (1)

sdk/storage/azure-storage-file-datalake/src/test/java/com/azure/storage/file/datalake/DataLakePathNameEncodingTests.java:232

  • This Stream.of(...) is currently written as a long single statement across a couple of very long lines, which is likely to trip the 120-character Checkstyle LineLength rule. Please wrap it with one Arguments.of(...) per line for readability and to keep line lengths under the limit.
    private static Stream<Arguments> windowsReservedCharacterSupplier() {
        return Stream.of(Arguments.of("a\"b", "a%22b"), Arguments.of("a\\b", "a%5Cb"), Arguments.of("a/b", "a%2Fb"),
            Arguments.of("a:b", "a:b"), Arguments.of("a|b", "a%7Cb"), Arguments.of("a<b", "a%3Cb"),
            Arguments.of("a>b", "a%3Eb"), Arguments.of("a*b", "a*b"), Arguments.of("a?b", "a%3Fb"));

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@ibrandes Isabelle (ibrandes) changed the title javadoc edits, removing double encode, adding test class [Storage] Fix double encoding of path names on Datalake clients Aug 10, 2026
@ibrandes
Isabelle (ibrandes) requested a balanced review from Copilot August 10, 2026 22:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

dataLakePathClient.getServiceVersion(), dataLakePathClient.getAccountName(),
dataLakePathClient.getFileSystemName(), Utility.urlEncode(dataLakePathClient.pathName),
PathResourceType.DIRECTORY, dataLakePathClient.getSasToken(), dataLakePathClient.getCpkInfo(),
dataLakePathClient.getFileSystemName(), dataLakePathClient.pathName, PathResourceType.DIRECTORY,
dataLakePathClient.getServiceVersion(), dataLakePathClient.getAccountName(),
dataLakePathClient.getFileSystemName(), Utility.urlEncode(dataLakePathClient.pathName),
PathResourceType.FILE, dataLakePathClient.getSasToken(), dataLakePathClient.getCpkInfo(),
dataLakePathClient.getFileSystemName(), dataLakePathClient.pathName, PathResourceType.FILE,
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants