Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FolderSchemeHandlerFactory Handle URL encoded paths only in absolute path #5052

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jbohunovsky-IDG
Copy link

@jbohunovsky-IDG jbohunovsky-IDG commented Feb 12, 2025

Related to pull request #2364

Summary:
The original fix can potentially lead to non-existing root path. This PR fixes the issue when the rootFolder has "escaped" URL characters even thought that's how the folder is named (e.g. hello%2Fworld folder would try to lookup hello/world folder which doesn't exist).

Changes:

  • Moved WebUtility.UrlDecode from the full path to absolute path only.

How Has This Been Tested?

  1. I've used CefSharp.Wpf.Example.netcore project, where I've added new custom scheme with FolderSchemeHandlerFactory, with the following parameters:
    • rootFolder: "escaped%2Fslash"
    • schemeName: "app"
    • hostName: "test"
  2. Then I've created folder escaped%2Fslash in the output directory and created two simple HTML files, one called index.html and the other file with spaces.html.
  3. Run the project and navigate to app://test/index.html and app://test/file%20with%20spaces.html. Both files have been loaded correctly.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Updated documentation

Checklist:

  • Tested the code(if applicable)
  • Commented my code
  • Changed the documentation(if applicable)
  • New files have a license disclaimer
  • The formatting is consistent with the project (project supports .editorconfig)

@AppVeyorBot
Copy link

@amaitland
Copy link
Member

Thanks for the PR!

Would this be a breaking change? Would there be an impact on upgrading users?

@jbohunovsky-IDG
Copy link
Author

I don't believe this is a breaking change, as the absolute path (URL address path) is untouched and the root folder (defined by the developer) already has a check that it exists, so if someone had URL encoded root folder, they would get an error everytime upon accessing any path within that folder (as decoding that path would lead to a different path than which was defined by the developer), which is what this PR is fixing.

@amaitland
Copy link
Member

so if someone had URL encoded root folder, they would get an error everytime upon accessing any path within that folder

Is that not a breaking change? You'd only find this out by running the application and accessing the paths.

This will need to be documented so upgrading users are at least aware of the change.

@jbohunovsky-IDG
Copy link
Author

jbohunovsky-IDG commented Feb 14, 2025

That's the current behaviour. If you set the root folder to example%20folder, it will pass the Directory.Exists check in the constructor, but then in the Create method the full path will be URL decoded, effectively changing the root folder to example folder which doesn't exist. This PR fixes that.

Users will not be affected if they don't use URL encoded root folder, but if they do, it's not working properly for them right now anyways.
So I don't think anyone is going to be affected by this, but I do agree this change should be documented just in case.

@AppVeyorBot
Copy link

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