[00:02:31] legoktm: do you have a few seconds to review and merge https://gerrit.wikimedia.org/r/#/c/wikimedia/slimapp/+/476782/ ? [00:04:04] bd808: sure, but we removed Generic.PHP.DeprecatedFunctions.Deprecated from our PHPCS ruleset a few major versions ago [00:04:45] hmm... so maybe not needed with the bump in version for phpcs I self-merged? [00:04:59] let me check for that before we merge then I guess :) [00:06:20] seems a good way to test the bump :D [00:06:41] also, jenkins should be running PHP 7.3 tests now [00:08:02] yeah, I saw that happen and was kind of happy to see it :) [00:08:18] travis build is running... https://travis-ci.org/wikimedia/slimapp/builds/461548523 [00:11:04] poop. still there in the version of phpcs that I'm stuck with to keep PHP 5.5 compat. I'll change the patch to just disable that sniff :) [00:14:37] bd808: if you file a bug we can do a backport to the 19.x branch [00:15:13] legoktm: do you think its worth the effort? [00:15:38] for this its just as easy for me to disable the sniff for this project [00:15:46] easier actually :) [00:16:10] bd808: I think it might benefit other people, so we should probably do both [00:16:11] * bd808 builds php 7.1.24 to test locally [00:16:25] okey doke. I can certainly file the bug [00:28:40] bug filed and cherry-picked fix uploaded to gerrit :) [00:35:26] and now green on Travis too -- https://travis-ci.org/wikimedia/slimapp/builds/461556711 [01:11:39] bd808: thanks :) [01:12:08] that sniff was just problematic for multiple reasons [01:12:58] and I'm happy to backport things to 19.x as long as its not too intrusive (this one looked incredibly trivial) and it'll benefit people [06:40:18] <_joe_> oh wow so we're not encrypting sessions for php 7.2 users? [07:37:29] _joe_: we are, it's using openssl [08:03:45] <_joe_> legoktm: oh ok, I was a bit worried [14:21:45] _joe_, legoktm: Most session data isn't encrypted at the MediaWiki level. There are special functions that can be used to get/set encrypted data in the session. The first available option from this list is used: openssl aes-256-ctr, openssl aes-256-cbc, mcrypt rijndael-128 ctr, mcrypt rijndael-128 cbc, (if $wgSessionInsecureSecrets is set, which is not the default) no encryption. If none are available, it throws an exception. In all cases (even [14:21:45] "no encryption") it then attaches an HMAC for integrity checking. [14:23:49] MaxSem, legoktm: I don't see any code path where installing openssl would make Session::getSecret() start throwing. The only way for it to throw would be if you removed both openssl and mcrypt, leaving no usable option for encryption. [19:54:19] anomie: if you create a bunch of sessions encrypted with mcrypt and then upgrade to a version that only has openssl, you would get PHP warnings about failed deserialization because the algos are different [19:56:24] MaxSem: PHP warnings != exceptions. [19:56:47] MaxSem: More specifically, PHP warnings != *throwing* exceptions. [20:39:16] Uh, actually I was right, because openssl_decrypt() return false and our code would throw an exception [20:40:09] The error message from OpenSSL is truly epic: "error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad decrypt" [20:48:07] I see no code path in Session.php where openssl_decrypt() returning false results in an exception being thrown. The exception created there is never thrown, it's just used to easily have the logger include a backtrace. [20:48:35] Eurgh [20:48:42] * MaxSem pukes [20:49:49] OK, so mcrypt is ready to die [20:50:28] Maybe, add Sodium as another option for ppl who don't have OpenSSL [22:21:28] anomie hi, "User::newSystemUser( 'Flow talk page manager', [ 'steal' => true ] );" dosen't seem to work if the site is in actor. [22:22:23] it seems to return null [22:22:23] > var_dump(User::newSystemUser( 'Flow talk page manager', [ 'steal' => true ] )); [22:22:24] NULL