Last modified: 2007-07-17 11:45:50 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 3173 - Add Content Disposition to XML export (proposed patch included)
Add Content Disposition to XML export (proposed patch included)
Status: VERIFIED FIXED
Product: MediaWiki
Classification: Unclassified
Special pages (Other open bugs)
1.11.x
All All
: Normal enhancement with 1 vote (vote)
: ---
Assigned To: Marc Meurrens (be) http://www.meurrens.org/
:
: 10603 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-08-17 08:47 UTC by Marc Meurrens (be) http://www.meurrens.org/
Modified: 2007-07-17 11:45 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
the full SpecialExport.php file proposal (a diff file will also be uploaded) (13.69 KB, application/x-php)
2005-08-17 08:49 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
the diff file (-Naurb) for the proposed SpecialExport.php patch (3.10 KB, patch)
2005-08-17 08:50 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
partial re-design of Special Export (24.84 KB, application/x-php)
2005-08-18 12:08 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
screenshot (english) (49.35 KB, image/png)
2005-08-18 12:12 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
screenshot in french (50.46 KB, image/png)
2005-08-18 12:12 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
diff -Naurb (23.79 KB, patch)
2005-08-18 12:18 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
partial re-design of SpecialExport.php (24.81 KB, application/x-php)
2005-08-18 13:02 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
diif -Naurb (23.92 KB, patch)
2005-08-18 13:07 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
haDoc comments (3.78 KB, text/plain)
2005-08-18 13:18 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
diff -Naurb (25.09 KB, text/x-patch)
2005-08-18 21:59 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
hacker documentation (18.15 KB, text/plain)
2005-08-18 22:00 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
the SpecialExport.php including eveything (but using internally my conventions) (24.82 KB, application/x-php)
2005-08-18 22:01 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
just FYI : the script producing the haDoc (hacker documentation) file (11.05 KB, text/plain)
2005-08-18 22:03 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
the patched SpecialExport.php file (24.48 KB, application/x-php)
2005-08-20 16:21 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
diff -Naurb for Defines.php (1.24 KB, patch)
2005-08-20 16:24 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
diff -Naurb for Language.php (450 bytes, patch)
2005-08-20 16:25 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
diff -Naurb for LanguageFr.php (760 bytes, patch)
2005-08-20 16:29 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
diff -Naurb for SpecialExport.php (17.76 KB, patch)
2005-08-20 16:31 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
diff -Naurb for DefaultSettings.php (1.04 KB, patch)
2005-08-20 16:36 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details
diff -Naurb for LanguageFr.php (+ correction of an existing typo) (1.07 KB, patch)
2005-08-20 17:26 UTC, Marc Meurrens (be) http://www.meurrens.org/
Details

Description Marc Meurrens (be) http://www.meurrens.org/ 2005-08-17 08:47:43 UTC
This is a follow up of bug report 3159 (I erroneously commited as a bug report
to the SpecialImport functionnality).

Indeed the solution is in 'SpecialExport' and the problem is not in 'SpecialImport'.

In order to facilitate the saving ox XML files, exported from mw, in an
appropriate way
(which is not an immediate operation even on good browsers like firefox,
and even for not-too-stupid users...), we should rely on the Content-Disposition
HTTP
header (at least, optionaly).

Everything, including the comments, are provided in my new version of file
SpecialExport.php.
(Just watch all lines around those including the pattern "// !" in this file)
Or, of course, in the SpecialExport.diff file.

I'll immediately upload these 2 files.
Comment 1 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-17 08:49:28 UTC
Created attachment 795 [details]
the full SpecialExport.php file proposal (a diff file will also be uploaded)
Comment 2 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-17 08:50:34 UTC
Created attachment 796 [details]
the diff file (-Naurb) for the proposed SpecialExport.php patch
Comment 3 Brion Vibber 2005-08-17 09:19:16 UTC
This sounds like it would break the ability to view the XML within Mozilla/Firefox 
(which is very, very helpful for debugging, experimenting, and exploring the system).

