Change 332737 had a related patch set uploaded (by Samwilson):
[WIP] Add an expiry to the localStorage replication of the block cookie

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

localStorage expiry is a problem that would be worth solving more generally - see T121646: Document usage policy for LocalStorage in MediaWiki code and T110353#2948511.

@Tgr: you're right, but for the time being my patch for this just addresses the blockcookie usage of localstorage. When a more general feature is developed, it won't be too hard to migrate this.

When I tested cookie blocks on testwiki I see the testwikiBlockID cookie. However when I checkout out this branch on my local devwiki, I don't get such a cookie after blocking a user. Any idea what I'm doing wrong? Maybe I'm missing some environmental dependency?

Unrelated, I did discover localStorage["MediaWikiModuleStore:wiki"] which is HUGE. On enwiki it was nearly 4 megabytes!!! It looks like mostly code to me, as opposed to actual data. I wonder what the heck that's for... but I digress.

Unrelated, I did discover localStorage[''MediaWikiModuleStore:wiki"] which is HUGE.

It's client-side caching for Javascript modules. See T66721.

It's the Javascript module cache for ResourceLoader. Since all JS modules are loaded concatenated in a single request, localStorage caching helps to avoid redownloading everything when a single module changes.

In T152952#3051142, @MusikAnimal wrote:

When I tested cookie blocks on testwiki I see the testwikiBlockID cookie. However when I checkout out this branch on my local devwiki, I don't get such a cookie after blocking a user. Any idea what I'm doing wrong? Maybe I'm missing some environmental dependency?

Did you set $wgCookieSetOnAutoblock = true;?

So I've been testing cookie blocks on CommTechWiki, and here are my findings:

First I tested deleting cookies/localStorage. When the cookie is deleted but still in localStorage, the cookie is restored. Same if I delete localStorage but keep the cookie – the localStorage key/value is restored. This worked when I changed IPs, too. The only thing that's weird about changing IPs is the message says "You've been autoblocked because your IP address has been recently used by..." when in fact it was not used by the blocked account. I don't if this is a big deal, but I thought I'd mention it.

Next, I noticed that when you unblock someone or the block expires, the cookie and the localStorage key/value aren't removed. Is that intentional? When I restored the block, it restored the autoblock on the second account that I was still logged into, presumably because I still had the cookie. However I would think there'd be a new block ID, so I shouldn't be autoblocked again based on the cookie alone.

For the localStorage key, should we use something more MediaWiki-specific than blockID? Maybe MediaWikiBlockID? What happens if a user is blocked on multiple MediaWiki wikis? Would they share the same key in localStorage? Is it possible we could inadvertently cause an autoblock on a different, non-WMF wiki? Cookies by contrast have a domain associated with them.

As for this ticket itself, I wasn't able to test it. I guess I'd have to change the server time?

Thanks for testing!

The only thing that's weird about changing IPs is the message says "You've been autoblocked because your IP address has been recently used by..." when in fact it was not used by the blocked account. I don't if this is a big deal, but I thought I'd mention it.

Is this on the first request you make from the new IP? Because maybe what's happening is that when you first request an edit page, it's loading the block ID from the cookie and loading it, and at that point blocking your IP. Then you try to save the page, but you can't because your IP is blocked (and IP block-triggers are loaded before cookie-block trickers).

So it's not incorrect. I'm not sure what the best solution is to get around it though. Maybe it's good to confuse the blocked people? ;)

Next, I noticed that when you unblock someone or the block expires, the cookie and the localStorage key/value aren't removed. Is that intentional? When I restored the block, it restored the autoblock on the second account that I was still logged into, presumably because I still had the cookie. However I would think there'd be a new block ID, so I shouldn't be autoblocked again based on the cookie alone.

No, they should be removed. But only when the block expires or after 1 day (whichever is sooner). I'm confused though: where does the new block ID come from? You mean for your new IP?

For the localStorage key, should we use something more MediaWiki-specific than blockID? Maybe MediaWikiBlockID? What happens if a user is blocked on multiple MediaWiki wikis? Would they share the same key in localStorage? Is it possible we could inadvertently cause an autoblock on a different, non-WMF wiki? Cookies by contrast have a domain associated with them.

LocalStorage is like cookies, and limited to the same-origin. But if multiple wikis are in subdirectories on the same domain, there could be an issue. The mediawiki.cookie module does this with a prefix, but mediawiki.storage does not. I'd say that if this is considered a problem it should be fixed in that module, rather than here.

As for this ticket itself, I wasn't able to test it. I guess I'd have to change the server time?

Yeah, I've tried testing by blocking for an hour and coming back later on. It's not an easy thing to test though. I've also been manually changing the times in the expiry.

LocalStorage is like cookies, and limited to the same-origin. But if multiple wikis are in subdirectories on the same domain, there could be an issue. The mediawiki.cookie module does this with a prefix, but mediawiki.storage does not. I'd say that if this is considered a problem it should be fixed in that module, rather than here.

Actually, I was wrong. Firefox has separate LocalStorage for each subdomain. The thing that is weird about Firefox is that it shares a single 10MB storage limit across an entire domain name rather than having a separate 10MB limit for each localStorage instance (i.e. each subdomain). So this isn't an issue for cookie blocking.

Yeah, I've tried testing by blocking for an hour and coming back later on. It's not an easy thing to test though. I've also been manually changing the times in the expiry.

I tested this by just setting short blocks. Seems to work as expected.

@Samwilson: Added one suggestion in Gerrit. Otherwise, I think this should be ready to go.

So I blocked an account for 6 months, and I see the expiry in the cookie is right. However the expiry in localStorage appears to be the time the block was made. Is that intentional? If so maybe "expiry" isn't the right word to use?

Another thing... I unblocked the account, then re-blocked. The expiry on the cookie was updated as expected, but the localStorage expiry remained unchanged.

Correction, the localStorage expiry is the time of the block + 1 day, I think... I'm also not accounting for timezone change. If so it was + 1 day + 5 hours (since I'm UTC-5). This suggests maybe the localStorage expiry is stored in the user's local timezone and not UTC?

