Page MenuHomePhabricator

Special:MyPage dies with action=edit&redlink=1
Open, NormalPublicBUG REPORT

Description

Steps to Reproduce:

  1. Activate "mySandbox: Add a “Sandbox” link to the personal toolbar area." on Wikidata.
  2. Have your /sandbox page created.
  3. Click "Sandbox" in the personal toolbar area.

Actual Results:

  • You will see a blank page (status "200 OK").

Expected Results:

  • You are redirected to your /sandbox page.

If the sandbox does not exist, it works as expected. The cause is combination of Special:MyPage and action=edit&redlink=1, these two components are sufficient for reproducing.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 5 2019, 10:08 AM
Rehman added a subscriber: Rehman.Aug 7 2019, 2:08 AM
DannyS712 added a subscriber: DannyS712.EditedAug 7 2019, 3:46 AM

Special:MyPage should allow redlink as a parameter:

As far as I can tell, the only possible culprit is Hooks::run( "RedirectSpecialArticleRedirectParams", [ &$redirectParams ] );

However, when I go to https://www.wikidata.org/wiki/Special:Mypage/sandbo?action=edit&redlink=1&editintro=Template%3AUser_sandbox&preload=Template%3AUser_sandbox%2Fpreload ("sandbo", not "sandbox"), it works fine. I tested it with "sandbOx", "Sandbox", and a few other variations - as far as the only problematic target is "sandbox". Furthermore, I tried this on mediawiki.org, and the same issue occurred: every Special:Mypage/foo?... worked except for sandbox, and even sandbox worked with every parameter other than redlink being passed.

What extensions are currently activated that use this hook?
The VisualEditor extension is one of the extensions that uses this hook (its the only 1 I could find, but I searched quickly).
See https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/VisualEditor/+/refs/heads/master/includes/VisualEditorHooks.php at onRedirectSpecialArticleRedirectParams: $redirectParams[] = 'veaction';

On a hunch, I went to a wiki that didn't have VisualEditor installed, and the link worked fine: https://ar.wikipedia.org/wiki/Special:MyPage/sandbox?action=edit&redlink=1&editintro=Template%3AUser_sandbox&preload=Template%3AUser_sandbox%2Fpreload

Just for a simpler test case, the problem also occurs on https://www.wikidata.org/wiki/Special:MyPage?action=edit&redlink=1

For the problem to occur, the page must already exist, and it must open in the classic wikitext editor (not VE or NWE). If viewing that URL seems to work fine, then check your preferences, disable VE and NWE, and try again.

I don't think VisualEditor is actually causing this problem. I was able to reproduce on a local wiki with VisualEditor not installed.


The problem is apparently triggered because two redirections are required to reach the target (Special:MyPage?action=edit&redlink=1User:Yourname?action=edit&redlink=1User:Yourname), and because MediaWiki actually doesn't do a real redirection for the first one to avoid leaking the user's identity through page view stats (T109724). It doesn't occur when setting $wgHideIdentifiableRedirects=false.

I couldn't figure out how exactly this happens though. The code doing the second redirect (in EditPage::edit()) seems like it should just work (although I guess it's good that it doesn't, because that would allow the leak again). I think it would be good if someone could identify why it's failing the way it does.

Regardless, it can be fixed by doing the right redirect (or fake redirect) directly.

Change 529179 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] RedirectSpecialArticle: Avoid double redirect for action=edit&redlink=1

https://gerrit.wikimedia.org/r/529179

@mobrovac Do you or anyone else from the team have any idea why this bug results in a blank page instead of redirecting?

mobrovac added subscribers: tstarling, Anomie.

@mobrovac Do you or anyone else from the team have any idea why this bug results in a blank page instead of redirecting?

I looked around but couldn't see anything obvious as to why. Perhaps @Anomie or @tstarling might know.

Change 529179 merged by jenkins-bot:
[mediawiki/core@master] RedirectSpecialArticle: Avoid double redirect for action=edit&redlink=1

https://gerrit.wikimedia.org/r/529179

I'm no developer, but just though I'd link the original discussion here: https://www.wikidata.org/wiki/MediaWiki_talk:Gadget-mySandbox.js#Link_to_sandbox_is_wrong

The OP suggested an edit, which may or may not be the right modification to fix this. Cheers.

@mobrovac Do you or anyone else from the team have any idea why this bug results in a blank page instead of redirecting?

I looked around but couldn't see anything obvious as to why. Perhaps @Anomie or @tstarling might know.

Wouldn't surprise me if it's the same as for T227700#5334850, which was similarly worked around rather than tracked down.

After digging into it, it seems the problem is as follows:

  1. At MediaWiki.php line 274, it creates a DerivativeRequest in the process of trying to bypass the actual Special:MyFoo redirect for T109724: A combination of Special:MyPage redirects and pagecounts allows an external site to know the wikipedia login of an user, and injects that into the context.
  2. The subsequent processing within that method works right. The right code gets executed and OutputPage gets the redirect target properly set.
  3. At OutputPage.php line 2488, it calls ->response() on the DerivativeRequest created at step 1. That returns a FauxResponse, not a WebResponse.
  4. At OutputPage.php line 2517, it tries to set the "Location" header on that FauxResponse, which doesn't send the header to the browser because it's a FauxResponse rather than a WebResponse.
  5. Thus the browser never sees that it's supposed to be a 301 redirect. It gets a 200 response with the empty content that MediaWiki serves when producing a redirect. It would probably have been less confusing if MediaWiki would serve a stub body like RFC 7231 § 6.4 recommends in a few places.

If we make DerivativeRequest->response() return $this->base->response(),[1] the redirect goes through as expected. That also fixes the problem from T227700#5334850 without the need for a "force" path-parameter. On the other hand, it's possible that that might reopen the hole that T109724 was trying to patch (for that matter, rMW68558351c835: RedirectSpecialArticle: Avoid double redirect for action=edit&redlink=1 may have done so too). Maybe the thing to do would be to detect the redirect at step 2 (near the end of MediaWiki::processRequest()) or the FauxResponse at step 4 and render some interstitial page in that case.

[1]: I'm not sure whether all other uses of DerivativeRequest would continue to do the right thing if their ->response() started returning a real WebResponse instead of a fake one, though. So if we went that route we might need to instead have a setter for the response and use it in this code path.