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

cdnjs: Support handling of resources with embedded slashes afte e6b5e0d #7796

Merged
merged 2 commits into from
Sep 27, 2024

Conversation

matthiasblaesing
Copy link
Contributor

With commit e6b5e0d java.io.File.createTempFile was replaced by
java.nio.file.Files.createTempFile. The latter has better security
properties, but fails to handle prefixes/suffixes, that contain
slashes and instead raises an exception.

The temporary files don't need files, that follow the names of their
base resources, so with this commit the tempfiles are created with
fixed prefixes and suffixes.

Closes: #7783

With commit e6b5e0d java.io.File.createTempFile was replaced by
java.nio.file.Files.createTempFile. The latter has better security
properties, but fails to handle prefixes/suffixes, that contain
slashes and instead raises an exception.

The temporary files don't need files, that follow the names of their
base resources, so with this commit the tempfiles are created with
fixed prefixes and suffixes.

Closes: apache#7783
@matthiasblaesing matthiasblaesing added JavaScript [ci] enable web job and extra JavaScript tests (webcommon/javascript2.editor) ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Sep 25, 2024
@matthiasblaesing matthiasblaesing added this to the NB24 milestone Sep 25, 2024
Copy link
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

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

code looks good, sorry for the trouble

I believe the reason why it used to work is this:
https://github.com/openjdk/jdk/blob/a02d895f7ad59fe33f8a761dbd7bceb0b8dfefc0/src/java.base/share/classes/java/io/File.java#L2044-L2045
its an impl detail (possibly security fix?) which essentially drops everything except the filename, so if you add a path as prefix it is ignored.

so File.createTempFile("a/b", "hugo") becomes b2136422930822303311hugo (also interesting that it requires a 3+ char prefix, but the example above reduces it to 1)

@matthiasblaesing
Copy link
Contributor Author

@mbien thanks for checking

@matthiasblaesing matthiasblaesing merged commit 004d754 into apache:master Sep 27, 2024
31 checks passed
@matthiasblaesing matthiasblaesing deleted the slash_download_cdnjs branch October 2, 2024 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) JavaScript [ci] enable web job and extra JavaScript tests (webcommon/javascript2.editor)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CDNJS update library throws exception
2 participants