So my block was at Mon Mar 06 2017 11:31:20 GMT-0500 (EST). The localStorage expiry was set to 1488904280.963285, so new Date(1488904280.963285 * 1000) is Tue Mar 07 2017 11:31:20 GMT-0500 (EST).

The cookie expires at the same time of the block (to a max of 24 hours) but the localstorage item always has a 24 hour expiry time from the time that it's created. This is because there's no way to retrieve the cookie's expiry time from the JS module, and because it's just simpler. It doesn't matter if it ends up expiring later than the cookie or the block, because it won't have any effect if after the block's expired. (I'll document all this on mw.org.)

This suggests maybe the localStorage expiry is stored in the user's local timezone and not UTC?

No, it should be UTC, the epoch is always UTC. In your example, it's correctly 24 hours after the block.

But you're right, what if the block is changed? So if someone has the localstorage expiry set to 1pm tomorrow, then before then gets re-blocked for a shorter time (say 1 day down to 1 hour): the cookie would expire at say 10pm tonight, but they'd be left with a localstorage value that, even after 10pm would try to recreate the cookie. Then, the cookie would lead to the block being loaded... except it wouldn't because at that point the block's expired. So that's okay.

In the other direction: someone gets blocked for an hour, then it's extended to a week: the localstorage expiry is 24 hours, then in a two days' time when they try to edit a page the localstorage will be removed as expired, but the cookie will still be there so they'll still be blocked. However it'd take a 2nd request to re-set the localstorage value, because it's currently only removing *or* adding it. I think it needs an extra check for this, so it can remove an expired localstorage item and re-add it if it's still valid.

In the other direction: someone gets blocked for an hour, then it's extended to a week: the localstorage expiry is 24 hours, then in a two days' time when they try to edit a page the localstorage will be removed as expired, but the cookie will still be there so they'll still be blocked. However it'd take a 2nd request to re-set the localstorage value, because it's currently only removing *or* adding it. I think it needs an extra check for this, so it can remove an expired localstorage item and re-add it if it's still valid.

I was overlooking the fact that the cookie also never lives beyond 24 hours, and has to be recreated by another request made in this time. I've simplified that bit of code (thanks @Krinkle).

Sorry, let me make sure I've got this right: The cookie expires after 30 days, and localStorage after 1 day, correct? I tested this on CommTechWiki and it seems that the cookie is now being set to expire after 24 hours.

1. Blocked User:MusikPuppet for 1 week
2. MusikPuppet has a cookie set for +24 hours from the time of the block, same for localStorage (the latter being correct)
3. Re-blocked MusikPuppet for 1 month
4. Cookies and localStorage again set to +24 hours from the time of the new block
5. MusikPuppet deletes their cookies
6. Now the cookie expiry is correctly at +1 month

I wonder if we could add browser tests for this somehow (i dont know much about browser tests).the behaviour seems complex enough that testing would be good.

In order to simplify, we've decided to go the other direction for the time being: remove the localstorage replication entirely. The above patch is updated.

I think simplifying it is a good idea for now. The whole thing had become so complicated it was virtually impossible to test all the edge cases. Let's get the basic idea working reliably and then it can be improved from there.

Change 332737 merged by jenkins-bot:
[mediawiki/core] Remove the localStorage replication of the block cookie

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