Bug 254336 – WebKit Bugzilla patch review action redirects non-patches infinitely

• Status: REOPENED
• Component: Tools / Tests

[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.

[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>

[[e-mail]]

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. :(