Percent encode artifact names in fetch-content#927
Open
Eijebong wants to merge 1 commit intotaskcluster:mainfrom
Open
Percent encode artifact names in fetch-content#927Eijebong wants to merge 1 commit intotaskcluster:mainfrom
Eijebong wants to merge 1 commit intotaskcluster:mainfrom
Conversation
Previously, trying to use fetch-content for an artifact that had a space
in it would result in errors like `Download failed: URL can't contain
control characters. '[...]' (found at least ' ')`
This commit changes fetch-content so we encode the artifact name when
building the download URL instead of putting it verbatim in the URL.
Because we're later reusing the URL to get the base name to know which
file to write, we now decode the basename so the resulting file doesn't
contain percent encoded characters.
Before:
```
$ export MOZ_FETCHES='[{"task":"FNpL6lwoTVyuWE3wgNeuUw","artifact":"public/Twilight Princess-0.2.3.apworld","extract":false}]'
$ uv run python3 src/taskgraph/run-task/fetch-content task-artifacts -d /tmp/fetch-test
attempt 1/5
Downloading https://taskcluster.bananium.fr/api/queue/v1/task/DBFKDCtCSi6gg35xPqZHOA/artifacts/public/Twilight Princess-0.3.0.apworld to /tmp/fetch-test/Twilight Princess-0.3.0.apworld
Downloading https://taskcluster.bananium.fr/api/queue/v1/task/DBFKDCtCSi6gg35xPqZHOA/artifacts/public/Twilight Princess-0.3.0.apworld
Download failed: URL can't contain control characters. '/api/queue/v1/task/DBFKDCtCSi6gg35xPqZHOA/artifacts/public/Twilight Princess-0.3.0.apworld' (found at least ' ')
sleeping for 60.00s (attempt 1/5)
```
After:
```
$ export MOZ_FETCHES='[{"task":"FNpL6lwoTVyuWE3wgNeuUw","artifact":"public/Twilight Princess-0.2.3.apworld","extract":false}]'
$ uv run python3 src/taskgraph/run-task/fetch-content task-artifacts -d /tmp/fetch-test
attempt 1/5
Downloading https://taskcluster.bananium.fr/api/queue/v1/task/DBFKDCtCSi6gg35xPqZHOA/artifacts/public/Twilight%20Princess-0.3.0.apworld to /tmp/fetch-test/Twilight Princess-0.3.0.apworld
Downloading https://taskcluster.bananium.fr/api/queue/v1/task/DBFKDCtCSi6gg35xPqZHOA/artifacts/public/Twilight%20Princess-0.3.0.apworld
https://taskcluster.bananium.fr/api/queue/v1/task/DBFKDCtCSi6gg35xPqZHOA/artifacts/public/Twilight%20Princess-0.3.0.apworld resolved to 155321 bytes with sha256 77c8ccca3da7b040b94a7c8b4c68dc5234556dbc1c5b51f65afc19655f6d00ab in 0.486s
PERFHERDER_DATA: {"framework": {"name": "build_metrics"}, "suites": [{"name": "fetch_content", "value": 0.4895111569785513, "lowerIsBetter": true, "shouldAlert": false, "subtests": []}]}
```
As another workaround, users could percent encode the artifact name in
the fetch config but that would result with the filename on disk still
having the percent encoded characters in it and be confusing because the
artifact in the config wouldn't match what the task actually has.
i.e: MOZ_FETCHES='[{"task":"FNpL6lwoTVyuWE3wgNeuUw","artifact":"public/Twilight%20Princess-0.2.3.apworld","extract":false}]'
Eijebong
force-pushed
the
percent-encode-fetch-content
branch
from
593e099 to
711493e
Compare
Contributor
|
Isn't this backwards-incompatible, i.e. |
Eijebong
added
the
BREAKING CHANGE
Contributor
Author
|
True. Slapped a breaking change label on this. I don't expect anyone to run into this because I would hope that if someone had seen this before they'd had opened an issue but it is a breaking change |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Previously, trying to use fetch-content for an artifact that had a space in it would result in errors like
Download failed: URL can't contain control characters. '[...]' (found at least ' ')This commit changes fetch-content so we encode the artifact name when building the download URL instead of putting it verbatim in the URL. Because we're later reusing the URL to get the base name to know which file to write, we now decode the basename so the resulting file doesn't contain percent encoded characters.
Before:
After:
As another workaround, users could percent encode the artifact name in the fetch config (i.e:
MOZ_FETCHES='[{"task":"FNpL6lwoTVyuWE3wgNeuUw","artifact":"public/Twilight%20Princess-0.2.3.apworld","extract":false}]') but that would result with the filename on disk still having the percent encoded characters in it and be confusing because the artifact in the config wouldn't match what the task actually has.