Commit graph

354 commits

Author SHA1 Message Date
James D. Forrester
0958a0bce4 Coding style: Auto-fix MediaWiki.Usage.IsNull.IsNull
Change-Id: I90cfe8366c0245c9c67e598d17800684897a4e27
2020-01-10 14:17:13 -08:00
Kunal Mehta
99007e96c7 Use namespaced IPUtils class
Change-Id: I047e099a93203a59093946d336a143d899d0271f
2020-01-01 02:36:49 -08:00
Umherirrender
925e3eb30b Set method visibility in some classes
Change-Id: I3c3d59d4b3edf2459efeac890721a43475e27198
2019-12-05 17:42:55 +00:00
Tim Starling
164a3ac1f0 Remove IE 6 security features from server-side code
* Deprecate WebRequest::checkUrlExtension() and have it always return
  true. This reverts the security fixes made for T30235.
* Remove IEUrlExtension. This is a helper for checkUrlExtension() which
  is not used in any extensions.
* Remove CSS sanitization code which is specific to IE6. This reverts
  the changes made to fix T57332, and related followups. I confirmed
  that the relevant test cases do not result in XSS on IE8.
* Remove related tests.

Bug: T232563
Change-Id: I7318ea4a63210252ebc64968691d4f62d79a63e9
2019-11-28 15:11:56 +11:00
James D. Forrester
7bcd897dd6 WebRequest: Update comment with a TODO now that HHVM is gone
Change-Id: I444d1be4d7194e1d0c705ac9a4e79c73e69a1c78
2019-10-10 12:56:42 -07:00
Timo Tijhof
480b7341f8 Setup: Move wgArticlePath validation to its main consumer (PathRouter)
The variable is also read in a few other places, such as to
export the value from api.php (siteinfo) and load.php (mw.config)
but those requests don't need to be held back by this extra
logic.

Alternatively, if we really want to require this for all consumption,
we should probably let PathRouter provide the value and require
consumers to use it. E.g. services->getPathRouter->getArticlePath,
or something like that.

As easy first step, I'm moving it to PathRouter, called from
WebRequest::getPathInfo which is still called on all index.php
requests for any wiki page action in any namespace (incl Special)
when the wiki uses anything other than the default 'index.php?title='
article path.

Test Plan:
* Set '$wgArticlePath = 'bla';`
* View /mediawiki/index.php/Main_Page, and observe the fatal
  error message (same as before this change).

Bug: T189966
Change-Id: Id06c2557e2addb58faeef0b6f7767a65b8de55a5
2019-09-25 23:06:52 +00:00
Daimona Eaytoy
dddc912fd6 Update docblocks for methods potentially returning null
These were discovered by setting `null_casts_as_any_type` to true in
phan, and filtering by `PhanTypeMismatchReturnNullable`. Of course there
are others, some of which are false positives, but we cannot suppress
them now (or the UnusedSuppressionPlugin will complain).

Change-Id: Ia8443e575c22f47a6d8c63038f4e7ac36815fc27
2019-09-15 15:22:39 +00:00
jenkins-bot
28d9e60182 Merge "Stop mangling $_GET and provide WebRequest::getQueryValuesOnly()" 2019-09-06 17:38:26 +00:00
Tim Starling
0c0676c34e Stop mangling $_GET and provide WebRequest::getQueryValuesOnly()
I doubt there was ever a good reason for mangling $_GET to add the
title, this was just b/c for the sake of b/c. It was formerly used in
core but that was so long ago that I doubt there was any usage in
extensions at the time. Now there is one usage of $_GET['title'] in an
unmaintained extension, but it was only added in 2017.

Also I added WebRequest::getQueryValuesOnly() which is an interface to
the unmodified $_GET. The motivation is allowing OAuth to work with the
REST API, since OAuth needs an unmangled view of $_GET for signature
generation. The Action API gets around the problem with a special hack
in interpolateTitle(), disabling it for the Action API only.

A review of callers of getQueryValues() suggests that many would
benefit from using getQueryValuesOnly() instead. But I only changed it for
callers in api.php and thumb.php since the effect of the change there is
certainly beneficial, whereas callers under index.php may possibly be using
the path parameters to construct self-links.

