This reverts commit 0d453d915a.
Reason for revert: Trying to find a better approach than an abstract class
Change-Id: I69c0ec25cb920f41cb0eb6b3ad921d7b02290581
AbstractLocalizedException acts like a shim to ILocalizedException in preperation for refactoring
Bug: T287405
Change-Id: I01e40e82457d49487829d9a435b7a1198369ecf6
This helps phan to detect unreachable code and also impossible types
after the functions.
It helps phan to avoid false positives for array keys
when the keys are checked before
Bug: T240141
Change-Id: I895f70e82b3053a46cd44135b15437e6f82a07b2
Log INormalizedException messages in a structured way, allowing
the logging infrastructure to group them better.
Change-Id: I877909d1113ab93b4b8a115af5bb0fe039ea32d6
In a few places where a PermissionManager is used
but only GroupPermissionsLookup is needed
Also update references to the class in PermissionManager
that referred to it as GroupPermissionLookup
Change-Id: I5d7a13900852a38768a106aeee1ce012c3a04ea2
In order to allow Authority to know about user blocks,
we need a narrow interface to represent such blocks.
This deprecates some methods on AbstractBlocks in favor
of new methods on the Block interface that avoid binding to
the User class.
Bug: T271494
Change-Id: I7bb950533970984a014de0434518fbbefb695131
Follows-up f2543d442a, in which I accidentally removed the prefix.
Also, while at it, drop the "$class: " prefix that we normally
add to exceptions. For errors we internally construct ErrorException,
but this is something a clas name rarely seen by PHP developers and
has sometimes led to confusions about whether this is an exception
or not. We did still have "PHP Notice" and "PHP Warning" right after
it (except for deprecations) which should act as confidence indicator
that they are in fact not exceptions. We also have the channel
name ("error" instead of "exception"). But removing this will help
boost that confidence further, plus it had no added value anyhow, so
less is more?
Change-Id: Ib93f48c94d642f519558aab40d143b43e1d9ed16
This is micro-optimization of closure code to avoid binding the closure
to $this where it is not needed.
Created by I25a17fb22b6b669e817317a0f45051ae9c608208
Change-Id: I0ffc6200f6c6693d78a3151cb8cea7dce7c21653
* Add $wgRequestTimeLimit. If it is non-null, it starts a time limit,
using Excimer with a fallback to set_time_limit().
* Add pretty formatting of timeout exceptions thrown by the library.
Related refactoring.
* Expose the library's critical section feature in MediaWikiServices
* In Setup.php, call warnIfHeadersSent() before sending session headers.
This helped to debug a related issue I had.
* In wfTransactionalTimeLimit() use the new library, and respect the
existing library time limit if it is larger than
$wgTransactionalTimeLimit.
Bug: T269326
Depends-On: I6409ad8a5cba775c27b0d5a79d3300c4dac4c91a
Change-Id: I2e6f6351c451407c06cc7e20932548f7b62e36b6
This key predates MediaWiki's standardised notion of request IDs,
and of PSR-3 and Monolog usage.
Bug: T199607
Change-Id: Ibdd5bf12591761ab45be12ba72943e9d94f678cb
This should make error logs easier to work with through a couple
of ways:
* The stack trace is now complete, instead of missing the first
crucial step, which is often the one used for filtering
purposes and for identifying errors within a given deployed
version of MediaWiki. (E.g. when filtering out an error that is
expected to be fixed by the next release and/or when checking
how prominent an error currently is).
* Logstash reports that report message + trace will not need to be
edited by hand to include the file+line.
* The workflow for Logstash generally follows one of two patterns.
The default is to filter by exception.file (including line number),
which is very sure to catch all possible variants thrown from
the same code, regardless of any variables in the message, but
has the downside of not matching week-over-week consistency due to
file paths (at least for WMF) containing the deployment version.
The other option is to filter by message, which has the risk of
possibly excluding too much if there are multiple unrelated ways
to trigger the issue, but is a sensible second option. This is
usually done by filtering on normalized_message for non-exception
errors, but doesn't work well for exceptions because they contain
the file paths and do so in-between the class and message words,
and thus are not compatible with Logstash's default substring/term
match. The alternative of exception.message is then considered but
is lacking the class/type, which can be fragile.
With this change applied, no editing is needed, and no multiple
approaches need to be considered with the same option.
Either filtering by exception.file as-is, or filtering by
normalized_message as-is, regardless of whether it is an exception
error or other message in another channel, will both work.
Bug: T271496
Change-Id: I5908ed53f9b97b3c9cde126aca89ab6fc197c845
This reverts commit 900c6663b0.
Reason for revert: breaks Parsoid CI. Instead of fixing the test in presence of the FlaggedRevs hook, I would rather convert DefaultPreferencesFactory to Authority and fix the tests once instead of doing it twice.
Change-Id: Iaa440a9804c9ed97339e737162ef64ccf29ceb51
Taint check checks for possible security issues by tracking html
escaping and more by using phan.
This slows done the phan-job a bit and requires more ram
Keep the DoubleEscaped issues out to make reviewer easier
Adds suppression for false positives
Adds taint-annotation to help taint-check
Removes suppression for code phan now understand better by the tracking
of keys in taint-check
Fix some small issues by adding int cast or htmlspecialchars calls
Bug: T216348
Bug: T268920
Change-Id: I849ac4f120fd15b483e8939d4db45c98dc351259
A PHP fatal error from MWExceptionRenderer::output() causes the
exception handler to be re-entered, with the original exception thrown
away. So, catch all throwables. That way we can see both error messages,
neither is thrown away.
Bug: T263911
Change-Id: Ie98438cbdd328fe295c9b5202d79edb0c8fb41c5
This does not help to avoid the suppression inside the class,
but it avoids the suppression for all callers
Remove unneeded local vars while looking at the code
Bug: T269895
Bug: T216348
Change-Id: I5ce9a2dddca2b3bb556eb7adb5305178db09a71a
$file and $line are reported as possibly undefined in static code
analyzer, but the $level = 0 does not pass self::$fatalErrorTypes,
just return earlier
Value 0 is not used:
https://www.php.net/manual/en/errorfunc.constants.php
Change-Id: Ib8431d6ed496ee50447c8c976afe17ebac03e156
* Always use trigger_error for deprecation warnings, not just in development.
They are still silent from the run-time perspective (not thrown as
exceptions).
Previously this code path was only called when $wgDevelopmentWarnings
is set to true. For most dev environments and for CI, this means
nothing much changes given that DevelopmentSettings.php set this to true.
* In the code path that handles native PHP warnings, when setting the $file
and $line attribution that Logstash/Kibana report as "exception.file"
use the same offset as the one that wfDeprecated() has computed from
the back trace. This means it no longer (wrongly/uselessly) attributes
all deprecation warnings to MWDebug.php.
* Trim the message suffix from "Called from <method> in <file>" to
just "Called from <method>". This reduces noise and makes the message
more stable over multiple MW branches. The stack trace is still there
like before.
== Before (only with $wgDevelopmentWarnings) ==
> PHP Deprecated: Use of wfGetScriptUrl was deprecated in MediaWiki 1.35.
> [Called from MediaWiki::__construct in /var/mediawiki/includes/MediaWiki.php at line 67]
>
> Error from line 393 of /var/mediawiki/includes/debug/MWDebug.php
>
> #0 [internal function]: MWExceptionHandler::handleError()
> #1 /var/mediawiki/includes/debug/MWDebug.php(393): trigger_error()
> #2 /var/mediawiki/includes/debug/MWDebug.php(297): MWDebug::sendMessage()
> #3 /var/mediawiki/includes/debug/MWDebug.php(270): MWDebug::sendRawDeprecated()
> #4 /var/mediawiki/includes/GlobalFunctions.php(1032): MWDebug::deprecated()
> #5 /var/mediawiki/includes/GlobalFunctions.php(2548): wfDeprecated()
> #6 /var/mediawiki/includes/MediaWiki.php(67): wfGetScriptUrl(string)
> #7 /var/mediawiki/load.php(50): MediaWiki->__construct()
== After (always) ==
> Use of wfGetScriptUrl was deprecated in MediaWiki 1.35. [Called from MediaWiki::__construct]
>
> Error from line 67 of /var/mediawiki/includes/MediaWiki.php
>
> #0 [internal function]: MWExceptionHandler::handleError()
> #1 /var/mediawiki/includes/debug/MWDebug.php(293): trigger_error()
> #2 /var/mediawiki/includes/debug/MWDebug.php(270): MWDebug::sendRawDeprecated()
> #3 /var/mediawiki/includes/GlobalFunctions.php(1038): MWDebug::deprecated()
> #4 /var/mediawiki/includes/GlobalFunctions.php(2548): wfDeprecated()
> #5 /var/mediawiki/includes/MediaWiki.php(67): wfGetScriptUrl(string)
> #6 /var/mediawiki/load.php(50): MediaWiki->__construct()
Bug: T252923
Change-Id: I1d4a166b6dff8b0e19fce3fab409f4a89e734ee6
For compliance with the new version of the table interface policy
(T255803).
This patch was created by an automated search & replace operation
on the includes/ directory.
Bug: T257789
Change-Id: I17e5e92e24c708ffc846945a136347670a3a20c7
For compliance with the new version of the table interface policy
(T255803).
This patch was created by an automated search & replace operation
on the includes/ directory.
Bug: T257789
Change-Id: Ie32c1b11b3d16ddfc0c83a757327d449ff80b2e4
For compliance with the new version of the table interface policy
(T255803).
This patch was created by an automated search & replace operation
on the includes/ directory.
Bug: T257789
Change-Id: I5ffbb91882ecce2019ab644839eab5e8fb8a1c5f
For compliance with the new version of the table interface policy
(T255803).
This patch was created by an automated search & replace operation
on the includes/ directory.
Bug: T257789
Change-Id: If560596f5e1e0a3da91afc36e656e7c27f040968
Exceptions classes are nearly always value objects, and should in most
cases by newable.
Bug: T247862
Change-Id: I4faa8ec6ea8bc44086cfc8075b32d10eea61e9df
Per the Stable Interface Policy, PHP interfaces should not be
directly implemented by extensions, unless they are marked to be safe
for that purpose.
Bug: T247862
Change-Id: Idd5783b70fc00c03d57f5b1a887f0e47c4d7b146
This annotates classes that can safely be instantiated by
extensions, per the Stable Interface Policy.
Bug: T247862
Change-Id: Ia280f559874fc0750265ddeb7f831e65fd7d7d6a
This reminder, while useful to see on a web page during local
development or when live upgrading a small wiki, is not so
useful in error logs and production monitoring.
In addition to being general noise and boilerplate to have to
know to ignore (and to know where to look instead), it has the
unfortunate side-effect of letting the 255-char trimmed normalized
messages containing none of the actual errors.
Remove this from the DBQueryError exception message, and place
it instead in the code paths of:
* MWExceptionRenderer::getHTML, used when a fatal error
happens but we are still able to render it in a skinned
output page.
Test Plan:
- Edit SpecialBlankpage.php#execute, and add the following,
which involves a non-existant 'pagex' table:
$db = wfGetDB( DB_REPLICA );
$db->select( 'pagex', '1', 'foo = 2', __METHOD__ );
- View Special:Blankpage on your wiki
* MWExceptionRenderer::output (main 'else' branch), used
when a fatal error is unable to recover and thus render
a plain text HTML response.
Test Plan:
- Edit SpecialBlankpage.php#execute, and add the following,
which involves an SQL syntax error:
$db = wfGetDB( DB_REPLICA );
$db->select( 'page', '1', 'foo =/ 2', __METHOD__ );
- View Special:Blankpage on your wiki
Bug: T255202
Change-Id: Ie08199ced767486f9e049934a334a1438f266aa6
No behavioural change. This change in abstraction is prep
for I1d4a166b6df and makes that diff cleaner.
Change-Id: If3f5836fcb2fc0c16aa1c3bdc1333d8c8f892f3b
Migrate all callers of Hooks::run() to use the new
HookContainer/HookRunner system.
General principles:
* Use DI if it is already used. We're not changing the way state is
managed in this patch.
* HookContainer is always injected, not HookRunner. HookContainer
is a service, it's a more generic interface, it is the only
thing that provides isRegistered() which is needed in some cases,
and a HookRunner can be efficiently constructed from it
(confirmed by benchmark). Because HookContainer is needed
for object construction, it is also needed by all factories.
* "Ask your friendly local base class". Big hierarchies like
SpecialPage and ApiBase have getHookContainer() and getHookRunner()
methods in the base class, and classes that extend that base class
are not expected to know or care where the base class gets its
HookContainer from.
* ProtectedHookAccessorTrait provides protected getHookContainer() and
getHookRunner() methods, getting them from the global service
container. The point of this is to ease migration to DI by ensuring
that call sites ask their local friendly base class rather than
getting a HookRunner from the service container directly.
* Private $this->hookRunner. In some smaller classes where accessor
methods did not seem warranted, there is a private HookRunner property
which is accessed directly. Very rarely (two cases), there is a
protected property, for consistency with code that conventionally
assumes protected=private, but in cases where the class might actually
be overridden, a protected accessor is preferred over a protected
property.
* The last resort: Hooks::runner(). Mostly for static, file-scope and
global code. In a few cases it was used for objects with broken
construction schemes, out of horror or laziness.
Constructors with new required arguments:
* AuthManager
* BadFileLookup
* BlockManager
* ClassicInterwikiLookup
* ContentHandlerFactory
* ContentSecurityPolicy
* DefaultOptionsManager
* DerivedPageDataUpdater
* FullSearchResultWidget
* HtmlCacheUpdater
* LanguageFactory
* LanguageNameUtils
* LinkRenderer
* LinkRendererFactory
* LocalisationCache
* MagicWordFactory
* MessageCache
* NamespaceInfo
* PageEditStash
* PageHandlerFactory
* PageUpdater
* ParserFactory
* PermissionManager
* RevisionStore
* RevisionStoreFactory
* SearchEngineConfig
* SearchEngineFactory
* SearchFormWidget
* SearchNearMatcher
* SessionBackend
* SpecialPageFactory
* UserNameUtils
* UserOptionsManager
* WatchedItemQueryService
* WatchedItemStore
Constructors with new optional arguments:
* DefaultPreferencesFactory
* Language
* LinkHolderArray
* MovePage
* Parser
* ParserCache
* PasswordReset
* Router
setHookContainer() now required after construction:
* AuthenticationProvider
* ResourceLoaderModule
* SearchEngine
Change-Id: Id442b0dbe43aba84bd5cf801d86dedc768b082c7
Make sure that CAUGHT_BY_HANDLER is only for errors caught by the
handler from MWExceptionHandler::installHandler().
Add CAUGHT_BY_ENTRYPOINT constant for entrypoint try/catch logic,
e.g. MediaWiki::run()/ApiMain::executeActionWithErrorHandling().
Use Throwable to catch more types of errors given that PHP 7.2
is already required.
Change-Id: Ib496e26572c98d771a772972676c02c05b872e31
