-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
Hi,
I'm proposing a breaking change to MediaWiki\Shell\Command::restrict(). I made a mistake in my original implementation of this function that I believe we're better off fixing. For background on what shell restrictions are, see https://www.mediawiki.org/wiki/Manual:Shell_framework#Restrictions.
To summarize the ticket I filed https://phabricator.wikimedia.org/T257278:
Command manages $restrictions as bitfields and by default sets it to Shell::RESTRICT_DEFAULT (39). To disable restrictions, the idea was to call ->restrict( Shell::RESTRICT_NONE );. However, ->restrict() adds to $restrictions (using |=) instead of overwriting...so it's impossible to disable a default restriction.
My proposal is to have ->restrict() overwrite the current $restrictions instead of adding to it. Based on codesearch, every non-test caller is already set up to work this way, so in practice, it shouldn't be a breaking change.
The Gerrit patch for this is https://gerrit.wikimedia.org/r/c/mediawiki/core/+/609870/ and contains multiple examples of the correct way to call the fixed function .
Comments, alternative suggestions welcome. I would like to get this in for 1.35 if possible.
Thanks, - -- Legoktm
Maybe introduce a function like removeRestrictions()?
On Wed, Jul 8, 2020 at 2:53 PM Kunal Mehta legoktm@member.fsf.org wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
Hi,
I'm proposing a breaking change to MediaWiki\Shell\Command::restrict(). I made a mistake in my original implementation of this function that I believe we're better off fixing. For background on what shell restrictions are, see https://www.mediawiki.org/wiki/Manual:Shell_framework#Restrictions.
To summarize the ticket I filed https://phabricator.wikimedia.org/T257278:
Command manages $restrictions as bitfields and by default sets it to Shell::RESTRICT_DEFAULT (39). To disable restrictions, the idea was to call ->restrict( Shell::RESTRICT_NONE );. However, ->restrict() adds to $restrictions (using |=) instead of overwriting...so it's impossible to disable a default restriction.
My proposal is to have ->restrict() overwrite the current $restrictions instead of adding to it. Based on codesearch, every non-test caller is already set up to work this way, so in practice, it shouldn't be a breaking change.
The Gerrit patch for this is https://gerrit.wikimedia.org/r/c/mediawiki/core/+/609870/ and contains multiple examples of the correct way to call the fixed function .
Comments, alternative suggestions welcome. I would like to get this in for 1.35 if possible.
Thanks,
- -- Legoktm
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE2MtZ8F27ngU4xIGd8QX4EBsFJpsFAl8Fs30ACgkQ8QX4EBsF JptHyA//Uo5h3l4EUTJlBNIoOR1jKlN7onKfzHZrxDPKvLDlBCrAYB9YLkxZ6Pfb /SP8c0mUH5UvQBH+SL/qZPmHTfwbnB1auOuyzu+XedFZxET26kh29glWSwbxf9KL i+7dAuO8gFM/q7gIuYMCc+TmZu+SQEOgr4Ek+iD/fIcQondpxmzaAdTUJ8UPn9a0 OLONdqo7elFM8VRID0YlIzO4DCKymmpW/aEiJChOTxy11IUr4ZQ8n1r8aY66LZ2y qlsm00hofPkVH19Pjqj5T1E80etUcMwh6uVdpctgJBNFXNYIuaAG6sHatbZXH3WJ /zIfzhNBXB19233fQ3Cn/f+fLYXQPGxw5Z/ATEtlHSAFAeWwXtkpvOED0fAO0X8r 0BbAdHkWYPkbGTPimrf5R1dwvmnw09p4qGUMGGb9Nw6KwsJpOMoZL8oBMa120ktI FzJ6EPB3iv1XbsY1IOaekmdThISV/V1pB4PpteqpK44li+1kPCg4sJIqyaFMG86p ejgzlXMiLw8g9skezFGsZJeaUbGwu+Fu0GuAQ/4+py50jsmAEsO49u6md7TtzhUu Loa39JpisHk6Kk5otfL+f1b8MWNBBVuqixQTCL/T78G3jfotG9swjuqqKq8MNY6U qgx2lL8LEEaUik7MwhvxJ00A6p89HV9KTezTdwmlwWwmKA83ec0= =HouQ -----END PGP SIGNATURE-----
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
wikitech-l@lists.wikimedia.org