Rest\RequestFromGlobals uses $_GET directly, which means that this
change causes it to not return PathRouter matches as GET parameters
anymore.

Change-Id: Ic469577fae17c0b1ac69466df7bc9f03e61c74e3
2019-09-05 15:00:28 +10:00
Timo Tijhof
64281b6c52 Setup: Move wgActionPath logic to PathRouter
This is only relevant when processing page views or when constructing
Title urls with an 'action' query. Pretty important stuff, and worth
optimising for if we had to choose, but we can defer it in this case
without slowing it down, which is better for everything else.

It also means we don't mutate configuration (beyond setting whole values
as dynamic defaults), which seems desirable, and makes the overall behaviour
easier to test. Handling absence of 'view' should be PathRouter's
responsibility, not Setup.

Bug: T189966
Change-Id: I9c1eea2dcea74be0e283eb2b175268315ced1793
2019-09-04 20:29:03 +00:00
Timo Tijhof
fe889b7d6b WebRequest: Optimise WebRequest::getPathInfo()
Called for all PHP web requests from WebStart.php via
WebRequest::interpolateTitle.

* Use isset() instead of empty() where we only need to check
  that the key is supported.

* Don't import global `$wgUsePathInfo` in the common case.

* Migrate from deprecated wrapper Wikimedia\suppressWarnings
  to AtEase::suppressWarnings.

* Use strpos() instead of preg_match(). Consistently faster,
  albeit not by much (for 100 iterations: 0.04ms vs 0.23ms).

* Don't create unused $matches array for the common case.

Bug: T189966
Change-Id: I0de126953c25f3629cb85a0d4e46598baf261c15
2019-09-03 18:32:21 +00:00
Daimona Eaytoy
5eac6d131c Unsuppress more phan issues (part 3)
Bug: T231636
Depends-On: I78354bf5f0c831108c8f606e50c87cf6bc00d8bd
Change-Id: I58e67c2b38389df874438deada4239510d21654f
2019-08-31 16:38:55 +00:00
Aaron Schulz
1bb1ff8dd3 Short-circuit WebRequest::getGPCVal() for printable ASCII strings
This avoids accessing the content Language instance in many cases

Change-Id: I82bd66496180f947c1af16c2728a1548df03dcf9
2019-08-23 14:53:38 +00:00
Fomafix
110a5877e9 Use [...] instead of array(...) in PHP comments and documentation
Change-Id: I0c83783051bf35fe785bc01644eeb2946902b6b2
2019-06-17 21:15:09 +02:00
Marko Obrovac
01c2e25f0a Allow the request ID to be passed in via the X-Request-Id header
For tracing and logging purposes, we want to be able to see/generate the
list of all of the requests that happen in the environment for a given
external incoming request. To that end, allow Mediawiki to accept the
request ID provided by the incoming request as its own.

Since this may be problematic for set-ups that don't have an entity in
front of MW that sanitises the headers on the way in, introduce a new
global variable, `$wgAllowExternalReqID`, that can disable this
behaviour. By default, the feature is disabled.

Bug: T201409
Change-Id: I605471fb8b5bbc290baeecc7d80d9d715cb240c9
2019-05-15 07:27:38 +00:00
Timo Tijhof
7929d190b4 WebRequest: Change getFullRequestURL() to use local getProtocol()
Previously it relied on wfGetServerUrl to decide the url,
which passed PROTO_CURRENT on to wfExpandUrl(), when then uses
global $wgRequest and checks getProtocol() on that.

For typical web requests, these would be the same object by
reference, but the needless indirection makes the code harder
to reason about and harder to test.

Instead, check this locally and pass on an explicit HTTP or HTTPS
to wfGetServerUrl/wfExpandUrl.

Change-Id: I797bd2f909625536c3af36f015fb2e94bf922ba9
2019-04-20 00:58:17 +01:00
Fomafix
9cbb8f104d Use https://www.php.net/ instead of https://secure.php.net/
Change-Id: I0acca592c6909e91b28b904da49dcbd6a43cd2a5
2019-04-12 06:44:48 +02:00
Fomafix
2c75885c6f WebRequest: Simplify getGPCVal
* Call $contLang->checkTitleEncoding() only with a string value as
  parameter. (A boolean value here caused T218883.)
* MediaWikiServices::getInstance()->getContentLanguage() always return a
  Language object. An exist check is not necessary.

