Closed Bug 1157699 Opened 9 years ago Closed 6 years ago

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

Categories

(Tree Management :: Treeherder, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: emorley, Unassigned)

References

Details

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
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.
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
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.