Page MenuHomePhabricator

FlickrUpload stopped working due to API request through iframe && X-Frame-Option DENY
Closed, ResolvedPublic

Description

Original Bug title:
FlickrUpload stopped working due to API request through iframe && X-Frame-Option DENY

Issue:
From the casual user's perspective.

On Wikimedia Commons, Flickr upload through Upload Wizard ( https://commons.wikimedia.org/wiki/Special:UploadWizard ) does no longer work. After selecting a file from Flickr for upload, the throbber spins around but Upload Wizard never knows if the upload succeeded.

For reproducing you should be a member of the sysop or the image-reviewer user group.

JavaScript error console.
Mozilla Firefox latest RC.

Load denied by X-Frame-Options: https://commons.wikimedia.org/w/api.php does not permit framing.
Error: Permission denied to access property 'document'
https://bits.wikimedia.org/commons.wikimedia.org/load.php?debug=false&lang=en&modules=ext.uploadWizard.apiUploadFormDataHandler%2CapiUploadHandler%2Cevents%2CformDataTransport%2CiFrameTransport%2Cpage%7Cjquery.arrowSteps%2Cvalidate%7Cmediawiki.api.category%2Cparse%2Ctitleblacklist%7Cmediawiki.feedback%7Cmediawiki.libs.jpegmeta%7Cschema.UploadWizardErrorFlowEvent%2CUploadWizardFlowEvent%2CUploadWizardStep%2CUploadWizardTutorialActions%2CUploadWizardUploadActions%2CUploadWizardUploadFlowEvent%7Cuw.EventFlowLogger%2Cbase%7Cuw.controller.Deed%2CDetails%2CStep%2CThanks%2CTutorial%2CUpload%2Cbase%7Cuw.model.Description%2Cbase%7Cuw.ui.Step%2CThanks%2CWizard%2Cbase&skin=vector&version=20141021T180751Z&*
Line 12

All response headers: http://pastebin.de/145139

This regression has been presumably introduced with yesterday's deployment of MW 1.25wmf4


Version: unspecified
Severity: normal
URL: https://commons.wikimedia.org/wiki/Commons:Village_pump#.22Select_images_from_Flickr.22_.28Upload_Wizard.29
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=65423

Details

Reference
bz72340

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:55 AM
bzimport added a project: UploadWizard.
bzimport set Reference to bz72340.
Rillke created this task.Oct 22 2014, 12:30 AM

Not setting a web browser since I think it will affect all of them.

Tgr added a comment.Oct 22 2014, 1:06 PM

Could you retry in debug mode and paste the console output?

Tgr added a comment.Oct 22 2014, 3:17 PM

No commit between wmf3 and wmf4 seems relevant.

(In reply to Tisza Gergő from comment #2)

Could you retry in debug mode and paste the console output?

Sure.
https://commons.wikimedia.org/wiki/Special:UploadWizard?debug=true&uselang=en-gb

Load denied by X-Frame-Options: https://commons.wikimedia.org/w/api.php does not permit framing.
Error: Permission denied to access property 'document'
https://bits.wikimedia.org/static-1.25wmf4/extensions/UploadWizard/resources/mw.IframeTransport.js
Line 100

Tgr added a comment.Oct 22 2014, 6:16 PM

Can you double-check whether this affects normal uploads? I don't see why such an error would be limited to Flickr uploads. (Actually I would expect it to affect normal uploads only. Why do Flickr uploads even have a transport? And an iframe transport at that, in current Firefox?)

(In reply to Tisza Gergő from comment #7)
This has been a bug since a very long time: For Flickr uploads the iframe transport was used no matter which browser you were using. Now, it's just worse because the X-Frame-Options header changed; interestingly for upload only as far as I can see.

For normal uploads, I see a POST XHR in my console.

Tgr added a comment.Oct 22 2014, 8:12 PM

(In reply to Rainer Rillke @commons.wikimedia from comment #8)

This has been a bug since a very long time: For Flickr uploads the iframe
transport was used no matter which browser you were using.

Do you know if there is a bug report for this already?

Poking around, I found https://gerrit.wikimedia.org/r/#/c/22290 which was supposed to fix this problem (and did fix it for normal uploads). So the API is used for normal uploads (with an iframe transport) but not for Flickr uploads? Weird.

Probably the whole iframe transport could be retired, given that MediaWiki doesn't support IE7 anymore.

(In reply to Tisza Gergő from comment #9)

the API is used for normal uploads (with an iframe transport)

For uploads from the user's file system, an XHR is used -- *not* an iframe (except on older IEs which are apparently no longer supported).

Its caused by d25cb992, which changed how

Ok the api request in question was

-----------------------------24311189326665
Content-Disposition: form-data; name="filename" 1413936868606Leipzig Market, Saxony, Germany (LGM2014).jpg
-----------------------------24311189326665
Content-Disposition: form-data; name="action" upload -----------------------------24311189326665
Content-Disposition: form-data; name="stash" 1 -----------------------------24311189326665
Content-Disposition: form-data; name="ignorewarnings" 1 -----------------------------24311189326665
Content-Disposition: form-data; name="comment" DUMMY TEXT -----------------------------24311189326665
Content-Disposition: form-data; name="format" jsonfm -----------------------------24311189326665
Content-Disposition: form-data; name="url" https://farm8.staticflickr.com/7214/13781228444_81b22a1af8_o.jpg
-----------------------------24311189326665
Content-Disposition: form-data; name="token" <redacted>+\ -----------------------------24311189326665--

Note the lack of a format parameter - I guess the upload wizard doesn't check for errors?

Anyhow, that makes the format be jsonfm. In d25cb992 the formatted output was changed to use OutputPage, which has its own X-Frame-Options handling that overrides the API's $wgApiFrameOptions. Which is why we're getting DENY for this request.

gerritadmin wrote:

Change 168235 had a related patch set uploaded by Brian Wolff:
Respect $wgApiFrameOptions in formatted API output mode

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

Tgr added a comment.Oct 23 2014, 7:27 AM

(In reply to Tisza Gergő from comment #9)

(In reply to Rainer Rillke @commons.wikimedia from comment #8)

This has been a bug since a very long time: For Flickr uploads the iframe
transport was used no matter which browser you were using.

Do you know if there is a bug report for this already?

bug 65423

(In reply to Bawolff (Brian Wolff) from comment #11)

Note the lack of a format parameter - I guess the upload wizard doesn't check
for errors?

There is a
Content-Disposition: form-data; name="format" jsonfm
in the request. Sorry for the messy pastebin.

(In reply to Gerrit Notification Bot from comment #12)

Change 168235 had a related patch set uploaded by Brian Wolff:
Respect $wgApiFrameOptions in formatted API output mode
https://gerrit.wikimedia.org/r/168235

While this patch will allow FlickrUpload to continue working, UploadWizard still really needs to be fixed to not be using format=jsonfm. "json = $( doc.body ).find( 'pre' ).text();" that I see in https://bits.wikimedia.org/static-1.25wmf4/extensions/UploadWizard/resources/mw.IframeTransport.js is not sane.

Tgr added a comment.Oct 23 2014, 4:15 PM

As I said, there is no reason whatsoever for UploadWizard to even use a transport for Flickr upload, much less to use the iframe transport, so using jsonfm is really least of the problems here.

gerritadmin wrote:

Change 168235 merged by jenkins-bot:
Respect $wgApiFrameOptions in formatted API output mode

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

(In reply to Brad Jorsch from comment #15)

needs to be fixed to not be using format=jsonfm

"according to mdale we need to do this because IE does not load JSON properly in an iframe"
[excerpt of comment from cited UploadWizard's source code]

(In reply to Rainer Rillke @commons.wikimedia from comment #18)

"according to mdale we need to do this because IE does not load JSON
properly in an iframe"
[excerpt of comment from cited UploadWizard's source code]

I'm sure there are better ways than screen-scraping.

Tgr added a comment.Oct 23 2014, 7:11 PM

(In reply to Brad Jorsch from comment #19)

I'm sure there are better ways than screen-scraping.

Adding a JSONPI output format to the API, or something equally horrible. Given that IE6/7 is not supported any more, this is not a problem worth solving though.

gerritadmin wrote:

Change 168386 had a related patch set uploaded by Gergő Tisza:
Respect $wgApiFrameOptions in formatted API output mode

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

gerritadmin wrote:

Change 168386 merged by jenkins-bot:
Respect $wgApiFrameOptions in formatted API output mode

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

Gilles moved this task from Untriaged to Done on the Multimedia board.Dec 4 2014, 9:28 AM