Change 482901 had a related patch set uploaded (by Dbarratt; owner: Dbarratt):
[mediawiki/core@master] Title::checkUserBlock() should ensure the retrieved action matches the passed in restriction.

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

Can you describe what the effect of this would be (before and after your change) if the actions didn't match?

In T213220#4932221, @aezell wrote:

Can you describe what the effect of this would be (before and after your change) if the actions didn't match?

Action 1

• Name: test
• Restriction: tester
• Requires Unblock? true

Action 2

• Name: tester
• Restriction: test
• Requires Unblock? false

Action 3

• Name: sample
• Restriction: sampler
• Requires Unblock? false

Result (input: test)

• Before change: Blocked
• After change: Blocked

Result (input: tester)

• Before change: Not Blocked
• After change: Blocked

Result (input: sampler) (See T213738)

• Before change: Blocked
• After change: Blocked

Excellent. Thanks.

This shows me what I couldn't quite work out from the code. In some instances, the name collision would cause a reversal of the effective block?

I think that makes this more important to fix. Initially, it read like something more along the lines of cleaning up code smell but it appears now as a decent vector for bugs.

In T213220#4934444, @aezell wrote:

In some instances, the name collision would cause a reversal of the effective block?

Potentially, yes. The example I gave is an extreme example. No such mis-match exists (that I'm aware of), but it is certainly possible. Also, and somewhat unrelated, an Action is not required to have a restriction at all. For instance the purge restriction matches the purge action. Current this results in the user not being blocked from purge (which is correct), after this change, the user is blocked from purge (since the action has no restriction to match it against). To resolve this conflict (before T213738 is resolved) I've added purge to the whitelist of actions that are not prevented by a block (like read). If there are other actions like this, the worse case scenario is that they are blocked where they previously where not (on partial blocks).

Change 482901 merged by jenkins-bot:
[mediawiki/core@master] Ensure action object matches passed in right in Title::checkUserBlock()

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

Hmm... I actually don't think there is a way to test this since the problem is theoretical.

Could you possibly fudge some fixture data that causes the problem?

Thanks for the explanation by the way. It's helped me understand the issue.

In T213220#4936526, @aezell wrote:

Could you possibly fudge some fixture data that causes the problem?

Oooo..... maybe we are talking about two different things. I was thinking testing as in "QA testing", but I suppose I could write a test with mock actions (we already have those). Should I do that?

We were talking about two different things. In fact, in this case, a unit test is likely the better of the two options because I agree that QA on this with real data is probably impossible (or too much work to be reasonable).

Change 491675 had a related patch set uploaded (by Dbarratt; owner: Dbarratt):
[mediawiki/core@master] Add tests to ensure that retrieved actions match passed in restrictions.

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

Change 491675 merged by jenkins-bot:
[mediawiki/core@master] Add tests to ensure that retrieved actions match passed in restrictions

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

In T213220#4936516, @dbarratt wrote:

Hmm... I actually don't think there is a way to test this since the problem is theoretical.

If there isn't a way to test this, I'm going to put this in the Done column.