Skip to content

Fix escaping of file names for song URIs#52

Open
aspettl wants to merge 5 commits intoBoddlnagg:masterfrom
aspettl:fix-songs-uri-escaping
Open

Fix escaping of file names for song URIs#52
aspettl wants to merge 5 commits intoBoddlnagg:masterfrom
aspettl:fix-songs-uri-escaping

Conversation

@aspettl
Copy link
Contributor

@aspettl aspettl commented Dec 22, 2025

When song file names use characters that need to be escaped in URIs, then there can be problems. An example is the character # that is automatically removed with everything behind it.

  • internal representation of song:// URIs needs escaped file names (this was inconsistent so far)
  • in the .ppp portfolio files, the file name is stored unescaped (as before)

@aspettl
Copy link
Contributor Author

aspettl commented Dec 22, 2025

I just noticed that EscapeDataString also escapes the / in case of subfolders, which I don't like. (Unfortunately just using EscapeUriString is not enough, it does not fix the actual problem with the #.)

I guess I'd need to escape every segment separately for a nice URI (or use the .NET URI class correctly everywhere starting from file:// URIs), but need to see if that is worth it...

@aspettl aspettl marked this pull request as draft December 22, 2025 10:37
@aspettl aspettl marked this pull request as ready for review December 22, 2025 13:39
@aspettl
Copy link
Contributor Author

aspettl commented Dec 22, 2025

I extracted now methods for handling song URIs and put the encoding logic in there.

/// <returns>The "song://" URI with encoded special characters if needed.</returns>
public static Uri GetUri(string filename)
{
var escapedFilename = string.Join("/", filename.Split('/', '\\').Select(Uri.EscapeDataString));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment the # is the only problematic character that I know - compared to just using EscapeUriString (considering the character limitations of Windows file names). So probably it would also be sufficient to use EscapeUriString plus one additional Replace("#", "%23").

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant