Callers should not catch an unchecked exception, so it doesn't belong
in a function signature. Unchecked exceptions indicate a coding error,
which by definition the code will not be able to handle correctly.
If any of these exceptions were supposed to be in response to an edge
case, user input, or initial conditions, then they should be changed
to a runtime error. If the exception class cannot be changed, then
the annotation should include a comment explaining its purpose and
prognosis.
Bug: T240672
Change-Id: I2e640b9737cb68090a8e1cb70067d1b74037d647
Add doc-typehints to class properties found by the PropertyDocumentation
sniff to improve the documentation.
Once the sniff is enabled it avoids that new code is missing type
declarations. This is focused on documentation and does not change code.
Change-Id: I46f46f1855ca32c89a276b06f4e2051ff541886e
And deprecated aliases for the the no namespaced classes.
ReplicatedBagOStuff that already is deprecated isn't moved.
Bug: T353458
Change-Id: Ie01962517e5b53e59b9721e9996d4f1ea95abb51
This patch introduces a namespace declaration for the
Wikimedia\Http to MultiHttpClient and establishes a class
alias marked as deprecated since version 1.43.
Bug: T353458
Change-Id: I4ebc1a3b496de08b2b430301da376578d13fcfe6
This patch introduces a namespace declaration for the
Wikimedia\FileBackend to FileBackend and establishes a class
alias marked as deprecated since version 1.43.
Bug: T353458
Change-Id: Id897687b1d679fd7d179e3a32e617aae10ebff33
This patch introduces a namespace declaration for the
MediaWiki\Json to FormatJson and establishes a class
alias marked as deprecated since version 1.43.
Bug: T353458
Change-Id: I5e1311e4eb7a878a7db319b725ae262f40671c32
In FileBackend::streamFile(), do not send "headers" in "headless" mode.
In HTTPFileStreamer::stream(), do not treat $headers as an exception.
Just use $headerFunc like with all the other headers.
Nothing relies on the old behavior, and the only things using "headless" are
integration tests anyway. The same holds true for STREAM_HEADLESS.
Change-Id: I75faf9195a713d6a8a8f1273cb9e747980f4d57f
"Deny from all" is deprecated; the replacement syntax has been
available since Apache 2.4 (originally released in 2012).
See <https://httpd.apache.org/docs/2.4/howto/access.html>.
Bug: T360850
Change-Id: I825053ccefe34f6ca4e04af5ad2601f79e4d51a7
Every Wednesday, we get hundreds of user-visible storage failures,
because there is only one global storage token and it has a lifetime of
one week. The code assumes that requesting a token regularly will avoid
expiry, but that is not the case.
So, when a request fails with a 401, refresh the auth token, then retry
the request. Factor out HTTP request handling to allow this.
Swift gives us the remaining lifetime of the token in seconds, so use
that as the server and process cache expiry time. There will be a
stampede when it expires, but my laptop can do 700 req/s single-
threaded, so it should be OK.
Add tests to cover the cases previously handled by checking the return
value of getAuthentication().
Bug: T358830
Change-Id: If9fe4dde4127592adae1b81e8eb925c4f59443d9
MemoryFileBackend, a class which is only used for testing, does not
correctly respect the ignoreMissingSource option to its move operation.
The copy succeeds due to the forwarded ignoreMissingSource option, so it
proceeds with the source deletion, which fails.
This was implemented in the base class method FileBackendStore
::doMoveInternal, but every other subclass was overriding it.
So, make it abstract, by analogy with doCopyInternal and
doDeleteInternal. Add an implementation specific to MemoryFileBackend
which handles ignoreMissingSource correctly.
Change-Id: Ib5733046d741b962c65db42b5364705b073c4b06
It's possible to run FileBackendIntegrationTest against an actual Swift
server, but there were a few failures when I tried it:
* In SwiftFileBackend::getDirListPageInternal(), fix a PHP warning due
to strpos(null).
* In setUp(), fix exception "Backend domain ID not provided" due to
missing domainId parameter.
* In doTestGetLocalCopyAndReference404(), getLocalCopy() on a sharded
backend for a file that cannot exist, due to having no shard, returns
null not false, which I think is fine. Use a path that can exist so
that the assertion passes.
* In doTestGetFileList(), there is an assertion to confirm that
getFileList() on a non-existent container will return null, but in
Swift this is not known until iteration begins, so it returns an
iterator. Fix the assertion.
Change-Id: If3fc47eb285b51366e7cfd8b6cf6e64bd0f8cd41
Remove $fileOpHandles since it was effectively no longer used. Rename
$curFileOpHandles to $batch and note its type. Remove unnecessary
comments and improve the comment about T230245 by explaining what was
going on with that bug.
Followup to I67f9f25b3bc68a389c49c39b7b6def2343bda34e.
Change-Id: I809afc7ce4f2f32018c975ec0073cd8a4af21763
Too many queued FileOps may end up causing 'Too many open files'
failures, and mysterious error messages claiming that existing
files were not found.
Bug: T230245
Change-Id: I67f9f25b3bc68a389c49c39b7b6def2343bda34e
This replaces the old pass-by-reference $predicates array and makes the
intent clearer. It also supports lazy-loading of predicated properties,
which useful for avoiding expensive computations like SHA-1 hashes when
they end up not being needed.
Pass FileOp::doPrecheck() a mutable "batch level" FileStatePredicates
instance along with an immutable "operation level" FileStatePredicates
instance. The later can be used in callbacks that need to recall the
predicated file states just before the start of the operation.
Make FileOp::precheckDestExistence() support lazy size/sha1 closures.
Bug: T348294
Change-Id: I709c1920d2bee4b76f69e38c1643a44a73d63265
* Switch out raw Exceptions, mostly for InvalidArgumentExceptions.
* Fake exceptions triggered to give Monolog a backtrace are for
some reason "traditionally" RuntimeExceptions, instead, so we
continue to use that pattern in remaining locations.
* Just entirely give up on PostgresResultWrapper's resource vs. object mess.
* Drop now-unneeded false positive hits.
Change-Id: Id183ab60994cd9c6dc80401d4ce4de0ddf2b3da0
For readability. Allowed since PHP 7.4.
I searched for integer literals of 6 or more digits, and also changed
some nearby smaller numbers for consistency.
Bug: T353205
Change-Id: I8518e04889ba8fd52e0f9476a74f8e3e1454b678
The SwiftBackend theoretical max file size was increased to 5 GB
Sysadmins of MediaWiki are in a better position to judge what
is acceptable performance-wise for uploaded files than we are. The
answer presumably depends significantly on the hardware mediawiki
is running under. This increases the max limit to 32 GB (previously 4GB).
With files this big, operations such as copying and calculating SHA1
do become expensive. However everything still works, albeit with
some delay (I tested on a laptop with HDD, I imagine things are
significantly faster with an NVMe drive).
This changes the overall FileBackendStore limit which affects all
subclasses that haven't overriden the limit, including those from
extensions. Swift was set to 5GB as that backend has to change the
way it uses the Swift API for objects larger than 5GB.
This should not affect ordinary users as $wgMaxUploadSize is by
default only 100 MB in MediaWiki (4GB in Wikimedia).
Bug: T191805
Change-Id: Id091d51b620edbeea2d9df8c75422807bfc7aea0
array[]|bool[]|null[] means: It's either an array of arrays, an array
that contains only bools, or an array where all values are null. But
this is not what's happening here, as far as I can tell.
This minor fixup is motivated by T348688, but doesn't solve it.
However, now it's more obvious that the values that trigger the
reported error are "null", i.e. some instance of self::$RES_ERROR.
Bug: T348688
Change-Id: Ia9debe97d21a3abff7bff75f16a94bec8be6ddc5
This is a direct follow up to Id0e4b0d from 2019 where these
"constants" have been introduced. I believe the reason for using
protected properties was that we still had to support PHP 7.0 back
then, but class constants can only be marked as protected since
PHP 7.1.
According to codesearch these are apparently meant to be internal
to the filebackend classes in core and not used anywhere else.
Change-Id: I55683e3540f8d76324370efecb98d08e07547c28
Put 100 bytes of the body into the log message to possible see more
information about the gateway error message
See T328872#8757648
Bug: T328872
Change-Id: I8b2df6d318d8de912eb9b992051b209bab236f37
Avoid calling next() or count() when the buffer is null.
This avoids the risk of type errors.
Also improve some comments.
Change-Id: Ida422f4b7f185ac8bffc479ab5c82711e5621c89
Use str_starts_with, str_ends_with or string offset where appropriate.
This fixes a bug in MimeAnalyzer where the "UTF-16LE" header could not
be identified because of wrong constant. This is the exact type of bug
that the new functions can avoid.
Change-Id: I9f30881e7e895f011db29cf5dcbe43bc4f341062
FileBackend::getLocalReference and FileBackend::getLocalCopy should
report failure and missing with different values to make error reporting
for the caller easier (FileBackendStore::doConcatenate in this case).
The existing subclass implementation for
FileBackendStore::doGetLocalCopyMulti already doing that.
Change-Id: Ie29aed48dc33bcd9fe1ed1e91685596044dfd988
When trying to purge thumbnails, currently it tries to purge from the
main backend only leading to unhappiness
Bug: T331138
Change-Id: I2b13912296df68b2833b8c7c0bfe17d4ba362fce
If you store a WeakMap in the object, containing only circular
references in the values, then nothing is keeping the WeakMap alive and
it can be deleted along with its parent TempFSFile by the garbage
collector.
A WeakMap contains normal references to its values, so those normal
references can be marked by the GC like any other references.
Instead, store the WeakMap as a static property.
Bug: T332461
Bug: T332397
Change-Id: Idbaa1019c2bac3d1dd9b001753c2f4c0aaf66e98