Change-Id: Idd3e3a2baa5072e862d7502e30079a1b33d6a866
2019-03-21 20:47:57 +01:00
Shreyas Minocha
d5b2a1621b
Fix small typos
"publically" → "publicly"

Change-Id: I712d97c0504d52c96532b4aac4f332ef39670955
2019-01-27 17:40:18 +05:30
Umherirrender
a4caa4d0c6 build: Updating mediawiki/mediawiki-codesniffer to 22.0.0
Added spaces around .
Removed empty return statement which are not required
Removed return after phpunit markTestIncomplete,
which is throwing to exit the test, no need for a return

Change-Id: I2c80b965ee52ba09949e70ea9e7adfc58a1d89ce
2018-09-16 15:51:11 +00:00
Aryeh Gregor
90d4f56fe4 Mass conversion of $wgContLang to service
Brought to you by vim macros.

Bug: T200246
Change-Id: I79e919f4553e3bd3eb714073fed7a43051b4fb2a
2018-08-11 22:44:29 -06:00
Tim Starling
f6d582a91e Avoid a redirect loop when the request URL is not normalized
If the request URL was not normalized, for example having a double slash
in it, this could cause it to fail to match in the PathRouter. But the
canonicalizing redirect was using the normalized URL, causing a redirect
loop exception.

So:

* If the PathRouter fails to match with the original URL, try matching
  against the normalized URL. This allows it to still work for
  normalized URLs with a double slash in the title part of the path.
* Have WebRequest::getFullRequestURL() always return the URL without
  removing dot segments or interpreting double slashes. Just append
  the path to the server.
* Make MediaWikiTest.php use WebRequest instead of FauxRequest, allowing
  it to reproduce the exception in question. Add relevant test.
* Add tests for the new PathRouter behaviour.

Bug: T100782
Change-Id: Ic0f3a0060904abc364f75dae920480b81175d52f
2018-07-16 15:55:59 +10:00
Umherirrender
130ec2523d Fix PhanTypeMismatchDeclaredParam
Auto fix MediaWiki.Commenting.FunctionComment.DefaultNullTypeParam sniff

Change-Id: I865323fd0295aabd06f3e3c75e0e5043fb31069e
2018-07-07 00:34:30 +00:00
Bartosz Dziewoński
485f66f174 Use PHP 7 '??' operator instead of '?:' with 'isset()' where convenient
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
2018-05-30 18:06:13 -07:00
Kunal Mehta
06ca92eb8c Re-enable MediaWiki.Usage.SuperGlobalsUsage.SuperGlobals sniff
Disable it in specific files and places where there are legitimate uses
to access $_GET and $_POST directly.

For EditPage, which wants to output $_POST for debugging information,
introduce WebRequest::getPostValues() as a wrapper, matching the
existing ::getQueryValues().

Change-Id: I2cb0a7012fb7ed29dcd720056b42f56508ddc5fa
2018-05-19 15:07:25 -07:00
James D. Forrester
de6dab71e3 Remove $wgScriptExtension (deprecated and ignored since 1.25)
* Remove left-over mention of the .php5 entry points in docs.

* Remove dead logic in NoLocalSettings for php5 entry points.

* Remove dead match in WebRequest for php5 entry points (they'd
  redirect since 1.25, and not seen by PHP).

Change-Id: Ia0ee8588591860b8fe34030c8503f38e9bce31f3
2018-04-19 01:11:23 +01:00
Aryeh Gregor
d3da5e08d3 Improve test coverage for ApiBase.php
One bug fixed: a timestamp of '00' or similar would get interpreted as
'now' by mistake instead of Unix timestamp 0, without throwing the
warning for using 0 instead of 'now'.  This is because it called
wfTimestamp() once to parse the input date, got a Unix timestamp of 0
back, and then tried passing that 0 back to wfTimestamp again to
reformat as a wiki date, but it got reinterpreted as 'now'.

Also fixed parameters with type "user" to validate usernames more
correctly.  This might be risky, though, if I missed any valid
usernames, or if API clients were for some reason relying on passing in
invalid usernames.  If we don't actually want to do this, we should add
a comment explaining why we're allowing any title without a fragment
rather than validating properly.

Still lots more work to do here.

