Skip to content

Removed encoding of request to retrieve files and folders by path, to avoid double encoding via the typed client#19457

Merged
iOvergaard merged 2 commits intorelease/16.0from
v16/bugfix/remove-double-encoding-on-request-by-path
Jun 2, 2025
Merged

Removed encoding of request to retrieve files and folders by path, to avoid double encoding via the typed client#19457
iOvergaard merged 2 commits intorelease/16.0from
v16/bugfix/remove-double-encoding-on-request-by-path

Conversation

@AndyButland
Copy link
Copy Markdown
Contributor

@AndyButland AndyButland commented Jun 2, 2025

Prerequisites

  • I have added steps to test this contribution in the description below

Fixes: #19446

Description

In testing we found that when running under IIS errors were thrown when requesting script, stylesheet and partial view files. Using Kestrel didn't show these issues. The requested path string was being double encoded - once in our own code and once automatically via the typed HTTP client. Seemingly the latter web server could handle this, but not the former.

Removing the double encoding by removing our initial encoding before the request is passed to the typed API client looks to resolve the issue.

Testing

Verify script, stylesheet and partial view files and folders can be created, retrieved, updated and deleted (I've done this on Windows, using both IISExpress and Kestrel).

… avoid double encoding via the typed client.
Copilot AI review requested due to automatic review settings June 2, 2025 05:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes manual encodeURIComponent calls on path parameters to prevent double encoding when using the typed HTTP client. It ensures that script, stylesheet, and partial-view paths are passed raw and rely on the client’s automatic encoding.

  • Removed encodeURIComponent(path) wrappers from all getByPath, putByPath, deleteByPath, and rename service calls.
  • Confirmed behavior works under IIS without path errors.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
stylesheet-folder.server.data-source.ts Removed manual encoding for folder retrieval/deletion paths
stylesheet-detail.server.data-source.ts Removed manual encoding for stylesheet CRUD paths
rename-stylesheet.server.data-source.ts Removed manual encoding for rename action
script-folder.server.data-source.ts Removed manual encoding for folder retrieval/deletion paths
script-detail.server.data-source.ts Removed manual encoding for script CRUD paths
rename-script.server.data-source.ts Removed manual encoding for rename action
partial-view-folder.server.data-source.ts Removed manual encoding for folder retrieval/deletion paths
partial-view-detail.server.data-source.ts Removed manual encoding for partial-view CRUD paths
rename-partial-view.server.data-source.ts Removed manual encoding for rename action
Comments suppressed due to low confidence (2)

src/Umbraco.Web.UI.Client/src/packages/templating/stylesheets/tree/folder/repository/stylesheet-folder.server.data-source.ts:55

  • Add unit or integration tests covering paths with special characters (spaces, symbols) to verify that removing manual encoding still yields correct HTTP requests.
path: { path },

src/Umbraco.Web.UI.Client/src/packages/templating/stylesheets/tree/folder/repository/stylesheet-folder.server.data-source.ts:55

  • [nitpick] The same change is repeated across many data sources; consider abstracting path parameter handling into a shared helper or base class to reduce duplication.
path: { path },

@iOvergaard iOvergaard self-assigned this Jun 2, 2025
Copy link
Copy Markdown
Contributor

@iOvergaard iOvergaard left a comment

Choose a reason for hiding this comment

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

This works great, but I see we have other such cases in the code base. I will spend a bit of time testing those out and applying the same fix if necessary.

@AndyButland AndyButland changed the title Removed encoding of request to retrieve files and folders by path, to avoid double encoding via the typed client. Removed encoding of request to retrieve files and folders by path, to avoid double encoding via the typed client Jun 2, 2025
Copy link
Copy Markdown
Contributor

@iOvergaard iOvergaard left a comment

Choose a reason for hiding this comment

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

Found a single thing in the log viewer, but was not directly related to the http client. I removed the extra encodeUriComponent. LGTM.

@iOvergaard iOvergaard enabled auto-merge (squash) June 2, 2025 10:09
@iOvergaard iOvergaard merged commit 11c6ea9 into release/16.0 Jun 2, 2025
23 checks passed
@iOvergaard iOvergaard deleted the v16/bugfix/remove-double-encoding-on-request-by-path branch June 2, 2025 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants