[00:01:31] It'd definitely be nice to get that extension undeployed [00:03:04] only 3 wikis using it [00:04:19] It'd be easier to manually write the SQL queries, FFS. [00:05:23] ? [00:05:42] Than painstakingly confirming the maintenance script is right. [00:05:56] Two years to move 200 DB rows. [00:06:11] lol [00:06:26] * James_F is just grumpy about how hard it is for us to actually finish things. :-) [00:06:27] I think we just run it, disable the extension, then clean up if necessary [00:06:33] WFM. [00:07:37] Though not at 1am when I need to go to bed [00:07:43] Job for later in the day post sleep [00:07:51] +1 [00:08:25] https://phabricator.wikimedia.org/T141954#4600256 [16:11:25] Krinkle: do you still think the ITEM TOO BIG is UBN? From my investigation, there seems to be nothing to see here... for pages > 1 MB, we get a cache entry > 4 MB, which seems to be hitting memcache's limit [16:42:41] anomie: did you look at the missing preview in McrUndo? [16:42:53] I could work on that on the train tomorrow, if you don't [16:46:26] DanielK_WMDE_: Well, I'd be nice if the application either was able to make it work within the limit, or to decide based on it to not do it for input larger than a certain size (consistently, based on pre-serialization, for more predictable behaviour). [16:46:36] But that's rather difficult I guess given the client doesn't know the max size [16:47:03] So I'm not sure how to avoid an error level log message here while still acknowledging the behaviour as acceptable [16:47:12] DanielK_WMDE_: Did you find it affects non-stash as wel? [16:47:16] e.g. regular parser cache? [16:49:01] Krinkle: the parser cache doesn't log if this fails. [16:49:09] I could make a patch that changes this [16:49:34] Yeah, you mentioned that yesterday, I thought maybe you determined indirectly whether it also fails to store there [16:49:41] but yeah, let's find out. [16:49:47] Krinkle: it would fail less foten, since the cached object is smaller, since it doesn't contain the PST content [16:50:05] I assume it does contain the final HTML though [16:50:16] wait, why does the stash contain both HTML and PST? [16:51:04] Krinkle: Memcached error for key "ruwiki:pcache:idhash:6440754-0!canonical" on server "/var/run/nutcracker/nutcracker.sock:0": ITEM TOO BIG [16:51:08] does happen [16:51:13] right [16:51:49] It's not UBN I suppose, so long it is based on user input being too big and it being a pre-existing issue [16:52:00] I am unable to see now why it appeared to be a new issue [16:53:09] Would be nice I guess if both were INFO level, or if we have a check based on strlen($wikitext) being > something, and then make it an error iff that is small [16:53:19] because the PHP code doesnt know the reason is "TOO BIG", it just knows it fails. [16:53:21] Which can still be bad. [16:54:14] or... well, I guess {{subst:BIG}} would fool that. prolly based on pstContent [16:54:37] we need better error handling from memc [16:54:53] unlike the sql driver, it seems to expose very little. maybe we can tune that. [16:55:11] we could also enforce a limit in php, and have the memc limit set higher than that [16:55:22] we do that for uploads, iirc [16:55:37] Yeah [16:55:52] That means we'd have to do serilalize in user land though, right? [16:55:57] to be able to check its size [16:56:25] yea. we kind of want to do that anyway, to get away from php serialize [16:56:34] Yeah [16:56:48] I just want to avoid having the client serialize a serialized blob as a "string" with serialize() [16:56:54] that would inflate it somewhat [16:57:13] hopefully it has a 'raw' mode as well, for simple strings, or some other indication [16:57:17] don't know if that can be avoided [16:57:39] for parser output & friends,it wouldn't really matter [16:57:44] it does have modes besides serialize [16:57:46] but for small payloads, it would be annoying [16:57:59] also perf-wise, we call memc a lot [18:35:58] James_F: https://gerrit.wikimedia.org/r/461702 would look to be what's missing from running the disabledaccount migration script [18:36:00] lol [18:36:38] Hauskatze: ^^ [18:36:47] Ha. [18:37:12] That will do an AND won't it? [18:37:14] * Reedy double checks [18:37:20] * Hauskatze looks [18:37:43] Yeah, do you want AND there rather than OR? [18:38:10] James_F: DissableAccount does empty password AND email IIRC [18:38:15] (Isn't blank password sufficient?) [18:38:43] dunno [18:38:48] apart from I typoed [18:39:02] Details. [18:39:32] It wouldn't surpirse me if we have people on these wikis with no email set [18:39:44] So them getting blocked, although funny, would be unintended [18:40:14] Yeah. Though they get their accounts created through CreateAccount, they might later unset their e-mail. [18:40:17] var_dump( wfGetDB( DB_REPLICA )->selectFieldValues( 'user', 'user_id', [ 'user_password' => '', 'user_email' => '', ] ) ); [18:40:21] 5 rows [18:40:22] WFM [18:41:23] Hauskatze: Happy if I run the script with that enhancement then? :P [18:41:42] Reedy: if it catches the disabled accounts fine, okay [18:41:51] Well, it gets a list of everyone in the inactive group [18:42:00] It gets a list of everyone with no password and no email [18:42:03] Combines them [18:42:08] Makes sure they're unique [18:42:10] Then does the same stuff [18:42:14] James_F: SHIP IT [18:42:32] ok, that should do it I think [18:42:43] +1 [18:43:33] one day I'll learn enough PHP and MediaWiki code to be confident enough with those patches... [18:53:01] Could somebody by chance review https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/CentralAuth/+/372803/ please? [18:53:13] it's been some months there [18:57:52] Well, that's CentralAuth for you. Deeply scary code. :-( [19:07:10] blanking the password is not really reliable btw [19:07:26] user could have a bot password or any number of other things [19:07:41] on non-WMF wikis they might have some non-password-based login method even [19:07:45] This definitely predates botpasswords etc [19:07:54] And it's cruft [19:07:57] And it's going away [19:08:06] AuthManager::revokeAccessForUser exists for that purpose [19:08:15] Certainly, if it was staying around... We should be fixing it [19:08:31] (plan is to disable it today) [19:09:02] o'bye disableaccount [19:09:15] I'm just saying, if the extension is still in use today, we might want to clean up any security holes it might have [19:09:26] It is [19:09:28] (bot passwords and owner-only OAuth consumers, basically) [19:09:32] But I'm also saying I plan to get rid of it tonight [19:09:43] long overdue [19:10:09] oh, rigt, it's going to be converted into login-disabled blocks [19:10:14] that should be safe [19:10:36] First net extension we're dropping since MwEmbedSupport in June; before that UnicodeConverter in May, and CleanChanges in February. [19:10:46] E_TOOMANYEXTENSIONS. [19:11:00] and I'm proposing to get rid of UserMerge James_F [19:11:06] +1 [19:11:14] given that we're not able to use it it makes no sense to keep it there [19:11:17] Should the extension be marked on mw.o as a security risk after it's undeployed? [19:11:19] (IMHO) [19:11:20] Oh, and before CleanChanges it was Cards in June 2017. [19:11:42] Krenair: it's going to be archived I think? [19:11:53] although maybe worth marking anyway [19:11:59] archive? whee, more work for me :P [19:12:31] * Reedy cracks the whip [19:12:33] free labour [19:12:57] No no, keep them enthralled, don't let them go! [19:13:12] I plan to archive all the OCG repos for Collection as requested, but after dinner [19:13:39] Hauskatze: Thank you, as always. [19:13:46] I should be able to do all the remaining work myself after CI stuff, etc. [19:13:55] James_F: for one thing I know how to do... :) [19:15:34] I haven't looked into the Collection/OCG stuff for a long time [19:15:40] how's that going? [19:16:26] they ain't going, not used as per mdhollowa-y :) [19:20:02] James_F: Is there a list of extensions added/removed by month? [19:20:17] Wouldbe interseting to catalog by year at least +/- [19:20:37] Krinkle: `git log wmf-config/extensions-list`. [19:21:40] (Aka "no, but effort plus raw data plus experience can make one in your head".) [19:26:49] fwiw James_F CongressLookup can be undeployed or deactivated as it was targeted for the Senate vote and that happened long ago [19:27:10] the ball is on the House roof, if not thrown-out already [19:27:18] I'm speaking about the Net Neutrality campaign [19:27:28] Yeah. Is there a Phab task? [19:27:37] I'm not sure one exists [19:28:03] maybe we can just deactivate on InitialiseSettings in the next swat window [19:28:24] although if it's not going to be used, just uninstall fully :) [19:29:02] dinner, see you [22:29:14] yay [22:29:21] travis is passing on the security patches [22:31:01] 7.2 is painful though [22:32:45] wat [22:32:45] https://travis-ci.org/wikimedia/mediawiki/builds/431252924 [22:32:49] HISTORY change broke postgres? [22:32:58] oh, stalled [22:32:59] meh [22:40:20] Reedy: btw, there's two identical retValue PHP Notice Undefined index still. [22:40:24] From other files in EducationProgram [22:40:31] I didn't realise at first it was a different file. [22:40:35] :( [22:40:37] How far are we with killing that? [22:40:38] Example? [22:40:54] > /srv/mediawiki/php-1.32.0-wmf.22/extensions/EducationProgram/includes/pagers/CoursePager.php:170 [22:40:54] Well, jynus did the sql dumps that need confirming, moving... [22:40:59] So I guess within a week or so [22:41:00] and [22:41:01] > /srv/mediawiki/php-1.32.0-wmf.22/extensions/EducationProgram/includes/pagers/OrgPager.php:148 [22:41:20] havent' looked but presumably the same construct [22:41:24] as the one we fixe [22:41:24] d [22:41:32] Anyway, ignored in my dashboard for now [22:41:40] don't waste time on it :) [22:41:51] Would need a bit more context to attempt to fix [22:41:57] a switch with no default [22:42:04] "Variable $retValue might have not been defined more... (⌘F1)" [22:42:06] PHPStorm knows what's up [22:42:23] Just disable the damn thing. [22:42:31] but yeah... Hopefully it's not gonna be around for too much longer [22:42:35] The SQL dumps are done, so... [22:42:47] We can leave it installed just in case we need to re-activate it for some reason. [22:43:25] I want to get the sql dumps somewhere public before disabling it [22:43:57] Otherwise vojtech will get stroppy [22:44:11] https://phabricator.wikimedia.org/T174802#4598130 [22:45:32] I should have a look at the dumps tomorrow [23:01:45] James_F: https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/461814/ [23:01:51] Bets that upsets someone like... The SMW folk? [23:05:15] probably upsets the spell checkers ;) [23:09:10] legoktm: Be nice. Reedy can't help being from Yorkshire where they speak funny. ;-) [23:09:21] fecking code [23:10:34] lol [23:16:51] xhprof has not been updated for PHP 7.0 [23:17:01] also the code is not all that great [23:17:15] so the first thing is to decide whether we want to keep it or write a new thing [23:17:35] TimStarling: there are two or three xhprof PHP 7 forks [23:17:54] ok, that is both helpful and sucky [23:18:13] we will take over PECL maintainership [23:18:44] https://github.com/tideways/php-xhprof-extension I think is the most maintained one [23:18:54] (MW also supports it, and it's packaged for Debian) [23:19:20] if we change PECL then the manual at php.net/xhprof should also change, it's important [23:19:30] then there's https://github.com/phacility/xhprof/tree/experimental (the branch name should inspire confidence) [23:20:36] Also the PHP version doesn't include all the HHVM functionality like https://github.com/phacility/xhprof/pull/52 [23:21:17] I have a patch of my own which is not anywhere [23:21:26] TimStarling: https://secure.phabricator.com/T9805#233567 has some notes about how xhprof should have never been allowed in PECL because of the CLA [23:21:31] https://bugs.php.net/bug.php?id=64165 [23:21:47] closed on the basis of xhprof being unmaintained [23:27:35] so he wants it to be renamed [23:29:03] seems like, but it also doesn't seem like they're accepting patches anyways so any effort to maintain it is effectively a fork [23:31:39] there's no trademark [23:52:22] but it's unclear whether it's worth wading into this for what is really a very modest extension [23:52:33] the tideways version is apparently less than 1000 lines