This might hint at an edge-case in the PHP CodeSniffer sniff that should
detect if methods are separated by a single empty line. Feel free to
investigate. I, personally, can't invest more time in this than
suggesting this quick fix.
Change-Id: Ib3c60eac76f255b4fe929f7933de256222716576
When a wiki is down, it is not necessarily useful to be able to
search the web. Additionally, there is general consensus that
the hard-coded Google search form should be removed.
Bug: T208871
Change-Id: I5bcae848de1144d4fc1116c475b2e2ab1ccc3f7d
AbortAutoAccount, AbortNewAccount, AbortLogin, LoginUserMigrated,
UserCreateForm, and UserLoginForm are all unused in Wikimedia
production and rare in other extensions.
This also scraps the FakeAuthTemplate and LoginForm classes and
the occasional remainig references thereto.
Bug: T193755
Change-Id: I24d6fa963f402d4311fa00fc11536a37ee3bd31e
Follows-up ef06b528d9, which fixed the severity of all PHP errors
to have severity "Warning" or higher.
However, it also accidentally changed the label of "Notice"
to "Warning", which is confusing and incorrect.
Change-Id: Iffd39aa23b7f2cbff5cdaf876b8d4d595dcd6f96
These methods aren't identical, so consolidation isn't
immediately obvious, and creating dependencies is problematic
in error handling code given there is a lot of pressure on this
code to not by itself cause additional errors.
This means that maybe it's best to keep these inlined without
duplication, but at the very least we then need to remember
to keep these in sync. This duplication has been around for
a while now, but documentation can never come too late...
Change-Id: I60160f61c13c8e115d839acce222f110e30bc2f2
Things like "Catchable fatal error: Must be X, null given" stop
execution immediately after the error handler callback, and produce
an HTTP 500 Internal Server Error page. They are very fatal.
Per <https://secure.php.net/manual/en/language.errors.php7.php>,
on PHP 7 this results in a TypeError throwable which will
eventually fatal the request and be reported through
set_exception_handler, or be caught by our top-level 'catch'
and artificially forwarded to our set_exception_handler callback.
On HHVM, these fatal error types are PHP5-like in that they
don't have a throwable object to throw yet, instead the error
meta-data is sent directly as parameeters to set_error_handler,
the same as for warnings. We need to intercept them there so
that they are reported correctly.
Sample from PHP 7:
> MediaWiki\Storage\MutableRevisionRecord::newFromParentRevision( null );
> [no req] TypeError from line 50 of .../MutableRevisionRecord.php:
> Argument 1 passed to ...\MutableRevisionRecord::newFromParentRevision()
> must be an instance of MediaWiki\Storage\RevisionRecord, null given,
> called in /var/www/mediawiki/maintenance/eval.php(78) ...
[exit status: error(255)]
Sample from HHVM:
> MediaWiki\Storage\MutableRevisionRecord::newFromParentRevision( null );
> [hphp] set_error_handler called with:
> ... $type = 4096 // E_RECOVERABLE_ERROR
>
> [hphp] [...] Catchable fatal error:
> Argument 1 passed to ...\MutableRevisionRecord::newFromParentRevision()
> must be an instance of MediaWiki\Storage\RevisionRecord, null given
> ...
[exit status: error(255)]
Interestingly, if you forget to return false from set_error_handler
for fatal errors on HHVM, it can actually continue execution.
Bug: T205677
Change-Id: I18dd2ff37fa2c2679d0c598cbeff0c61c2fe0253
All PHP errors should be considered by monitoring queries
and error-rate alerts.
Levels DEBUG and INFO are for manual entries that can help in
debugging to see what path a program went down or what the
circumstances were in that code.
There is no reason to spread PHP error-types out on the full range
of PSR-3's DEBUG to ERROR spectrum. Instead, keep it within either
WARNING or ERROR, not below it.
Change-Id: I3f35a519b50aef5b93b9ab7a89a7c3e11d70681f
Previously an ugly {{SITENAME}} would show up for exceptions
that happened in the middle of processing a message
Change-Id: I4e3b675673dc3b74f89e4325f6a0a8b44162f478
Clarify and simplify exception output by deprecating
$wgShowSQLErrors and wgShowDBErrorBacktrace.
$wgShowExceptionDetails will now control most related output.
$wgShowHostnames will now solely control output of
MWExceptionRenderer::reportOutageHTML.
Bug: T165768
Change-Id: Idead2c11c499463dfa6293c3d4b33be3bde92e1a
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
Use human-readable stack trace instead of array dump,
try to display the URL and the request ID, use the same
message format as exceptions,
Bug: T189851
Change-Id: I3edf2dbd5639ceecc668719c065ecdce33157ff5
Storing the user name or IP in every row in large tables like revision
and logging takes up space and makes operations on these tables slower.
This patch begins the process of moving those into one "actor" table
which other tables can reference with a single integer field.
A subsequent patch will remove the old columns.
Bug: T167246
Depends-On: I9293fd6e0f958d87e52965de925046f1bb8f8a50
Change-Id: I8d825eb02c69cc66d90bd41325133fd3f99f0226
It's annoying and pointless. Instead, have MWException write them to
standard output where we can catch them with ob_start().
Bug: T170028
Bug: T170029
Change-Id: Icd99c1c39d4a30d78c511d33948ef639e1b92455
Add a $wgPropagateErrors configuration variable which can be used
to prevent passing handled errors to PHP (and thus logging them twice).
Bug: T45086
Change-Id: I64ab09762a04de2007b7d7864e3c504a1d6f8aee
These comments do not add anything. I argue they are worse than having
no comments, because I have to read them first to understand they
actually don't explain anything. Removing them makes room for actual
improvements in the future (if needed).
Change-Id: Iee70aad681b3385e9af282d5581c10addbb91ac4
phpdbg is a gdb-style debugger for PHP that is run from the command
line. However, it has a different PHP_SAPI value, so it was impossible
to run maintenance scripts with it (until now).
To avoid having to check both PHP_SAPI values in a bunch of places,
introduce wfIsCLI() to easily check whether running from the
command-line or not.
We're (CI team) interested in generating code coverage with phpdbg
instead of xdebug, hence this patch.
Bug: T184043
Change-Id: Id1f994ca146d7858cd8bb6ab6cdbb7718ff524fb
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
This message contains the request url, which is semi-user controlled.
Most browsers percent escape < and > so its probably not exploitable
(curl is an exception here), but nonetheless its not good.
Bug: T178451
Change-Id: I19358471ddf1b28377aad8e0fb54797c817bb6f6
This will help us consolidate the various uses into one single
method which will help us drive standardisation of these defacto
widgets.
Hopefully, by being a method of the Html class, which has a very
low barrier for use will drive down the inconsistent display of
warning/error boxes across MediaWiki's products
Various usages of warningbox and errorbox have been ported over.
I've retained some more complicated usages which make use of the
parser (wrapWikiMsg) and any where id and class are medled with
- we'll probably want to consider whether we want to encourage
those going forward as they encourage adjusting the styling.
Bug: T166915
Change-Id: I2757e1f4ff2599e93a7257fc644cab69063896d2
I registered the additional shutdown function for CLI only
as it shouldn't have effect otherwise.
Bug: T177414
Change-Id: I440d294eef5e307743cfc7f5ab3b531e8c973873
This function has gotten so unwieldy that a helper was
introduced. Instead, here's this class that makes
shelling out easier and more readable.
Example usage:
$result = Shell::command( 'shell command' )
->environment( [ 'ENVIRONMENT_VARIABLE' => 'VALUE' ] )
->limits( [ 'time' => 300 ] )
->execute();
$exitCode = $result->getExitCode();
$output = $result->getStdout();
This is a minimal change, so lots of stuff remains
unrefactored - I'd rather limit the scope of this commit.
A future improvement could be an ability to get stderr
separately from stdout.
Caveat: execution errors (proc_open is disabled/returned error) now
throw errors instead of returning a status code. wfShellExec() still
emulates this behavior though.
Competing commit: I7dccb2b67a4173a8a89b035e444fbda9102e4d0f
<legoktm> MaxSem: so you should continue working on your patch and I'll
probably refactor on top of it later after its merged :P
Change-Id: I8ac9858b80d7908cf7e7981d7e19d0fc9c2265c0
Let Monolog insert exception_id and exception_url so that consumers
of logging data (such as Logstash) may provide a normalised message
that does not contain these variants, for ease of aggregation
and message trend counting.
Bug: T45086
Change-Id: I7cc8f8c9e68031ad6771593d390079c0a3a535b9
In phpcs.xml rename renamed sniffs and add the failing sniffs,
because now the whole sniff is no longer excluded.
Change-Id: If5b0bd16028761abc2c47ace9e97d37ad14bb36f
Having such comments is worse than not having them. They add zero
information. But you must read the text to understand there is
nothing you don't already know from the class and the method name.
This is similar to I994d11e. Even more trivial, because this here is
about comments that don't say anything but "constructor".
Change-Id: I474dcdb5997bea3aafd11c0760ee072dfaff124c
Also add a period at the end of the sentence.
This changes the HTML comment from
<!-- Set $wgShowExceptionDetails = true; at the bottom
of LocalSettings.php to show detailed debugging
information
-->
to
<!-- Set $wgShowExceptionDetails = true; at the bottom
of LocalSettings.php to show detailed debugging
information. -->
This is a follow-up to 842b7a1769 (T162315).
Change-Id: I45ff4f856ad1c0dc72066fce7771077ff4663785
We have a utility function for this, so let's use it.
What I don't understand though is why Sanitizer uses custom PHP implementations
for both tag stripping and entity decoding, instead of the built-in functions.
If there's a security reason for this or the built-ins are inadequate, that's
fine, but then that should be documented (and we should possibly ban usage
of the built-ins).
Change-Id: I2ba2ecd388cb3d9cd2360ecaa236f3d444f0eabf
Right now, ErrorPageError *assumes* you're never running on the cli
or the API. It's kinda a crappy superclass to use for errors unless
you're 1000% sure you'll never hit that code path. Yay assumptions!
Ideally, all of this report() crap is cleaned up and unified across
the like 1192902117 places we have it spread out, but for now just
detect the scenario and delegate back to MWException, which does the
right thing
Bug: T168337
Change-Id: Ia2f490528e128527a7a5ef1f4f5eea36ec9ee810
Remove the exit(1), which does not seem to be needed by any callers.
Doing so means that post-send updates can still happen, such as the
pushing of lazy jobs.
Better avoid showing exceptions in doPostOutputShutdown(), given
that an error may have already been shown. By the post-send part,
it's to late to show errors anyway.
Bug: T100085
Change-Id: Ib1c75323f222a0e02603d6415626a4b233e8e1c7
MWExceptionHandler::rollbackMasterChangesAndLog() only logged exceptions
if there were already master changes. This is extremely problematic when
debugging, especially in situations like DeferredUpdates where they were
silently being swallowed.
This makes it log exceptions in all paths, erring on the side of logging
the same exception twice (theoretically it's possible I suppose) instead
of not at all.
Also make the method able to handle DBError exceptions, which most of
the callers seemed to be assuming. ApiMain was handling this explicitly.
Bug: T168347
Change-Id: I8739051f824a455ba669344184c3b11ac95cb561