Change-Id: I56b4290263df8698efdbddda71a7eabd9e303abc
2018-04-08 15:51:42 +03:00
jenkins-bot
9e0e67658e Merge "exception: Improve formatting of fatal error log messages" 2018-03-21 19:52:38 +00:00
Gergő Tisza
8ee55867c6 exception: Improve formatting of fatal error log messages
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
2018-03-21 19:27:12 +00:00
Kevin Israel
06ba5ca383 Remove internal use of deprecated $wgRequestTime
* Use $_SERVER['REQUEST_TIME_FLOAT'] unconditionally in WebRequest.php
  and libs/Timing.php. WebStart.php was doing this already without issue.
  The key existst since PHP 5.4, for both Web and CLI (we require 5.5).

* In wfDebug() and wfReportTime(), use $_SERVER['REQUEST_TIME_FLOAT'] instead.

* In ApiFormatBase and MWDebug, use WebRequest::getElapsedTime() instead.

* In Maintenance.php, remove setting of $wgRequestTime.

* In rebuildFileCache.php, update mocking to $_SERVER['REQUEST_TIME_FLOAT']
  so that we avoid re-introducing bug T24852.

Change-Id: I1b647da2862f815029caa533b592ec8a05b33806
2018-03-18 04:41:12 +00:00
Reedy
39f0f919c5 Update suppressWarning()/restoreWarning() calls
Bug: T182273
Change-Id: I9e1b628fe5949ca54258424c2e45b2fb6d491d0f
2018-02-10 08:50:12 +00:00
Umherirrender
3124a990a2 Use ::class to resolve class names in includes files
This helps to find renamed or misspelled classes earlier.
Phan will check the class names

Change-Id: I07a925c2a9404b0865e8a8703864ded9d14aa769
2018-01-27 20:34:29 +01:00
Thiemo Mättig
ef470ebf7f Remove @param comments that literally repeat what the code says
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
2018-01-10 14:14:26 +01:00
James D. Forrester
9635dda73a includes: Replace implicit Bugzilla bug numbers with Phab ones
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
2017-02-21 18:13:24 +00:00
Ricordisamoa
06c8656d95 Fix documentation comments for some WebRequest methods
WebRequest methods getRawVal(), getVal() and getArray()
can return null.

Change-Id: I555dfd93c7cdebc83aab89f3efe4de3018bc9de0
2017-02-20 22:25:02 +01:00
Victor Barbu
1bf8e81cdd Remove WebRequest::checkSessionCookie() method as being deprecated
Bug: T61113
Change-Id: I5285dbfc47d6429e16b7e7839bf55d48320d0bf0
2016-12-31 17:37:04 +00:00
jenkins-bot
9ac29c74ed Merge "Cleanup some incorrect return annotations" 2016-12-16 07:22:24 +00:00
jenkins-bot
3a2853e218 Merge "Add <!DOCTYPE html> to HTML responses" 2016-12-16 07:16:45 +00:00
Erik Bernhardson
d67197fa11 Cleanup some incorrect return annotations
Most of these are simply changing annotations to reflect
reality. If a function can return false to indicate failure
the @return should indicate it.

Some are fixing preg_match calls, preg match returns 1, 0 or false,
but the functions all claim to return booleans.

This is far from all the incorrect return types in mediawiki, there
are around 250 detected by phan, but have to start somewhere.

Change-Id: I1bbdfee6190747bde460f8a7084212ccafe169ef
2016-12-12 10:15:05 -08:00
Fomafix
202f695f67 Update weblinks in comments from HTTP to HTTPS
Use HTTPS instead of HTTP where the HTTP link is a redirect to the HTTPS link.

Also update some defect links.

Change-Id: Ic3a5eac910d098ed5c2a21e9f47c9b6ee06b2643
2016-11-07 15:24:46 +01:00
Fomafix
e101fa901b Add <!DOCTYPE html> to HTML responses
Change-Id: I080040913c4c9750104bc88b643a1ffdfd222502
2016-09-25 17:36:41 +02:00
Kunal Mehta
39ee83f388 Move IP::isConfigured/TrustedProxy() to ProxyLookup service
This creates a new ProxyLookup service to house the
IP::isConfiguredProxy() and IP::isTrustedProxy() functions. The main
purpose of this refactoring is to make the IP class entirely independent
from MediaWiki, so it can be split into a separate library.

