Bug 254336
| Summary: | WebKit Bugzilla patch review action redirects non-patches infinitely | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> |
| Component: | Tools / Tests | Assignee: | Michael Catanzaro <mcatanzaro> |
| Status: | REOPENED | ||
| Severity: | Normal | CC: | ap, lingcherd_ho, lingho, mcatanzaro, webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | All | ||
| OS: | All | ||
Michael Catanzaro
This is a decade-old bug that I've just never taken the time to properly report before now.
Try to load https://bug-254326-attachments.webkit.org/attachment.cgi?id=465550&action=review in WebKitGTK and the load will fail with error message "Load cannot follow more than 20 redirections." If we try the same thing with curl:
$ curl -L 'https://bug-254326-attachments.webkit.org/attachment.cgi?id=465550&action=review'
curl: (47) Maximum (50) redirects followed
So WebKitGTK has the same behavior as curl. Problem is https://bug-254326-attachments.webkit.org/attachment.cgi?id=465550&action=review redirects to https://bugs.webkit.org/attachment.cgi?id=465550&=&action=review, but that itself redirects back to https://bug-254326-attachments.webkit.org/attachment.cgi?id=465550&action=review, so there's no winning.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Alexey Proskuryakov
This happens when the attachment isn't a patch. It would be nice to make the redirection make more sense indeed.
Radar WebKit Bug Importer
<rdar://problem/107421556>
lingho@apple.com
How does one gets to this link that includes action=review?
Alexey Proskuryakov
Bugzilla emails contain links with "action=review" for every added attachment.
Michael Catanzaro
(In reply to Alexey Proskuryakov from comment #4)
> Bugzilla emails contain links with "action=review" for every added
> attachment.
Proposal: we could simply stop adding this to the links in emails? There are very few patches being landed via Bugzilla nowadays, so the desired action= is almost never going to be "review". Then most people would never notice this bug.
Alexey Proskuryakov
We can probably make the appending conditional almost as easily.
The review functionality is all ours, not coming from Mozilla, so whoever implemented this must have made too big of a simplification.
Michael Catanzaro
Pull request: https://github.com/WebKit/WebKit/pull/36263
Michael Catanzaro
Alexey, if you could check the pull request, that would be great. Thanks!
EWS
Committed 286257@main (26082efe5dfd): <https://commits.webkit.org/286257@main>
Reviewed commits have been landed. Closing PR #36263 and removing active labels.
Michael Catanzaro
This is not actually fixed on bugs.webkit.org. Is it possibly that the change has simply not been deployed yet? If it has been deployed, then we should reopen this.
Ling Ho
Verified that the changes have been deployed on the server.
Michael Catanzaro
OK, well it's still broken as of today, so reopening. :(