Uses new PHP 5.6 syntax like ...parameter unpacking and
calling anything looking like a callback to make the code more readable.
There are much more occurrences but this commit is intentionally limited
to an easily reviewable size.
In one occurrence, a simple conditional instead of trickery was much more readable.
This patch finishes all the easy stuf in the core, the remainder is either unobvious
or would result in smaller readability gains. It will be carefully dealt with in
further commits.
Change-Id: I79a16c48bfb98b75e5b99f2f6f4fa07b3ae02c5b
file_get_contents() started supporting a negative offset in 7.1+. But
we really just want to start with 0.
Also fix the order of arguments to assertSame() so that the expected
value is first.
Bug: T182366
Change-Id: I84c92652de5b51a43f6e2b58cd235d2889093453
Find: /isset\(\s*([^()]+?)\s*\)\s*\?\s*\1\s*:\s*/
Replace with: '\1 ?? '
(Everywhere except includes/PHPVersionCheck.php)
(Then, manually fix some line length and indentation issues)
Then manually reviewed the replacements for cases where confusing
operator precedence would result in incorrect results
(fixing those in I478db046a1cc162c6767003ce45c9b56270f3372).
Change-Id: I33b421c8cb11cdd4ce896488c9ff5313f03a38cf
Clean up use of @codingStandardsIgnore
- @codingStandardsIgnoreFile -> phpcs:ignoreFile
- @codingStandardsIgnoreLine -> phpcs:ignore
- @codingStandardsIgnoreStart -> phpcs:disable
- @codingStandardsIgnoreEnd -> phpcs:enable
For phpcs:disable always the necessary sniffs are provided.
Some start/end pairs are changed to line ignore
Change-Id: I92ef235849bcc349c69e53504e664a155dd162c8
Previously warnings about the sha1 of an uploaded file being
the same as an existing file were stripped if the existing
file was in local storage - this was to avoid duplicate
warnings if a file with the same name had already been found
and had the same content. Now the warning is only stripped
for local files with the same name as the uploaded file.
Bug: T180691
Change-Id: I455df30085c05320dca976b9f7f8fb711a083271
These methods also don't access any of the class
properties and could one day be factored out into
some file checking service.
This also means that individual checks can be used for
the attached task if made protected.
Bug: T163500
Change-Id: I7cf912507ee02c35b6a666d7ed48fcab001316d3
Only allow ENTITY declarations inside the doctype internal
subset. Do not allow parameter entities, recursive entity
references are entity values longer than 255 bytes, or
external entity references. Filter external doctype subset
to only allow the standard svg doctypes.
Recursive entities that are simple aliases are allowed
because people appear to use them on commons. Declaring
xmlns:xlink to have a #FIXED value to the xlink namespace
is allowed because GraphViz apparently does that so its
somewhat common.
This prevents someone bypassing filter by using default
attribute values in internal dtd subset. No browser loads
the external dtd subset that I could find, but whitelist
just to be safe anyways.
Issue reported by Cassiogomes11.
Bug: T151735
Change-Id: I7cb4690f759ad97e70e06e560978b6207d84c446
It's unreasonable to expect newbies to know that "bug 12345" means "Task T14345"
except where it doesn't, so let's just standardise on the real numbers.
Change-Id: I6f59febaf8fc96e80f8cfc11f4356283f461142a
&$this triggers warnings in PHP 7.1. Simply renaming the variable before
passing it by reference avoids the warning, without breaking backwards
compatibility.
Bug: T153505
Change-Id: I78ea04a01ecce82294837e92c2a05b00ffb6e0f6
Inkscape mangles namespace definitions created by Adobe Illustrator
(apparently it can't parse custom entities or something, maybe just
in 'xmlns' attributes). These files are still valid SVG, and not
a security issue (although Illustrator probably won't like them),
so it's okay to allow them.
Added tests with some example files.
* buggynamespace-original.svg
File generated by Illustrator (edited by hand to reduce filesize).
Based on <https://commons.wikimedia.org/w/?curid=16495597>.
* buggynamespace-okay.svg
The original file, opened and saved in Inkscape (no other changes).
* buggynamespace-okay2.svg
The original file, opened and saved in Inkscape twice.
* buggynamespace-bad.svg
The original file, edited by hand to remove custom entities.
This is not valid XML and should be rejected (although it's valid
when parsed as HTML, and some image viewers might display it).
* buggynamespace-evilhtml.svg
An SVG file using an entity declared namespace for a namespace
we want to ban. Based on buggynamespace-original.svg.
Bug: T144827
Change-Id: I0eb9766cab86a58d729f10033c64f57d2076d917
* FSFile should not be responsible for handling this much logic.
* Make more MediaHandler classes aware of the fact that an object
other than File might be passed in. Use the FSFile instead of a
useless empty stdClass object.
* Also added more fields to FSFile::placeholderProps to make it
more complete.
Change-Id: I9fe764b2a7261af507c6555e6a57273cf7d00d36
"I've a great idea", they said. "You know what would be cool? If I
made this boring getter, getLocalFile(), return something completely
different after the file was stashed. This will be a nice surprise for
someone in the future to discover", they added gleefully.
I am pretty sure everything still works, but I never could get async
upload publishing to work locally, so I'd appreciate some testing.
Change-Id: I11dcf2ed89e4f1dd8ddf081af521da005efdbf39
In UploadFromChunks, doStashFile() only stashes the current chunk,
and stashing the whole file after all chunks are uploaded is done
in concatenateChunks().
Also more custom code on top of custom code to handle ApiMessage
errors, because action=upload is special.
Follow-up to 2ee27d9a4d.
Change-Id: I968280353f15bbca9b1059631fa2cfd7c3cd2f1d
Consumers of this hook don't expect partial files, and even if they did,
they have no way to tell if they've been given a partial file.
Also document a pre-existing restriction that verifyUpload() must be
called first (for non-partial files). ApiUpload previously called this
function for partial files too, causing T143161.
Follow-up to 2ee27d9a4d.
Bug: T143161
Change-Id: I107cb957e046545ea72834d72233cc1ed2867e42
At the moment the job might start before the transaction
that creates the file's row in the DB has had a chance
to run.
Bug: T106740
Change-Id: If5b94e83d8bbcc6aebfe7193f7b580f03cbd627d
UploadBase:
* Introduce a new method, tryStashFile(), as a replacement for the
now-soft-deprecated stashFile(). The method runs the new hook and
returns a Status object, with an error (if the hook returned an
error) or a value (if it didn't).
* Introduce a new hook, UploadStashFile, allowing extensions to
prevent a file from being stashed. Note that code in extensions
which has not been updated for MediaWiki 1.28 may still call
stashFile() directly, and therefore not call this hook. For
important checks (not just for UI), extension authors should use
UploadVerifyFile or UploadVerifyUpload hooks.
* Extract common code of tryStashFile() and stashFile() to
a new protected method doStashFile().
SpecialUpload:
* Use tryStashFile() when stashing a file after a warning or
"recoverable error" was encountered.
ApiUpload:
* Refactor stashing code so that error handling only happens in one
place, not four different ones. Use Status objects rather than
exception throwing/catching for control flow.
* Simplify the error messages slightly (error codes are unchanged).
Produce better ones by always using handleStashException().
'stashfailed' is now always at root (not nested inside 'warnings'),
behaving the same as 'filekey' does on success.
* Use tryStashFile() when stashing. Handle errors so as to allow
custom API results passed via ApiMessage to be preserved.
Some API result changes for different requests are shown below.
api.php?action=upload&format=json&filename=good.png&file=...&stash=1
Before:
{
"error": {
"code": "stashfilestorage",
"info": "Could not store upload in the stash: Stashing temporary file failed: UploadStashFileException Error storing file in '/tmp/phpB32SRT': Could not create directory \"mwstore://local-backend/local-temp/3/3a\".",
"*": "See http://localhost:3080/w/api.php for API usage"
}
}
After:
{
"error": {
"code": "stashfilestorage",
"info": "Could not store upload in the stash: Error storing file in '/tmp/phpB32SRT': Could not create directory \"mwstore://local-backend/local-temp/3/3a\".",
"*": "See http://localhost:3080/w/api.php for API usage"
}
}
api.php?action=upload&format=json&filename=[bad].png&file=...
Before:
{
"upload": {
"result": "Warning",
"warnings": {
"badfilename": "-bad-.png",
"stashfailed": "Stashing temporary file failed: UploadStashFileException Error storing file in '/tmp/phpB32SRT': Could not create directory \"mwstore://local-backend/local-temp/3/3a\"."
}
}
}
After:
{
"upload": {
"result": "Warning",
"stashfailed": "Could not store upload in the stash: Error storing file in '/tmp/phpB32SRT': Could not create directory \"mwstore://local-backend/local-temp/3/3a\"."
"warnings": {
"badfilename": "-bad-.png",
}
}
}
Bug: T140521
Change-Id: I2f574b355cd33b2e9fa7ff8e1793503b257cce65
The check is meant to prevent weird encodings like UTF-7 or HZ.
Encodings like the WINDOWS-125X family which are extensions of ascii
are safe. Additionally people still use windows-1252 on rare occasion.
Bug: T72937
Change-Id: I6cd63274cc04a7fca3afd244b4122ea64042dced
The new hook runs at the beginning of UploadBase::performUpload().
Unlike the existing UploadVerifyFile hook, the new one provides the
information about the file description page being created, and the
user who is performing the upload. It also does not run for uploads
to stash.
The action=upload API and Special:Upload have been changed to treat
errors from UploadBase::performUpload() as recoverable, which means
that they will attempt to stash the file (previously they would require
the user to upload it again). This is mostly intended for errors from
the new hook, but I think it makes sense for the existing ones, which
are mostly transient filesystem erorrs.
Bug: T89302
Change-Id: Ie68801b307de8456e1753ba54a29c34c8063bc36
SVG filter incorrectly used the m modifier when checking if an href
attribute started with 'https?://', incorrectly matching attributes
such as, "javascript:alert(' http://foo')".
Bug: T122653
Change-Id: I41291fff344241cad3171f3e8050de99b62a2296
Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
