Skip to content

Common: Fix SplitPath for percent-encoded separators in SAF-style URIs#14633

Open
jbripley wants to merge 1 commit into
dolphin-emu:masterfrom
jbripley:pr/android-saf-m3u-splitpath-single
Open

Common: Fix SplitPath for percent-encoded separators in SAF-style URIs#14633
jbripley wants to merge 1 commit into
dolphin-emu:masterfrom
jbripley:pr/android-saf-m3u-splitpath-single

Conversation

@jbripley
Copy link
Copy Markdown

@jbripley jbripley commented May 1, 2026

Summary

Fix SplitPath so it correctly handles paths/URIs where path separators are percent-encoded (for example %2F), which affects Android Storage Access Framework (SAF)-style content://.../document/... paths used when opening .m3u playlists.

Without this, SplitPath can derive an incorrect parent path from SAF document URIs, causing downstream path resolution failures (including .m3u entry lookups).

What changed

  • Updated Common::SplitPath separator scanning to treat percent-encoded bytes as decoded characters during separator detection.
  • Added a small local helper for decoding %xx bytes (std::optional<char>-based).
  • Kept behavior for normal filesystem paths unchanged.
  • Preserved Windows : separator behavior.
  • Added/updated unit tests for SAF-style URI path splitting in StringUtilTest.

Files

  • Source/Core/Common/StringUtil.cpp
  • Source/UnitTests/Common/StringUtilTest.cpp

Verification

Ran full unit tests:

  • ctest --test-dir build/Source/UnitTests --output-on-failure

Result: 100% tests passed, 0 tests failed.

@sepalani
Copy link
Copy Markdown
Contributor

sepalani commented May 1, 2026

I lack experience in Android development but I have some questions:

  1. What does this PR fix precisely, do you have examples of places in the Android codebase affected by this?
  2. Won't this change break on Linux if a filename has %2F in it?

IMHO, I don't think that's a good idea for the generic SplitPath function to perform URI-decoding under the hood. When special characters are URI-encoded it's precisely to allow them not to be processed as "being special" (e.g. / path separator). So it'd probably be better to fix the Android logic (maybe such helper already exist in the Android codebase) so it decodes the SAF URI properly, then handle the appropriate document path.

That said, I'm not well-versed in Android development so I'd wait for other reviewers comments with a better Android background on that matter. In case this function should support URI path on top of regular filepath, it should probably be renamed to something less confusing.

@JosJuice
Copy link
Copy Markdown
Member

JosJuice commented May 1, 2026

Does this actually let you usefully use m3u files? As I understand it, the only way we currently have in Dolphin to open an m3u file is by opening a single file, and when you open a single file, Android won't give you permission to access other files in the same folder. Unless you've previously added that folder to your game list so Dolphin has access to all files in it, but then why not just use the game list instead of manually opening an m3u file?

I should also mention that content providers are not guaranteed to encode a path in the URI like this, and indeed, many content providers don't. The one content provider that literally everyone uses does, but technically that's undocumented behavior that to me feels shaky to rely on.

I also share sepalani's concern about this breaking non-SAF paths that happen to contain %, which in theory can exist even on Android.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants