Display an error if unexpected length values are used for the 'revision' parameter

RESOLVED WORKSFORME

Status

P3
normal
RESOLVED WORKSFORME
3 years ago
9 months ago

People

(Reporter: emorley, Unassigned)

Tracking

Details

(Reporter)

Description

3 years ago
In bug 1156025, a character was missed off the end of the URL.

ie instead of the correct URL:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8b7892dd4e7

This was used:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8b7892dd4e

Which results in the following API call:
https://treeherder.mozilla.org/api/project/try/resultset/?count=10&format=json&full=true&revision=c8b7892dd4e&with_jobs=false

This is problematic, since whilst treeherder does exact string matching of 'revision' against the revision in the database, when the user clicks the "pushlog" link, they get taken to:
https://hg.mozilla.org/try/pushloghtml?changeset=c8b7892dd4e
...which still returns results, since hg treats it as a revset (or whatever). You can even pass in branch names or similar to hgweb and it works fine.

It's not practical to re-invent hg revision handling within Treeherder, but we should at least make it more obvious to the user that Treeherder expects a specific revision SHA. ie: if the provided value is not of the correct length, both the API and the UI should display errors.

So to fix this bug, the /api/ URL above should give a 401 response and appropriate content, and the UI should display an error page unlike vaguely similar lines to the invalid repo/... ones.

https://github.com/mozilla/treeherder-ui/blob/16a72b1da5f00245c0f64954a155ceef5d13c9be/webapp/app/partials/main/jobs.html#L54-L90

https://github.com/mozilla/treeherder/blob/bd60d36364491789085cad44c0dd5c799ecf993b/treeherder/webapp/api/revision.py#L17-L21
(Reporter)

Comment 1

3 years ago
s/unlike/like/

Also we need to watch out for any complications arising from long vs short form SHAs (I think we just use the short form at present) and also future proof for when we start supporting Git.
(Reporter)

Comment 2

3 years ago
Making this depend on bug 1079796, since the error handling in the UI will change once we support both long and short hashes.
Depends on: 1079796
(Reporter)

Updated

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