I'd prefer a fix in for the corruption bug in the offending browser(s). Can you narrow 
it down to a particular application, version, or setting?
Comment 4 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-18 12:08:53 UTC
Created attachment 799 [details]
partial re-design of Special Export

my re-design of SpecialExport.php
+ to take care of the content-disposition problem
+ Brion's remarks to allow viewing in a browser
+ very small bug fixed (marked // !FIXED)
+ restructuring the file
(modifs are easely found by a grep on "// !" comments)
Comment 5 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-18 12:12:05 UTC
Created attachment 800 [details]
screenshot (english)

screenshot of the new design output 
(in english,
however the browser is in french :
firefox let you choice between viewing or downloading)

(GNU/Linux Ubuntu 5.04 , mediawiki 1.5.104.4 (patch of 1.5beta4) , firefox
1.0.6)
Comment 6 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-18 12:12:57 UTC
Created attachment 801 [details]
screenshot in french

(GNU/Linux Ubuntu 5.04 , mediawiki 1.5.104.4 (patch of 1.5beta4) , firefox
1.0.6)
Comment 7 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-18 12:18:18 UTC
Created attachment 802 [details]
diff -Naurb

this diff file is rather large,
(thus I also uploaded the final .php file)
Comment 8 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-18 13:02:27 UTC
Created attachment 803 [details]
partial re-design of SpecialExport.php

(removing a very old (?) phpDoc comment)
Comment 9 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-18 13:07:16 UTC
Created attachment 804 [details]
diif -Naurb

quite large (see php file)
Comment 10 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-18 13:18:58 UTC
Created attachment 805 [details]
haDoc comments

haDoc comments help the reader to see where a file was hacked
and/or how the file is structured.

this is NOT a substitute for phpDoc, nor for diff !

I join my haDoc file so that you may easely retrieve what 
has happened with the file.

See http://meta.wikimedia.org/wiki/HaDoc to eventually read more about haDoc
comments.
Comment 11 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-18 21:59:20 UTC
Created attachment 807 [details]
diff -Naurb
Comment 12 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-18 22:00:01 UTC
Created attachment 808 [details]
hacker documentation
Comment 13 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-18 22:01:16 UTC
Created attachment 809 [details]
the SpecialExport.php including eveything (but using internally my conventions)
Comment 14 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-18 22:03:27 UTC
Created attachment 810 [details]
just FYI : the script producing the haDoc (hacker documentation) file
Comment 15 Brion Vibber 2005-08-19 09:12:04 UTC
This patch contains a lot of extraneous comments, config variables don't match 
convention, code style doesn't match convention, comment doesn't match phpdoc 
convention. Use of a float to control behavior is very unusual and not recommended.

Usually when one submits a patch to an existing project, one attempts to match the 
code style of that code. One in particular doesn't rewrite the entire file in some 
totally different style, renaming all the variables and functions and changing all 
the indentation and whitespace -- it makes it harder to review, and would make 
maintenance of the code much much harder as well.

It will absolutely not be accepted in this state; please rewrite following the 
existing coding style if you'd like others to actually be able to review the patch 
behind all the noise.

Note that the xmldispositionfilename parameter appears to be placed into an HTTP 
header and HTML output without escaping; this may be a security risk.
Comment 16 Zigger 2005-08-19 15:44:58 UTC
(Please add wikibugs-l@wikipedia.org to the CC list when assigning a bug.)
Comment 17 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-20 16:21:31 UTC
Created attachment 814 [details]
the patched SpecialExport.php file 

this is the full file.
standard conventions are now strictly respected.

However, the file also includes a few comments to the attention
of developpers (they are clearly indicated as such) : these comments
are intended to explain the choices and should be removed before distribution.

I have removed my haDoc comments (which were not 'noise' ;-( ...)
and my own naming conventions (underscore for private, lowercase for instance
methods, uppercase for static) as requested.

Nevertheless, WITHIN the newly created functions and methods, local variables
are named according to the hungarian prefix convention (yes, I am a very very
old
computer engineer from the old C time and the first UNIX systems - and it's
probably much too late to change me...). Because, this is only used WITHIN
functions, nothing forbid to write a boolean $zCurOnly instead of $curonly.
Comment 18 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-20 16:24:25 UTC
Created attachment 815 [details]
diff -Naurb for Defines.php

a few constants must be made available for DefaultSettings, LocalSettings,
and SpecialExport.

Mind that I replaced my fuzzy parameters (real between 0 and 1)
by a few unsigned short values.
Comment 19 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-20 16:25:25 UTC
Created attachment 816 [details]
diff -Naurb for Language.php

2 prompt messages ha
Comment 20 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-20 16:29:04 UTC
Created attachment 817 [details]
diff -Naurb for LanguageFr.php

2 prompt messages have been added

+ the submit button needs a specific message
(the reason is explained as a comment in the SpecialExport.php file)

Mind the diff files may not be very accurate regarding line numbers
because I just extracted the lines related to SpecialExport

(my Language.php and LanguageFr.php already include many other changes
which are not related to SpecialExport)
Comment 21 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-20 16:31:44 UTC
Created attachment 818 [details]
diff -Naurb for SpecialExport.php

The essential is the content disposition management.
Mind that the patch also include a few corrections
for very minor bugs.
These fixes are documented in the file.
Comment 22 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-20 16:36:38 UTC
Created attachment 819 [details]
diff -Naurb for DefaultSettings.php

again, line numbering may have been distrurbed by other patches...
Comment 23 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-20 16:43:35 UTC
Brion wrote :

 ''Note that the xmldispositionfilename parameter appears to be placed into an HTTP
 header and HTML output without escaping; this may be a security risk.''

The filename is not escaped because it is NOT the filename provided by the user.
Instead, the filename *suggested* by the user is passed thru a function to generate
a valid Unix file name.
Unless I made an error, this function should return a value without risk.
If I made an error, the function the solution will be to correct this function
(but not to escape).

Mind that this function (and her sister wfRemoveAccents) may eventually be moved
to GlobalFunctions.php if one needs them to generate filenames elsewhere.
Comment 24 Marc Meurrens (be) http://www.meurrens.org/ 2005-08-20 17:26:57 UTC
Created attachment 820 [details]
diff -Naurb for LanguageFr.php (+ correction of an existing typo)
Comment 25 Brion Vibber 2005-08-20 23:31:28 UTC
New patch still does not follow coding conventions.

wfRemoveAccents() looks unhelpful and far from complete if it were 
desireable. wfUnixFileName() looks extremely unhelpful, and will make 
completely illegible useless names for many languages. Note that failing to 
escape output because at the moment it comes from a function which will 
produce 'safe' output is an unsafe practice. Many valid Unix filenames would 
produce JavaScript injections if output directly into HTML.

None of this addresses the original question, either: what browsers are 
affected by this problem to begin with and can they be fixed?
Comment 26 Brion Vibber 2007-07-16 16:07:02 UTC
*** Bug 10603 has been marked as a duplicate of this bug. ***
Comment 27 Thorsten Staerk 2007-07-16 16:16:19 UTC
Here's my patch:

--- SpecialExport-0.php 2007-07-16 18:11:59.000000000 +0200
+++ SpecialExport.php   2007-07-16 18:10:22.000000000 +0200
@@ -151,6 +151,7 @@ function wfSpecialExport( $page = '' ) {
                // This should provide safer streaming for pages with history
                wfResetOutputBuffers();
                header( "Content-type: application/xml; charset=utf-8");
+                header("Content-Disposition: attachment" );
                $pages = explode( "\n", $page );

                $db = wfGetDB( DB_SLAVE );
Comment 28 Rob Church 2007-07-16 17:06:38 UTC
Fixed in r24172. I didn't use the patches above.
Comment 29 Thorsten Staerk 2007-07-17 11:45:50 UTC
verified. Thanks a lot !

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links