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

Encode filepaths to be safe for Windows #489

Merged
merged 2 commits into from May 13, 2024

Conversation

bioball
Copy link
Contributor

@bioball bioball commented May 10, 2024

Also: don't URI-encode parentheses in pkldoc.

Implementation for SPICE-0003

Copy link
Contributor

@stackoverflow stackoverflow left a comment

Choose a reason for hiding this comment

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

Looks good.

return sb.toString();
}

/** The inverse of {@link #encodePath(String)} */
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically, this function is not really the inverse of encodePath, as it decodes anything, not only the chars that encodePath encodes. So one can decode (66)(6f)(6f).text as foo.txt which cannot be generated by encodePath. But I'm being pedantic and I think it's fine that it decodes anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Although, I'm actually getting rid of this altogether; this method isn't used anywhere.

// https://stackoverflow.com/questions/2678551/when-should-space-be-encoded-to-plus-or-20#:~:text=%20%20is%20a%20valid%20way,encodeURIComponent()%20does%20in%20JavaScript.)
return ret.replace("+", "%20")
val ret = URI(null, null, this, null)
return ret.toString().replace("/", "%2F")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the change here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous implementation was encoding parentheses in percent encoding.

Only encode characters that are strictly necessary in URI paths.

This removes encoding for parentheses, for example.
@bioball bioball force-pushed the windows-safe-paths branch 3 times, most recently from 63d4986 to 1ee4e2a Compare May 13, 2024 20:48
This changes the file paths to use characters that are safe for Windows.

Channges the output of the following:
* Package cache directory
* Generated pkl-doc files
* Kotlin generated code

Unsafe characters are encoded as (<hex>).
For example, the colon character `:` is encoded as `(3a)`.

Additionally, this changes the cache directory prefix (package-1 to
package-2).

Follows the design of apple/pkl-evolution#3
@bioball bioball merged commit a5c13e3 into apple:main May 13, 2024
4 checks passed
@bioball bioball deleted the windows-safe-paths branch May 13, 2024 21:06
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.

None yet

2 participants