Page MenuHomePhabricator

blocking for 100000 years doesn't create a block but fills block log
Closed, ResolvedPublic

Description

When blocking for 100000 years, block log registers a block for "100 millennia", but no block is actually made.


Version: 1.22.0
Severity: minor

Details

Reference
bz49580

Event Timeline

bzimport raised the priority of this task from to Lowest.
bzimport set Reference to bz49580.
bzimport added a subscriber: Unknown Object (MLST).
AzaToth created this task.Jun 14 2013, 2:39 PM

31 millennia, 6 centuries, 8 decades, 8 years, 269 days, 17 hours, 37 minutes and 3 seconds (999999999999 seconds) also fails to actually block the user but will create a log entry.

Looking at the debug toolbar, a block is created, but doesn't even last to the end of the success page load - It's inserted with ipb_expiry = '', but then Block::purgeExpired kills all such blocks ("DELETE FROM ipblocks WHERE (ipb_expiry < '20130614203608')", and a blank string is always smaller than another string).

I have tested this on a local instance and I get the same results with much lower block values, here is what I have found.

In the DB a timestamp of 20380119021407 and 20380119021408 are displayed on Special:Blocklist as very different datetimes (even though there is only a second difference). One displays as 1970, one displays as 2038..

As far as I can tell I have traced this back to useradjust around line 1881 in languages/language.php where $date returns incorrectly.
And again as far as I can tell this is due to mktime in this function (toward the bottom) occasionally returning a null value therefor returning the block expiry as the Unix epoch which has passed which Block::purgeExpired then removes from the db as an expired block.

Isarra added a comment.Jul 1 2013, 6:52 PM

There seem to be two major issues here: that these blocks of unusual times are failing, and also that the confirmation message and logs are still showing them as successful even when they do fail.

While perhaps this should be a separate bug, I would argue that latter issue is much more so because it is effectively covering up the former for the user.
Yes, things fail sometimes - that is normal enough and not unexpected in MediaWiki. But the user should know if it failed, whereas this is instead telling them the opposite, that it didn't fail even when it did. The expectation is that they would have immediate feedback telling them that the block didn't work, so then they can do something about it then and there (in this case for instance by trying again using one of the dropdown times). Without that feedback, and especially with misleading feedback from the software reporting that the block did work, not only will they not know they need to do anything about it in these cases, but it also presents potentially much more serious problems should it cover up any other bugs that may arise at similar points within the blocking process.

For example, while not serious, similar misleading feedback seems to occur with blocks with bogus times like 'a potato'. While that sort of input at least is unlikely to ever be expected to succeed as a serious block, there may be others.
Has anyone run into similar, with other blocks failing but also reporting as successful and adding to the block log? Or is the issue specifically related to where it is failing in both cases?

Well it turns out that I can not really test this / fix it on my system as my system currently uses 32 bit signed int which limits my timestamp to 2038-01-19 03:14:07 which I imagine is why my tests show the blocks stopping at a lower value.
I would say a similar thing is causing the reported issue but the limit for 64bit signed ints / timestamps is 293 billion years ish

(In reply to comment #5)

Well it turns out that I can not really test this / fix it on my system as my
system currently uses 32 bit signed int which limits my timestamp to
2038-01-19
03:14:07 which I imagine is why my tests show the blocks stopping at a lower
value.
I would say a similar thing is causing the reported issue but the limit for
64bit signed ints / timestamps is 293 billion years ish

I think that's https://en.wikipedia.org/wiki/Year_2038_problem :)

(In reply to comment #4)

There seem to be two major issues here: that these blocks of unusual times
are failing, and also that the confirmation message and logs are still showing
them as successful even when they do fail.

There's a third major issue, I think: why doesn't MediaWiki simply reject these plainly retarded expiry inputs? The correct answer to trying to block a user for a duration of 100,000 years or a potato or whatever is to simply reject the request. Is there an open bug about this? If not, can you open one, please?

(In reply to comment #7)

(In reply to comment #4)
> There seem to be two major issues here: that these blocks of unusual times
> are failing, and also that the confirmation message and logs are still showing
> them as successful even when they do fail.

There's a third major issue, I think: why doesn't MediaWiki simply reject
these plainly retarded expiry inputs?

The issue is that it does. It uses strtotime() to parse them, and it returns a non-error value for both "100000 years" (on 64-bit systems, apparently) and "a potato" (don't ask me why).

It should probably check for values over the database 'int' type capacity and earlier than today. But this bug covers that.

(In reply to comment #8)

It should probably check for values over the database 'int' type capacity and
earlier than today. But this bug covers that.

Not just 'int' capacity: if you attempt to block someone for longer than, say, 200 years, it should either convert to indefinite to be flatly rejected. The software can be a bit smarter here.

s/to be/or be/

I'm still waking up.

Isarra added a comment.Jul 8 2013, 6:45 PM

Aye, treating longer ones as indefinite would probably be sensible in general, but it wouldn't necessarily resolve the other problems.

Anomie added a subscriber: Anomie.Dec 19 2014, 10:04 PM

and "a potato" (don't ask me why).

"a" is an alias for UTC+1, from nautical time apparently. PHP's strtotime is extremely forgiving; since it saw a timezone identifier it recognized, it happily ignores the invalid rest of the string.

Change 181175 had a related patch set uploaded (by Anomie):
Reject out-of-range output when converting to TS_MW

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

Patch-For-Review

Change 181175 merged by jenkins-bot:
Reject out-of-range output when converting to TS_MW

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

Anomie set Security to None.
Anomie closed this task as Resolved.Dec 22 2014, 8:40 PM
Anomie claimed this task.

The bug as filed here is fixed, and should be deployed to WMF wikis with 1.25wmf14 (see https://www.mediawiki.org/wiki/MediaWiki_1.25/Roadmap for the schedule).

A new bug should be filed if applying blocks with a negative relative expiry or already-past absolute expiry needs to be tracked, or if an expiry of "a potato" is worth worrying about.