Change-Id: I60434a5f3d99880352bc0f72349c33b7d029ae09
2016-09-21 20:02:09 -07:00
Timo Tijhof
83df6d8123 WebRequest: Use getRawVal instead of getGPCVal where possible
Avoid getGPCVal() for simple methods that only need a boolean,
number or specific string outcome.

This saves overhead of touching $wgContLang or UtfNormal\Validator
and makes load.php initialisation no longer depend on it.

Change-Id: I8ce1fa31f5102b3fa18b0d0b9f56c42cb90146a1
2016-09-09 15:46:08 -07:00
Timo Tijhof
1ac5474b7b WebRequest: Add more unit tests
* Complete detectServer() coverage,
  test $wgAssumeProxiesUseDefaultProtocolPorts.
* Complete getAcceptLang() coverage.
* Add tests for getGPCVal() normalisation.
* Add tests for other getter methods.

Also:

* Ignore __construct() coverage as it only sets up properties from
  global state. The use of those properties are covered.

* Make normalizeUnicode() visibility explicit.

Change-Id: I6504136e6df47e504bc2e0e91fedddd2625f19d9
2016-09-08 21:58:09 -07:00
jenkins-bot
80a372f957 Merge "Expand SessionManager / AuthManager documentation" 2016-08-30 19:11:13 +00:00
Gergő Tisza
94e2aa7b55 Expand SessionManager / AuthManager documentation
Bug: T110628
Bug: T142154
Change-Id: Ib0a41f01b3d12267b2a94ea1375e6d13cacd2b69
2016-08-30 18:54:30 +00:00
Brad Jorsch
75a85b412c API: Use U+001F (Unit Separator) for separating multi-valued parameters
When a multi-valued parameter's value begins with U+001F, the values
will be split on that character instead of pipes. This will be useful
for things such as action=options&change= or meta=allmessages&amargs=.
Since MediaWiki doesn't otherwise accept C0 control characters
(WebRequest::getVal() replaces them with �), there's no possibility that
this will conflict with a literal use of U+001F.

Special:ApiSandbox and mw.Api are updated to make use of this, with the
latter having an option to disable the behavior in case something is
depending on [ 'foo', 'bar|baz' ] turning into 'foo|bar|baz'.

Pipe is still used as the separator when the value doesn't begin with
U+001F, and will be forever since it's generally more human-friendly and
is needed for backwards compatibility with basically every API client in
existence. The requirement that the value begin with U+001F, rather than
simply contain U+001F, is to avoid clients having to somehow
special-case "param=foo|bar" where that's intended to be a single value
"foo|bar" rather than two values "foo" and "bar".

Bug: T141960
Change-Id: I45f69997667b48887a2b67e93906364a652ace5a
2016-08-29 11:00:25 -04:00
Kevin Israel
5063fa6ee8 WebRequest::getText(): Update more of the doc comment
Support for server-side input transliteration, not just its
implementation for Esperanto, was removed in 3b5f60f2c8.
Now we only do Unicode and line ending normalizations.

Change-Id: Ie6172ee2d76a79006286a5f3c51b89bb762c258e
2016-07-21 03:37:10 -04:00
Brion Vibber
3b5f60f2c8 Remove old Esperanto character conversion support
Deletes LanguageEo.php class which only had remains of the server-side
character conversion (sx <-> ŝ, etc). This is being obsoleted in favor
of client-side IMEs provided by UniversalLanguageSelector extension.

Removes deprecated $wgEditEncoding, which was only used for this.

Turns Language::recodeInput() and Language::recordForEdit() into no-ops
for any old or extension code that happened to still use them.

Bug: T62677
Change-Id: Ib647353538d258dee941f2f7c571191060bc9c7d
2016-07-18 19:20:49 +00:00
Aaron Schulz
7f8d016f5a Avoid DBPerformance warnings on PURGE/TRACE requests
The former sometimes show up in the logs as they were causing
CentralAuth to use the master but the expectations treated
the request as a GET request. This makes things more
consistent.

Bug: T92357
Change-Id: I55bf3139c68f5926fe67a51cf0eb1b2ffe55d17b
2016-05-25 18:22:26 -07:00