[task-inspector] Don't encoding slashes in artifact name for artifact URLs

RESOLVED FIXED

Status

Taskcluster
Tools
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: whimboo, Unassigned)

Tracking

Details

(Reporter)

Description

2 years ago
As noticed today via bug 1295948 (mozharness) the artifact URLs like on the following page are HTML encoded:

https://tools.taskcluster.net/task-inspector/#PnwWXZOuT8CFZ8TkL0G0CA/0

Link to test_packages.json:
https://queue.taskcluster.net/v1/task/PnwWXZOuT8CFZ8TkL0G0CA/runs/0/artifacts/public%2Fbuild%2Ftarget.test_packages.json

Some tools might not work that great with it. But I also wonder why we have to encode the slash at all. Jonas, do you know why this is done?
Flags: needinfo?(jopsen)
The url pattern is: /task/<taskId>/runs/<runId>/artifacts/<name>

Each of the variables are URI component encoded. Otherwise we couldn't have slash in them, unless they
are at the end of the URL.

But for artifacts specifically, we have allowed not escaping slashes in the <name> parameter.
And given that an artifact could be an HTML with relative links, I think that's a sane thing to support.

So yes, let's stop encoding these on the tools site. I think this is the only API end-point that doesn't
require slashes the last variable to be encoded.

But I agree that relative urls is nice to support, so let's keep supporting it, and stop encoding slashes on the tools site.
Component: Queue → Tools
Flags: needinfo?(jopsen)
Summary: Consider not encoding slashes for extra path (eg. public) in artifact URLs → [task-inspector] Don't encoding slashes in artifact name for artifact URLs
I talked to jonasf about doing this in taskcluster-treeherder as well.  I copy-paste-wget a lot of artifact links from TH, and it's frustrating that they don't get named well by wget.  This ticket could track fixing both issues, or another ticket could be opened for just the TH integration.

Comment 5

2 years ago
Commit pushed to master at https://github.com/taskcluster/taskcluster-tools

https://github.com/taskcluster/taskcluster-tools/commit/309a9e32c277f369188ef61bfc3c7c17d583b687
fix bug 1295949 for public artifacts (#143)

* fix bug 1295949 for public artifacts

* Addressed feedback from @dustin @eliperelman

Updated

2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.