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
For example, documenting the method getUser() with "get the User
object" does not add any information that's not already there.
But I have to read the text first to understand that it doesn't
document anything that's not already obvious from the code.
Some of this is from a time when we had a PHPCS sniff that was
complaining when a line like `@param User $user` doesn't end
with some descriptive text. Some users started adding text like
`@param User $user The User` back then. Let's please remove
this.
Change-Id: I0ea8d051bc732466c73940de9259f87ffb86ce7a
Guzzle is a vendor library included with MediaWiki, and it is
sometimes useful to directly use a GuzzleHttp\Client instance.
Add a function to HttpRequestFactory for this purpose.
Bug: T263816
Change-Id: I35f4b911676c7a58dfa5a8dd0d104125188dd62c
This makes the PSR-7 interface for the MWHttpRequest available to all
consumers that want to make use of it. This is of general utility and
should not be limited to Wikibase. Also, it allows us to drop the Guzzle
dependency from Wikibase entirely.
As an alternative to moving the adapter (as is done with this patch), it
was considered to refactor MWHttpRequest to support the interface
directly. While that would come without the overhead of an additional
class, it would require adding the interface methods to an already large
class and deprecating the now redundant legacy methods. This would be an
invasive refactoring that would not only affect the classes inheriting
from MWHttpRequest, but also its consumers.
It would seem that a conceptual refactoring of that magnitude would
require a more substantial consensus among contributors (and maybe
consumers) of these classes. Moving this adapter is seen as a step in
building the case for why using standardized interfaces is a good idea
and allowing for the discussion to be grounded of incorporating an existing
adapter into a base class rather than the hypothetical discussion of
adjusting it to an so far unused interface.
Bug: T263989
Depends-On: I56ad52b561460121a8c84313cbd431dc811e2ae1
Change-Id: I2a27dead1d5f1403b8d255c5daf9ba5e7c313476
The headers passed to Guzzle via the Client constructor are termed
"conditional", meaning that headers in the Request override them. If you
set a "body" option containing a MultipartStream, it sets a default
Content-Type request header, overriding the one from the Client
constructor. So to send a multipart stream with a custom Content-Type,
it is necessary to pass headers to Guzzle in the Request object.
Change-Id: Ie7bfc548ce4211fbf3ec67e3913bbdd3482977c3
These were never meant to be part of the public interface and should not
ever have been marked with @since. They're only useful for constructing
the respective objects, which no outside users should be doing.
Change-Id: I86e01272d46fc72af32172d8a12b9180971d4613
A terminating line break has not been required in wfDebug() since 2014,
however no migration was done. Some of these line breaks found their way
into LoggerInterface::debug() calls, where they mess up the formatting
of the debug log.
So, remove terminating line breaks from wfDebug() and
LoggerInterface::debug() calls.
Also:
* Fix the stripping of leading line breaks from the log header emitted
by Setup.php. This feature, accidentally broken in 2014, allows
requests to be distinguished in the log file.
* Avoid using the global variable $self.
* Move the logging of the client IP back to Setup.php. It was moved to
WebRequest in the hopes that it would not always be needed, however
$wgRequest->getIP() is now called unconditionally a few lines up in
Setup.php. This means that it is put in its proper place after the
"start request" message.
* Wrap the log header code in a closure so that variables like $name do
not leak into global scope.
* In Linker.php, remove a few instances of an unnecessary second
parameter to wfDebug().
Change-Id: I96651d3044a95b9d210b51cb8368edc76bebbb9e
* Use createMultiClient() in CdnCacheUpdate. This will reduce the
timeout from a hard-coded 900s to a configurable 25s. This is not
used in WMF production.
* Use createMultiClient() in the VirtualRESTServiceClient service. This
should have no effect in production since the service is broken per
T175224.
* Use the new createMultiClient() in the old createMultiClient(). The
configuration is the same except that the maximum timeouts are now
respected.
Bug: T245170
Change-Id: I63139d29471bc59e9ef60032fd812a1f24644113
* Add HttpRequestFactory::createMultiClient(), which returns a
MultiHttpClient with configured defaults applied. This is similar to
the recently-deprecated Http::createMultiClient().
* Introduce $wgHTTPMaxTimeout and $wgHTTPMaxConnectTimeout which, if set
to a lower value than their defaults of infinity, will limit the
applied HTTP timeouts, whether configured or passed on a per-request
basis. This is based on the frequently correct assumption that ops know
more about timeouts than developers.
* In case developers believe, after becoming aware of this new situation,
that they actually do know more about timeouts than ops, it is possible
to override the configured maximum by passing similarly named options
to HttpRequestFactory::createMultiClient() and
HttpRequestFactory::create().
* Apply modern standards to HttpRequestFactory by injecting a logger and
all configuration parameters used by its backends.
* As in Http, the new createMultiClient() will use a MediaWiki/1.35
User-Agent and the 'http' channel for logging.
* Document that no proxy will be used for createMultiClient().
Proxy config is weird and was previously a good reason to use
MultiHttpClient over HttpRequestFactory.
* Deprecate direct construction of MWHttpRequest without a timeout
parameter
Bug: T245170
Change-Id: I8252f6c854b98059f4916d5460ea71cf4b580149
Updated Doxygen markup in several .php files triggering warnings when mwdocgen.php is executed. Removed
obsolete settings MSCGEN_PATH and TCL_SUBST from Doxyfile. The former would generate a warning in 1.8.16
while TCL support was removed in 1.8.18. Since TCL_SUBST was blank anyway, it was removed prior to getting
to .18 in production. Increased DOT_GRAPH_MAX_NODES from 50 to 200 since Doxygen complained about it being
too low for API and Maintenance.
Bug: T248706
Change-Id: I9c67f0807d1b43089d351263d4f591dee5501f36
Currently GuzzleHttpRequest is not sending any cookies.
Furthermore, Guzzle expects its own format of CookieJar,
which is not compatible with CookieJar used by MediaWiki.
Solution is to add Guzzle Middleware that would obtain Cookie header
from MediaWiki CookieJar and add it to outgoing Guzzle requests.
Note: special handling of received cookies (Set-Cookie header
from the server) is unnecessary, this is not Guzzle-specific code
and is already done in MWHttpRequest::parseCookies().
Still, adding both a test of sending cookies and receiving cookies.
Bug: T245644
Change-Id: If69840e65d5671989cf15450964da3c691fd164a
Scalar casts are still allowed (for now), because there's a huge amount
of false positives. Ditto for invalid array offsets.
Thoughts about the rest: luckily, many false positives with array offsets
have gone. Moreover, since *Internal issues are suppressed in the base
config, we can remove inline suppressions.
Unfortunately, there are a couple of new issues about array additions
with only false positives, because apparently they don't take
branches into account.
Change-Id: I5a3913c6e762f77bfdae55051a395fae95d1f841
The MWHttpRequest is implemented by the
CurlHttpRequest class and also the
GuzzleHttpRequest class. However, curl based rendering set
the CURLOPT_POST which implies that the 'Content-Type'
header defaults to 'application/x-www-form-urlencoded'.
To homgonize the functionality this patch mimics the
curl behaviour in Guzzle.
Bug: T232866
Change-Id: Id60a8de18e5f1e750a3bde23bd8b0deca4071165
This is for classes with a single undeclared property - aside from
BlockManager: I3f51fd3579514b83b567dfe20926df2f0930dc85 removed the
declaration of $permissionManager without actually removing all uses.
Change-Id: Ic2a95f77071312041be6e0633ea9b5325e98de42
This allows us to remove many suppressions for phan false positives.
Bug: T231636
Depends-On: I82a279e1f7b0fdefd3bb712e46c7d0665429d065
Change-Id: I5c251e9584a1ae9fb1577afcafb5001e0dcd41c7
All methods got moved to HttpRequestFactory or MWHttpRequest or dropped.
I made the return value of the new HttpRequestFactory::request/get/post
methods null on error instead of false, so that when we drop PHP 7
support, we can use a "?string" return value. This could theoretically
change behavior of code that was switched from the old Http methods, but
probably won't. I kept the old behavior for the deprecated methods.
I changed the default value of $wgHTTPProxy from false to ''. This way
it should be usable directly without a trivial wrapper method. For the
benefit of anyone who might have set it to false in LocalSettings.php, I
also recommend casting to string just in case.
Http::$httpEngine is deprecated. Eventually it will be removed along
with the curl and PHP engines, leaving only the Guzlle engine.
I also added deprecation of MWHttpRequest::factory, which occurred in
1.31, to the release notes for 1.34. Now hopefully we can hard-deprecate
it in another couple of versions.
Bug: T214390
Change-Id: I2a316a758d793857f248bd251b90f5e9a6440e3a
Note there is an important difference between the two ways to express
this: strlen() does a string cast, but the `=== ''` and `!== ''`
comparisons will only detect empty strings, but not null, false, or any
other falsy value that becomes an empty string when cast to be one.
I am only touching code where I'm sure the variable is guaranteed to be
a string.
This change is done because I find the strict comparisons much more
readable. The code does exactly one thing now, and no magic casts any
more.
Change-Id: I3e908a0c7c7b6c29b0e5a1414f2ba9062a215b93
Given we now use Guzzle as well, and don't require cURL for any
features at a basic level, our code should not be as assumptious
about the backend being curl. Such assumptions may be wrong
and lead to confusion and/or bugs.
Bug: T137926
Change-Id: I6ad7f76768348e1eb8c1fb46c8125cce9285dc22
In order to be able to trace requests across the production environment,
we attach the X-Request-Id to outgoing requests.
Bug: T201409
Change-Id: Iadd6cbd51a78a1876a1a318783b51635717d054f
Activate the changes from T212175 by changing the default
MWHttpRequest-derived type to guzzle. This is being done
as a separate change so that it is conveniently revertable
if issues arise.
Bug: T214703
Change-Id: I0868d2e7f3eed6d7c6b15334eb3187f3edcd7bec
Provide backward compatibility for callback functions in
GuzzleHttpRequest, which was missing in T202110, and restore
GuzzleHttpRequest as the default provided by HttpRequestFactory.
Bug: T212175
Depends-On: I4b45e79d35252d13f714f3271b87301ca515121a
Change-Id: I60d1a034b44874f6d24a04058db264eeb565f5e1
Task T202110 included a change to recognize an HTTP status code
of 0 (zero) as an error, but it failed to set a status message,
resulting in an exception. Changed to set a status message of
'Error' so that required value is not empty.
Bug: T212005
Change-Id: I5fb78555bfcaeccdd726432f4dfc70924a385c41
Temporarily switching back to curl/php as the default for
HttpRequest objects, to resolve production issues until a
long-term fix for the out-of-memory issue from T211886 is implemented.
Bug: T211886
Change-Id: I6201ac1292b41f82d920a431376d791a666355e8
The postBody option to GuzzleHttpRequest can be passed as an array
or as a string. We were previously handling the array case incorrectly.
Bug: T211806
Change-Id: I8f40b9de9d40a9361eb45103608bf3aaa943bf73
Create a GuzzleHttpRequest class using the external Guzzle
(docs.guzzlephp.org) library. This will be the new default request type,
but CurlHttpRequest and PhpHttpRequest remain available and accessible
via Http::$httpEngine.
Bug: T202110
Change-Id: Ie720be2628d7baf427b002847f103fd86ee4cff3
Depends-On: I143a6410d111e75f01dbbfd43f300e2e60247451
