fix(archive): sanitize target alias when writing file #668
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The target alias is currently included as-is in the destination filename when writing a JFR file to disk. This can be problematic since the target alias can contain characters like '/', for example `/deployments/quarkus-run.jar` in the quarkus-test sample application. The solution presented here is to URL encode the alias before including it in the destination filename, so the alias is transformed into ex. `%2Fdeployments%2Fquarkus-run.jar`.
jan-law
approved these changes
Aug 27, 2021
|
It's effectively the same, just using the image I pushed saves a bit of time building it. If you check out the PR branch and build it yourself you can verify that the image I pushed is really from the branch though :-) ---- On Fri, 27 Aug 2021 14:58:09 -0700 ***@***.*** wrote ----
@jan-law approved this pull request.
Works as expected!
One question - What's the difference between testing your branch with CRYOSTAT_IMAGE=quay.io/andrewazores/cryostat:sanitize-archive-alias sh smoketest.sh vs mvn clean package && sh smoketest.sh?
—You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android.
|
tthvo
pushed a commit
to tthvo/cryostat-legacy
that referenced
this pull request
Dec 9, 2024
…yostatio#668) Bumps [com.mycila:license-maven-plugin](https://github.com/mathieucarbou/license-maven-plugin) from 4.5 to 4.6. - [Release notes](https://github.com/mathieucarbou/license-maven-plugin/releases) - [Commits](mathieucarbou/license-maven-plugin@license-maven-plugin-4.5...license-maven-plugin-4.6) --- updated-dependencies: - dependency-name: com.mycila:license-maven-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #665
The target alias is currently included as-is in the destination filename when writing a JFR file to disk. This can be problematic since the target alias can contain characters like '/', for example
/deployments/quarkus-run.jarin the quarkus-test sample application. The solution presented here is to URL encode the alias before including it in the destination filename, so the alias is transformed into ex.%2Fdeployments%2Fquarkus-run.jar.To test:
Check the current behaviour upstream with
sh smoketest.sh. Create a new recording on the quarkus-test application, then attempt to archive this recording. The request will fail with an AccessDeniedException.Then retry this procedure but starting from
CRYOSTAT_IMAGE=quay.io/andrewazores/cryostat:sanitize-archive-alias sh smoketest.sh. The recording should be successfully archived with the alias encoded. Archiving recordings on the other targets, which already have "safe" aliases, should produce the same resultant archive filename as before.