The "${var}" and "${expr}" style string interpolations are deprecated in
PHP 8.2. Migrate usages in core to "{$var}" as appropriate.
Bug: T314096
Change-Id: I269bad3d4a68c2b251b3e71a066289d4ad9fd496
MediaWiki still supports PHP 7.2+, but we want to mainly test in newer versions
of PHP. Setting minimum_target_php_version to 7.2 this lets us run phan without
phan trying to get us to make PHP 7.2-incompatible changes to 'appease' PHP 8.0
or whatever later changes.
Some switches of generic 'resource' type-hinting to 'resource|object' to inform
phan to ignore this (triggering PHPCS at the time, ah well), rather than trying
to hint the specific novel PHP encapsulation classes to that have replaced them
from PHP 8.0 onwards but don't yet exist, and fixes from where we were checking
the results of implode and explode.
Bug: T293924
Change-Id: I629e3fb3adfad73beb3d424a07e643c2e079d9bb
Make phan stricter about array keys
Remaining false positive issues are suppressed.
The suppression and the setting change can only be done together
Bug: T304887
Depends-On: I3105a5fd4826f8667b5232834defc5ec93be32a1
Depends-On: Ie9610a6e83731468311edb3ed17f80fc509de385
Change-Id: I701f12ab94478c3b8e7fd82110ade74a8e6b04ef
$wgHTTPMaxTimeout and $wgHTTPMaxConnectTimeout used to default to INF,
the positive infinity float value. This value has no representation in
JSON. In order to allow default configuration values to be defined in
a JsonSchema, we allow 0 to be used to represent "no limit" instead.
Bug: T294788
Change-Id: Id1b832b46ac6984655dcf0c06d5af7d356cca800
* Enforce the ban on is_resource in phpcs
* In OrderedStreamingForkController, the comment was incorrect. I
confirmed using a small test script that if the child closes one end
of a socket pair, the other end will still be open, and is_resource()
will still return true, and fclose() will not fail. The issue was
introduced in c82d30d19c, it was not present in the
CirrusSearch copy of the class.
* Allow is_resource() for debug logging.
* Allow is_resource() for parameter validation where a stream may be
passed to a function, since there is no alternative.
Bug: T260735
Change-Id: I59a752f7bb4b043ddfb2434b52a02f9221bf54db
Keep using pipelining as long as it is supported. Add covering test,
mostly to check for exceptions.
No need to set it to zero if the option is false, since zero is the
default.
Bug: T264735
Change-Id: I1a3873c27d2002a7374b98548a8c43065ea0d8ba
These are useful when debugging requests that vary behavior based on
the headers specified, like Swift's x-copy-from. Additionally,
knowing the content-length is helpful when looking at how long
the request took.
We do know that some headers are likely to contain sensitive
passwords/tokens, so try to redact those ahead of time using a regex of:
(authorization|auth|password|cookie)
This will catch the standard Authorization header, plus Swift's
X-Auth-Token and X-Auth-Key. The regex is a best effort, but not
intended to be fully comprehensive as there's already plenty of
private data being included in logs.
Change-Id: I6e695f357f92ce8d40b54ea39002fb7858f5596e
The default not configured value for $wgLocalHTTPProxy is false, not
null. So in MultiHttpClient make sure it defaults to false and check for
false when deciding whether to use the proxy.
MWHttpRequest already has the correct behavior.
Bug: T296312
Change-Id: I3962c24f9cb159fb7d1e194e59c8bef5261371c3
This follows the same approach as MWHttpRequest, we inject
$wgLocalVirtualHosts, and for each request check whether it matches that
domain list, rewriting the request as necessary.
Unfortunately this requires a decent amount of code duplication because
MultiHttpClient is in includes/libs/ and can't depend on the same code
in MWHttpRequest.
Bug: T288848
Change-Id: Ia16d8f86b1cb20dde9fe487729d67d92af650cfe
Since 1.34 setting non-default HTTP engine
has been deprecated. It's time to remove
the old implementations. Only Guzzle is
now available.
Change-Id: I978b75827e69db02cbc027fe0b89a028adfc6820
For context see T275752, but the premise is that we want to be able to
set the HTTP protocol version so that for specific use cases. That way
we will be able to use the proper protocol per use case, mitigating
potential future regressions or selectively rolling out a newer protocol
version per use case. Support setting CURLOPT_HTTP_VERSION, while making
sure to default to the current default which lets curl decide which
version to use
CurlHttpRequest already forcibly set this to 1.0 for what is worth
Bug: T275752
Change-Id: I82f4f174997ecaa54e33eb848b5f007c982506dd
* Send Swift's MultiHttpClient logs to the FileOperation channel
* Add more detailed debugging information to those logs, including
request execution time.
Bug: T276017
Change-Id: I221be1f452e03d19b7cd81430a9349a9db14f81f
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
This is both dead code and if it works, it contradicts the fact that
cURL library is not officially required by MediaWiki.
The function existence has already been checked in self::isCurlEnabled()
so it should be presumed to exist here because the call is guarded
behind that check.
The class level doc also said if cURL is not available, requests will be
made serially, it does not say Exception will be thrown which will
effectively makes cURL a hard dependency for MediaWiki and makes the
serial request code useless.
Follows-up 0584339f5e (I2f9d4acbb49).
Change-Id: I7f172f177d7d17c80bb70b2fd6c67107f4340c21
Per my caller survey in the linked bug, it is hard to find a caller
which actually needs a long timeout, but many callers used the default
timeout without reviewing it. The default was increased from 300 to 900
as a quick hack to fix T226979, but that has now been fixed by overriding
the timeout in SwiftFileBackend specifically.
Reduce the default to 30 since that is a more reasonable value to use when
serving web requests. Since everything that previously used this default
was migrated to use HttpRequestFactory::createMultiClient(), the timeout
for them was already reduced from 900 to 25 with no apparent ill
effects. This cleanup change should have no production impact.
Bug: T245170
Change-Id: Id6029afa4e3f1c6551cd823c3b0def01afcdc571
* 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
Requested by a user at https://www.mediawiki.org/wiki/Topic:Vkk1ahk3eggd9747 for
whom their hoster provides curl but with multi-threaded functions removed for
some reason.
Change-Id: Id3877c600ae02feffb67f74a815430f8e679230a
Released just now.
Many old suppressions can now be removed. Enabling the issue for
undeclared variables is left to do later, given that there are
roughly 200 warning.
Change-Id: I99462a1e9232d6e75022912e2df82bc2038476ef
We set Curl to follow the redirect with CURLOPT_FOLLOWLOCATION,
however the response headers are being concatenated into a
comma-separated list. Curl follows redirect silently under the hook,
so we end up with a combination of headers from a redirect response
and an after-redirect response. We need to reset the headers.
Change-Id: I9a0f208e5479e5c126d36d788c545a23ff421438
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
All the magic functions needs public visibility to be callable from php
internals like garbage collector
Change-Id: I1baf04bf8ff787da880d46e4a6daa77f5a6de73f
Under buster, curl uses HTTP/2 (confirmed when running eval):
Buster:
GET xxx HTTP/2
Stretch:
GET xxx HTTP/1.1
The code presumes that it will always be HTTP/1.x.
We fix this by adjusting the regex to match HTTP2.
Bug: T232866
Change-Id: Ibde6036048d5939508df143ec5956abcd0718ad1
Use CURLMOPT_MAX_HOST_CONNECTIONS to enforce concurrent request limits.
This gives better concurrency than using naïve array_chunk() batches, which
were serialized and treated all URLs as pessimistically from the same host.
Allow connection reuse for multi-URL request batches. This avoids overhead
from reconnections and reduces the number of TIME_WAIT handles when many
batch operations happen in a short time frame. Previously, the use of the
CURLOPT_FORBID_REUSE flag meant that connections were cached but never
reused for multi-URL batches (only single-URL batches).
Connection limits can be verified by running large runMulti() batches
in an interactive shell and inspecting netstat for TCP connections.
Bug: T232128
Change-Id: I851fdc69e0d0d9f9c95dc42781f24d2077a12470
Use CURLMOPT_MAX_HOST_CONNECTIONS to enforce concurrent request limits.
This gives better concurrency than using naïve array_chunk() batches, which
were serialized and treated all URLs as pessimistically from the same host.
Allow connection reuse for multi-URL request batches. This avoids overhead
from reconnections and reduces the number of TIME_WAIT handles when many
batch operations happen in a short time frame. Previously, the use of the
CURLOPT_FORBID_REUSE flag meant that connections were cached but never
reused for multi-URL batches (only single-URL batches).
Connection limits can be verified by running large runMulti() batches
in an interactive shell and inspecting netstat for TCP connections.
Bug: T232128
Change-Id: I5c5f1eceb3fdb501a8f22f2b949756065f12379a
