Dealing with composer dependencies in early MediaWiki initialization

List overview All Threads
Download

newer

older

Difference between #goal and #epic

Train

Kunal Mehta

26 Jun 2019 26 Jun '19

2:21 a.m.

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 Hi, I (with Reedy's help) recently started work on librarizing MediaWiki's IP class into a separate composer package (wikimedia/ip-utils[1]). The main motivation was so that the Parsoid PHP port could use it[2]. However, I ran into an unexpected hitch[3], as it seems we're using the IP class before the composer autoloader is even intialized. Here's the basic initialization in Setup.php: - - AutoLoader.php (MediaWiki's) - - Defines.php - - DefaultSettings.php - $wgServer = WebRequest::detectServer() - Calls IP::splitHostAndPort() - - GlobalFunctions.php - - vendor/autoload.php (composer's) My understanding is that composer's autoloader runs late so extensions registering themselves using it can add their stuff to the necessary globals. And we call WebRequest::detectServer() in DefaultSettings.php so that in LocalSettings.php people can use the value of $wgServer for other stuff. I see 3 main ways to move forward: 1. Move vendor/autoload.php earlier in Setup.php, potentially breaking extensions that still rely on composer autoloading for initialization. 2. Set $wgServer = false or something in DefaultSettings.php, and then fill it in later in Setup.php *after* the composer autoloader has been loaded, potentially breaking anyone relying on the value of $wgServer in LocalSettings.php. 3. (status quo) not librarize code that runs before composer autoloader initialization. :( Advice/input welcome. [1] https://packagist.org/packages/wikimedia/ip-utils [2] https://gerrit.wikimedia.org/g/mediawiki/services/parsoid/+/77064cfff717 6493a2828bb4f95f397dfce7d659/src/Utils/Title.php#46 [3] https://gerrit.wikimedia.org/r/c/mediawiki/core/+/519089/ - -- Legoktm

...PGP SIGNATURE...

Show replies by thread

Bryan Davis

26 Jun 26 Jun

3:26 a.m.

New subject: Dealing with composer dependencies in early MediaWiki initialization

On Tue, Jun 25, 2019 at 8:21 PM Kunal Mehta <legoktm(a)member.fsf.org> wrote:

...

I see 3 main ways to move forward: 1. Move vendor/autoload.php earlier in Setup.php, potentially breaking extensions that still rely on composer autoloading for initialization. 2. Set $wgServer = false or something in DefaultSettings.php, and then fill it in later in Setup.php *after* the composer autoloader has been loaded, potentially breaking anyone relying on the value of $wgServer in LocalSettings.php. 3. (status quo) not librarize code that runs before composer autoloader initialization. :(

There may be more entanglements here than I'm seeing, but I think that there may be an option 4: add code in WebRequest to replace the use of IP::splitHostAndPort() and IP::combineHostAndPort(). IP::combineHostAndPort() is trivial, and I think that splitHostAndPort() could be replaced with a semi-clever call to parse_url() that looked something like: $parts = parse_url( 'fake://' . $_SERVER[$varName] ); Bryan -- Bryan Davis Wikimedia Foundation <bd808(a)wikimedia.org> [[m:User:BDavis_(WMF)]] Manager, Technical Engagement Boise, ID USA irc: bd808 v:415.839.6885 x6855

Máté Szabó

9:16 a.m.

New subject: Dealing with composer dependencies in early MediaWiki initialization

Hey, Looking at Setup.php, it seems to include the relevant items in the following order: - DefaultSettings.php - Composer autoloader - LocalSettings.php or config callback Could this allow us to initialise $wgServer in Setup.php, right after the Composer autoloader is included? It seems to me this would not break custom LocalSettings files that expect it to be set, as LocalSettings would not yet be included at that point. What do you think? Best ---- Máté Szabó SOFTWARE ENGINEER +36 30 947 5903 WIKIA sp. z o.o. z siedzibą w Poznaniu, ul. Abp. A. Baraniaka 6 Sąd Rejonowy Poznań – Nowe Miasto i Wilda w Poznaniu, VIII Wydział Gospodarczy Krajowego Rejestru Sądowego, KRS 0000254365 NIP: 5252358778 Kapitał zakładowy: 50.000,00 złotych

...

On 26 Jun 2019, at 04:21, Kunal Mehta <legoktm(a)member.fsf.org> wrote: -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 Hi, I (with Reedy's help) recently started work on librarizing MediaWiki's IP class into a separate composer package (wikimedia/ip-utils[1]). The main motivation was so that the Parsoid PHP port could use it[2]. However, I ran into an unexpected hitch[3], as it seems we're using the IP class before the composer autoloader is even intialized. Here's the basic initialization in Setup.php: - - AutoLoader.php (MediaWiki's) - - Defines.php - - DefaultSettings.php - $wgServer = WebRequest::detectServer() - Calls IP::splitHostAndPort() - - GlobalFunctions.php - - vendor/autoload.php (composer's) My understanding is that composer's autoloader runs late so extensions registering themselves using it can add their stuff to the necessary globals. And we call WebRequest::detectServer() in DefaultSettings.php so that in LocalSettings.php people can use the value of $wgServer for other stuff. I see 3 main ways to move forward: 1. Move vendor/autoload.php earlier in Setup.php, potentially breaking extensions that still rely on composer autoloading for initialization. 2. Set $wgServer = false or something in DefaultSettings.php, and then fill it in later in Setup.php *after* the composer autoloader has been loaded, potentially breaking anyone relying on the value of $wgServer in LocalSettings.php. 3. (status quo) not librarize code that runs before composer autoloader initialization. :( Advice/input welcome. [1] https://packagist.org/packages/wikimedia/ip-utils [2] https://gerrit.wikimedia.org/g/mediawiki/services/parsoid/+/77064cfff717 6493a2828bb4f95f397dfce7d659/src/Utils/Title.php#46 [3] https://gerrit.wikimedia.org/r/c/mediawiki/core/+/519089/ - -- Legoktm

...PGP SIGNATURE...

_______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l

David Barratt

3:42 p.m.

New subject: Dealing with composer dependencies in early MediaWiki initialization

...

Move vendor/autoload.php earlier in Setup.php

I would do that. In an ideal world, Composer's autoloader would be called first, and MediaWiki's autoloader would be registered through Composer. :)

...

potentially breaking extensions that still rely on composer autoloading

for initialization. That is wrong anyways. That would prevent a MediaWiki user from disabling an extension without removing the code via the Composer CLI. Breaking something we want to discourage, I think, is a good thing. David Barratt (he/him) Software Engineer, Anti-Harassment Tools Wikimedia Foundation On Wed, Jun 26, 2019 at 5:17 AM Máté Szabó <mszabo(a)wikia-inc.com> wrote:

...

Hey, Looking at Setup.php, it seems to include the relevant items in the

following order:

...

- DefaultSettings.php - Composer autoloader - LocalSettings.php or config callback Could this allow us to initialise $wgServer in Setup.php, right after the

Composer autoloader is included? It seems to me this would not break custom LocalSettings files that expect it to be set, as LocalSettings would not yet be included at that point. What do you think?

...

Best ---- Máté Szabó SOFTWARE ENGINEER +36 30 947 5903 WIKIA sp. z o.o. z siedzibą w Poznaniu, ul. Abp. A. Baraniaka 6 Sąd Rejonowy Poznań – Nowe Miasto i Wilda w Poznaniu, VIII Wydział

Gospodarczy Krajowego Rejestru Sądowego, KRS 0000254365

...

NIP: 5252358778 Kapitał zakładowy: 50.000,00 złotych

...PGP SIGNATURE...

_______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Brian Wolff

27 Jun 27 Jun

12:36 a.m.

New subject: Dealing with composer dependencies in early MediaWiki initialization

Another option is just removing the $wgServer back compat value. The installer will automatically set $wgServer in LocalSettings.php. The default value in DefaultSettings.php is mostly for compat with really old installs before 1.16. Allowing autodetection is a security vulnerability - albeit mostly difficult to exploit. The primary method is via cache poisioning and then either redirecting or otherwise tricking users about the fake domain. See the original ticket https://phabricator.wikimedia.org/T30798 . Another possibility is putting unsafe values in the host header to try and get an xss (followed by cache poisioning so its not just self xss). Im unsure off the top of my head what validation if any is done (im pretty sure its less strict than legal domains) so im not sure how practical that is. Anyways 1.16 was a long time ago, put my vote as we should make a breaking change and just throw an exception if wgServer is not set in LocalSettings.php -- Brian P.s. people with access to security tasks may also find the phab comment at https://phabricator.wikimedia.org/T157426#3192740 interesting where some of the implications of $wgServer were discussed (note the task was primarily about something else and is unfortunately still secret) On Tuesday, June 25, 2019, Kunal Mehta <legoktm(a)member.fsf.org> wrote:

...

...PGP SIGNATURE...

_______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Tim Starling

3:25 a.m.

New subject: Dealing with composer dependencies in early MediaWiki initialization

On 27/6/19 10:36 am, Brian Wolff wrote:

...

Interesting that I wrote there: "How about this: let's set $wgServer in the installer in 1.18, and remove $wgServer autodetection from DefaultSettings.php a bit later, say in 1.20." It was indeed 1.18, not 1.16, in which $wgServer started being set in LocalSettings.php. I added it to LocalSettingsGenerator.php here: https://www.mediawiki.org/wiki/Special:Code/MediaWiki/90105 Anyway, it's past 1.20 so I guess that would be a good thing to do. -- Tim Starling

Kunal Mehta

19 Jul 19 Jul

5:08 a.m.

New subject: Dealing with composer dependencies in early MediaWiki initialization

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 Hi, On 6/26/19 11:25 PM, Tim Starling wrote:

...

Thanks for the background Brian and Tim, and agreed, time to get rid of autodetection. I wrote <https://gerrit.wikimedia.org/r/c/mediawiki/core/+/524396>, and jenkins forced me to investigate that the CLI installer has not been setting $wgServer in LocalSettings, leaving wikis reliant upon autodetection. So in the patch I added an optional --server parameter to the CLI installer, with it defaulting to <http://localhost> if none is provided. Does that seem acceptable enough? I'm not sure what other behavior would be sensible. - -- Legoktm

...PGP SIGNATURE...

Brad Jorsch (Anomie)

2:46 p.m.

New subject: Dealing with composer dependencies in early MediaWiki initialization

On Fri, Jul 19, 2019 at 1:09 AM Kunal Mehta <legoktm(a)member.fsf.org> wrote:

...

So in the patch I added an optional --server parameter to the CLI installer, with it defaulting to <http://localhost> if none is provided. Does that seem acceptable enough? I'm not sure what other behavior would be sensible.

The other options I could think of would be to make --server a required parameter to the CLI installer, or to let the CLI installer generate a LocalSettings.php that does not result in a usable wiki (since it will give the error that $wgServer needs to be set in LocalSettings.php). -- Brad Jorsch (Anomie) Senior Software Engineer Wikimedia Foundation

Martin Urbanec

4:18 p.m.

New subject: Dealing with composer dependencies in early MediaWiki initialization

Dne pá 19. čvc 2019 15:48 uživatel Brad Jorsch (Anomie) < bjorsch(a)wikimedia.org> napsal:

...

On Fri, Jul 19, 2019 at 1:09 AM Kunal Mehta <legoktm(a)member.fsf.org> wrote:

I vote for the first option (--server required), or simply set it to localhost by default, anyone capable of using a CLI installer can change this trivially IMO.

...

-- Brad Jorsch (Anomie) Senior Software Engineer Wikimedia Foundation _______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Lucas Werkmeister

4:37 p.m.

New subject: Dealing with composer dependencies in early MediaWiki initialization

Am Fr., 19. Juli 2019 um 18:18 Uhr schrieb Martin Urbanec < martin.urbanec(a)wikimedia.cz>gt;:

...

I vote for the first option (--server required), or simply set it to localhost by default, anyone capable of using a CLI installer can change this trivially IMO.

Anyone who’s using the CLI installer *manually* can do that, sure, but how many scripts would be broken by making --server required? A lot of Travis CI scripts, at least, judging by code search… <https://codesearch.wmflabs.org/search/?q=maintenance%2Finstall\.php&files=travis> Defaulting to localhost seems sensible to me. -- Lucas Werkmeister (he/er) Full Stack Developer Wikimedia Deutschland e. V. | Tempelhofer Ufer 23-24 | 10963 Berlin Phone: +49 (0)30 219 158 26-0 https://wikimedia.de Imagine a world in which every single human being can freely share in the sum of all knowledge. Help us to achieve our vision! https://spenden.wikimedia.de Wikimedia Deutschland - Gesellschaft zur Förderung Freien Wissens e. V. Eingetragen im Vereinsregister des Amtsgerichts Berlin-Charlottenburg unter der Nummer 23855 B. Als gemeinnützig anerkannt durch das Finanzamt für Körperschaften I Berlin, Steuernummer 27/029/42207.

1743

days inactive

1766

days old

wikitech-l@lists.wikimedia.org

Manage subscription

9 comments

9 participants

tags (0)

participants (9)

Brad Jorsch (Anomie)
Brian Wolff
Bryan Davis
David Barratt
Kunal Mehta
Lucas Werkmeister
Martin Urbanec
Máté Szabó
Tim Starling