Page MenuHomePhabricator

Support running MediaWiki without 'curl' PHP extension
Closed, ResolvedPublic

Description

Approved proposal

  • Don't require curl at install time;
  • Keep existing non-curl path;
  • Add non-parallel MultiHttpClient fallback,
  • Provide warning about missing curl on Special:Version. Declined per T184652.

Original proposal

Right now we have some features that require curl (it's probably the #2 issue third parties have with installing VisualEditor, for instance, after mis-matched versions; it's also needed for Math and several other extensions).

Historically, we bent over backwards to provide a fallback using file streams. Although a few older paths still use this, it's not practical for the newer MultiHTTPClient which is used for the VirtualRESTServiceClient, amongst other important features.

Failure to have cURL installed is a big cause of third party user confusion, and the installer already for
some time has suggested that they install it. Instead of having so confusing a situation where you don't need cURL unless you do, now we just require it outright, simplifying things a great deal.

We already require five PHP extensions – xml, ctype, json, iconv, and (as discussed in March in T129435) mbstring.

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
brion added a comment.Jun 28 2016, 8:14 PM

tl;dr jump to end paragraph

I'm curious, do any core functions of MediaWiki require external requests? I can't think of any...just a bunch of things you can turn on (InstantCommons, caching, object stores, extensions).

Offhand, I think that InstantCommons would be the primary user-visible core feature on small installations that requires an HTTP client and does not currently require curl.

It may be that extension features are even more popular and I've no idea what they need; since Wikimedia Foundation doesn't make any attempt to survey feature usage of MediaWiki installs, we're kinda shooting in the dark here. :)

Perhaps the middle ground here? Don't require it, but strongly suggest it in the installer with the caveat that XYZ features just don't work unless you do? That would allow us to remove the fallbacks (security & maintenance burdens --) and just improve the failure scenarios when you try to turn on unsupported features.

For InstantCommons my understanding is that it presently works without curl, unless we actively choose to either remove the non-curl support in MWHttpRequest's PhpHttpRequest subclass or replace ForeignAPIRepo's use of MWHttpRequest with MultiHttpClient without adding a non-curl replacement.

On two fronts:

  1. As I understand we've already done the work to get PhpHttpRequest to initialize openssl with the right certs, so the costs to making sure InstantCommons works with our HTTPS-only world whether using curl or not have already been spent. But there are potential future work needs in terms of security, correctness, and expansion to support HTTP/2 etc, which we could neatly sidestep by deferring to libcurl's conveniently existing, well-supported implementation.
  1. ForeignAPIRepo could benefit from MultiHttpClient for some specific cases like fetching thumbnail URLs (where the web APIs don't allow for requesting multiple files at different sizes), but otherwise mostly will get parallelism from asking for information about multiple files in a single request (already provided for on the repo end, needs more work on the parser end).

So there's no strong need to make Instant Commons depend on curl for functionality; switching it to require curl would be our choice based on our own schedule for maintaining or removing the PhpHttpRequest class.

I hesitate to say "deprecate" because it's easy to confuse usages between "warning people that an interface is going to be removed in the near future so they should not use it" and "recommending that another, better, interface be used instead, but saying nothing specific about whether this one will continue to exist". Further, in this case there's no *interface* to deprecate, we're talking about removing an invisible backend, which actually changes the internal API contract of MWHttpRequest from 'always available' to 'might be unavailable'.

So, we could kill the non-curl backend, change MWHttpRequest to explicitly say when it's unavailable, and devise both install/upgrade-time and runtime warnings that point to a documentation page that's clear enough to explain to a harried, part-time sysadmin whose inherited a wiki probably set up by a former co-worker or teammate or forum denizen how to either fix or work around the error.

I'm not against this, as long as thought through and well documented. :)

demon added a comment.Jun 28 2016, 8:32 PM

Calling out this one point in particular so it doesn't get lost:

It may be that extension features are even more popular and I've no idea what they need; since Wikimedia Foundation doesn't make any attempt to survey feature usage of MediaWiki installs, we're kinda shooting in the dark here. :)

See also: T56425: Provide an opt-in ability to register the user's MediaWiki installation. We've wanted to this for some time, nobody's had the time to implement it though. There's lots of useful stats that we could gather about feature usage and support. That could help guide these sorts of decisions with real data.

bd808 added a subscriber: bd808.Jun 28 2016, 8:40 PM

Interesting data and analysis at http://mtdowling.com/blog/2013/05/02/requiring-curl-in-your-php-library/. Includes some analysis of presence of php-curl bindings by default at a number of shared hosting providers.

brion added a comment.Jun 28 2016, 8:49 PM

Yeah, be nice to have firmer data, and I think we should def do another push on T56425. Just knowing curl is available at various big hosts is a help for now though!

brion added a comment.Jun 28 2016, 9:10 PM

Ok, so updated outcome choices replacing the hard dependency with a soft dependency:

  1. no change (advocated most recently by @Florian in T137926#2384514)
  2. drop non-curl path for MWHttpRequest, add clear error messaging & install-time doc for features that use MWHttpRequest or MultiHttpClient on non-curl hosts (advocated most recently by @demon in T137926#2412673)
  3. keep non-curl path in MWHttpRequest, create alternate non-curl implementation of MultiHttpClient (advocated most recently by @Krinkle in T137926#2412298)

Any objection to going into tomorrow's discussion with those choices ready on the table?

Tgr added a comment.Jun 28 2016, 10:48 PM

Offhand, I think that InstantCommons would be the primary user-visible core feature on small installations that requires an HTTP client and does not currently require curl.

Also image sharing between wikis of the same farm, and SpamBlacklist (bundled with the tarball) fetching the blacklist from meta.

Ok, so updated outcome choices replacing the hard dependency with a soft dependency:

  1. no change (advocated most recently by @Florian in T137926#2384514)

I think a soft dependency would be a good compromise, too, as long as the standard functions of MediaWiki are still working (ok, now we would need to define "Standard functions") :)

@Jdforrester-WMF - would you be available to discuss this RFC this coming Wednesday (at E224)? If so, excellent, this may be our choice for that time slot!

Sure.

Also image sharing between wikis of the same farm, and SpamBlacklist (bundled with the tarball) fetching the blacklist from meta.

Its technically optional for image sharing on same farm (If you disable sharing image descriptions). SpamBlacklist is another area that I think would be highly like to affect unsophisticated users, who would be most troubled by dropping curl support.

@Jdforrester-WMF - would you be available to discuss this RFC this coming Wednesday (at E224)? If so, excellent, this may be our choice for that time slot!

Sure.

Excellent! Let's use this time to see if we can figure out a good answer to Brion's multiple choice question posed at T137926#2412996

Change 294259 abandoned by Jforrester:
[DO NOT MERGE] PHPVersionCheck: Require curl to be installed

Reason:
Per RfC discussion, we're not going to require this yet, just shout from the rooftops that you really /really/ should install it.

https://gerrit.wikimedia.org/r/294259

Last-call proposal:

  • Not require curl;
  • Keep existing non-curl path;
  • Add non-parallel MultiHttpClient fallback,
  • Provide environmental warnings about missing curl (and other things?) on Special:Version.
RobLa-WMF triaged this task as High priority.Jun 30 2016, 10:25 PM
Tgr added a comment.Jul 1 2016, 1:31 PM
  • Provide environmental warnings about missing curl (and other things?) on Special:Version.

That page already suffers from information overload. Maybe a good time to start a new special page which could eventually become some sort of site admin status panel? Missing packages, disabled optimizations, uninstalled security upgrades etc., like Drupal's status report panel.

  • Provide environmental warnings about missing curl (and other things?) on Special:Version.

That page already suffers from information overload. Maybe a good time to start a new special page which could eventually become some sort of site admin status panel? Missing packages, disabled optimizations, uninstalled security upgrades etc., like Drupal's status report panel.

Possibly. For this to serve the intended purpose, it would need to be viewable and easily findable by:

  1. Non-admin users of the site, so that they can pester the site admins in charge of the setup
  2. By people providing MediaWiki support (e.g. people on Freenode MediaWiki-General) who are trying to help a site admin figure out what is wrong with the site

Special:Version is traditionally where this type of information has gone, and it seems like it should at least be an easily findable hyperlink away from Special:Version if it's not included inline.

Bawolff added a comment.EditedJul 3 2016, 4:08 PM

I think its reasonable to put this in the Installed Software section. We already include the version of libicu the intl extension is compiled against, so we could just add curl + version of curl to that list.

Krinkle renamed this task from Require 'curl' PHP extension for MediaWiki to Support running MediaWiki without 'curl' PHP extension.Jul 6 2016, 8:15 PM
Krinkle raised the priority of this task from High to Needs Triage.
Krinkle triaged this task as High priority.
Krinkle moved this task from P5: Last Call to TechCom-RFC-Closed on the TechCom-RFC board.
Krinkle edited projects, added TechCom-RFC (TechCom-RFC-Closed); removed TechCom-RFC.
Krinkle updated the task description. (Show Details)
Jdforrester-WMF lowered the priority of this task from High to Low.Apr 19 2017, 6:59 PM
Krinkle updated the task description. (Show Details)Aug 1 2018, 12:55 AM
Krinkle moved this task from Approved to In progress on the TechCom-RFC (TechCom-RFC-Closed) board.
Krinkle updated the task description. (Show Details)
Krinkle removed a project: Proposal.

Change 494630 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] Remove various references to cURL in code comments

https://gerrit.wikimedia.org/r/494630

Change 494630 merged by jenkins-bot:
[mediawiki/core@master] Remove various references to cURL in code comments

https://gerrit.wikimedia.org/r/494630

Krinkle updated the task description. (Show Details)Mar 9 2019, 9:59 PM

Now that things work without cURL, including for the MultiHttpClient. Does it still make sense to output a warning? There is no degraded feature set.

There is certainly potential in terms of performance, but we don't output warnings for other such potentials either.

That's traditionally been exposed at install/upgrade time instead. We currently does this for diff3, apcu, and intl, for example.

CCicalese_WMF added a subscriber: CCicalese_WMF.

Low priority, not ready to be worked at this point

not ready to be worked at this point

@CCicalese_WMF: Does that mean this task is technically blocked? (On the subtask here?) If yes, its status should probably be stalled?

That should probably have stated, "we (Core Platform Team) don't have the resources to work it at this point, but will get to it soon". It is no longer blocked, so we put it in our Next queue.

@CCicalese_WMF: Thanks for clarifying! That means that a potential contributor could work on this, I guess. (That was the underlying question I had in mind.)

We (Core Platform Team) are reworking our workboards. This task is still tracked by the Core Platform Team, but we are no longer using the Next tag. Removing the tag does not imply any change to the status of this task.

This is blocked by T202352

Change 533983 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/core@master] [WIP] Remove MediaWikiServices dependency from MultiHttpClient

https://gerrit.wikimedia.org/r/533983

Wow. Amazing work. I am surprised that it's worth the effort to remove just one dependency. I am supporting this as suggested in T232948#5499371.

Pchelolo added a subscriber: Pchelolo.

The only patch linked here needs a manual rebase (cc @BPirkle), so I'm gonna move this out of code review needed column back into backlog.

Krinkle closed this task as Resolved.Jul 30 2020, 9:51 PM

Now that things work without cURL, including for the MultiHttpClient. Does it still make sense to output a warning? There is no degraded feature set.

There is certainly potential in terms of performance, but we don't output warnings for other such potentials either.

That's traditionally been exposed at install/upgrade time instead. We currently does this for diff3, apcu, and intl, for example.

Closing as such.