Fri, Jul 20
Affected as well.
I think I figured out the reason: stream_set_blocking doesn't work for pipes on windows, see doc and a linked bug report. I also printed the return value for these calls, and indeed they're all false. Apparently this won't be fixed on PHP's end since there actually isn't a good solution available on windows. I'm wondering whether there's a workaround that we can apply locally.
Wellll yes, sounds like this is due to the many bugs of streams on windows reported on the web (just search for something like "php windows proc_open" or similar stuff to read a bunch of them). Trying to read from the stream before entering the while loop results in eternal loading for pipes and pipes while pipes is readable, whatever function is used. Instead, if I change $desc to use files instead of pipes for stdout and stderr, everything works fine, with the output being sent to a file as expected. Sounds like a proper solution isn't easy to implement; maybe some workaround would be possible?
I guess this isn't easy at all. I could experimentally determine what causes the script to hang: it all happens on this line, when reading the first pipe (should be stdin). I also read on the web that stream management is quite bugged on windows from PHP's end, although it's not clear whether those bugs are actually fixed.
Thu, Jul 19
I also see that when wmfRealm is 'labs', the action 'block' is enabled but the right is not assigned. However, I actually don't know which sites are affected by this. I'll ask in the appropriate pages of the 2 wikinews above, while waiting for someone who knows what to do with labs.
Needs abusefilter-modify-restricted to be assigned. I'll send a patch in a moment.
I determined that it's actually the same problem: using concatenation makes the error disappear. However, this isn't a complete solution, as another problem pops out: the process starts running, but never stops. I guess it's the same as https://www.mediawiki.org/wiki/Topic:Ufijjebcmsvh2lij, for which I'm going to open a separate task.
Wed, Jul 18
@Huji No, I've been too optimistic while saying that I would have tried to fix both. Per my comment on that task (and the other before), it won't be easy at all.
Tue, Jul 17
Same here, a bit more info on MWwiki.
It's quite hard to help with this. First, edits from 2016 aren't in the recentchanges table anymore, so I can't perform a batch test for that period. Plus, the second link given isn't valid and refers to a not-existent change. Without more info, I probably won't be able to help much. Anyway, I think this might have been some sort of temporary problem, as we have already seen others like this.
Thanks to T193903 I could finally do the testing directly on enwiki, so I found out some abuselog entries with the described problem, for instance this one. Now, you can easily see that the added text (i.e. added_lines) almost matches a piece of stringy (it's on the second row, it takes few time to find it). I said "almost", because there's a difference, which I'll say explicitly since it's not that big deal: the character "5" from stringy is actually an "S" in the added text, and it would also be trasformed from "5" to "S" by applying normalization to stringy. Probably this is because some old version of equivset changed all S's to 5 and caused the filter to match, while now it doesn't happen anymore and there's no match.
Per Legoktm above. I also made some tests and this seems not to happen anymore.
Mon, Jul 16
Yay, thanks. I usually do, but it's quite easy to forget :-)
Right, done, thanks.
I tested it (both on my local wiki and on dewiki) and it works as expected, with the $1 being recognised both inside wikilinks and ifexists. Just a side note on the first version of the message (this one): you can't really expect that a message like that will work forever. It's pretty hacky, and when changing messages or related things in the code it's really hard to mantain compatibility with such stuff.
I'm looking at this. The main problem is that the core determines whether to add the checkboxes basing on the row log type, here. Since (of course) abusefilterprivatedetails is a stand-alone log not included in suppress log, this means that it's not straightforward to add a non-deletable log without changing the core.
I guess "Extension's default configuration provides optimal experience" is satisfied, but is there a specific requirement list for it? Also, same question for "Tested with web installer".
I'm looking at this, and also trying to overhaul blockautopromote. It makes no sense to perform a random block, so I'll just hardcode it to a fixed amount.
Per above, plus the "last hit" column was removed.
Sun, Jul 15
I guess this was fixed in rEABF942a95e80e95466b49f9ac9c14e1e1a249d5a507?
The urgent part is done.
Indeed. I can do the updates myself, but first I need to understand whether old messages are still meant to be used somewhere, or can be removed.
Very likely. Also, the commit above was merged right between the last successful build and the first faulty one.
Just noticed https://integration.wikimedia.org/ci/job/wmf-quibble-vendor-mysql-hhvm-docker/1242/console for Extension:Thanks, and the job is executed (and fails) for the main build as well. I feel this is really UBN!, at least removing the broken test per standard practice.
Same without Vagrant. I'm note sure how those new filters should work, but I guess it's just a missing i18n problem. Will probably need a patch to Thanks extension to add them.
I also guess J108 could be related?
To me this is surely high, given all these unrelated failures. Not setting to UBN! since I'm not completely sure whether this affects other repos as well.
Sat, Jul 14
I think this could be slightly different from other maintenance accounts: AF account can perform lots of action: for instance block, unblock, degroup, re-group, blockautopromote and unblockautopromote (although these last two aren't logged). This means that, on wikis where local config allows such actions, the account can often be found in several logs, and it's nicer for it to have a localised name.
Status of this bug right now: the message is actually parsed, so templates and wikisyntax are correct, though we still have the "You do not have permission to move this page, for the following reason:" message. However, this is actually intended (more or less): to filter "move" actions we use the MovePageCheckPermissions hook, which is indeed intended to check for user permissions. So actually we would need to change the used hook, but is there one which is suitable for our case?
Agreed that this should be on a per-filter basis. Automatically disabling filters from serverside just because they are staled is a complete WONTFIX, apart from being not that easy to code.
Fri, Jul 13
Well, it's true that this problem isn't easy to fix per comments above, otherwise it'd be pretty easy. I also thought of a couple of ways to determine whether the page was deleted, but all of them require having some more info in abuse_filter_log rows. Specifically, having afl_rev_id for page creations: with that, one could check if a revision with such id exists in the 'revision' table so to make a clear distinction. However, I can't see other ideas.
I'm looking at this, it's quite a simple fix focused on avoiding a string to bool conversion. However, this could make another problem appear, i.e. T44734 for revdel'ed revisions. I'm seizing the opportunity to fix both.
The patch above only needs Flow to properly format our entry in /test (and maybe some adjustments for /examine). However, not only I don't know Flow code at all, but I'm also having big problems in making Flow work on my wiki.
I determined that Flow entries aren't shown because the onOldChangesListRecentChangesLine hook returns false. This happens for two reasons: first, here the change is excluded from the view. Second, it doesn't exists in this cache. I'm not really sure about what we should do: maybe change the RC entry on AbuseFilter? Or maybe add specific methods to Flow to handle entries in Special:AbuseFilter/test? I'd appreciate it if someone from Flow would give it a look!
Wed, Jul 11
I looked at this. Upload is pretty different from other actions, since it has lot of specific file-related variables to be computed. While filtering the upload we have an UploadBase object from the hook to retrieve such info, however this is not the case for a RC entry. So my question is: do we have a method for retrieving file data from a RC entry? AFAICT, such data is stored in both image and oldimage tables, but they don't seem to be uniquely reachable from RC, if not with timestamp and title. A solution could be, given an RC upload row, to query both image and oldimage tables looking for a match on img_title/oi_name with rc_title and a similar one for timestamp, but it seems poorly efficient to me. Is there a better way?
Agreed that this is truly annoying. Took me a lot of time to file a report due to "concurrent connections".
Tue, Jul 10
I guess adding Equivset to vendor will be fine after T191740, or if AntiSpoof will be bundled (since it uses equivset as well). So maybe changing the job would be the best option?
I think we already purged common.js several times after the blackout, anyway let's see if it works. As for Google, I don't have a link but I can ask for it if you want. What I can do now is provide an example: try to google "Marea", using a cellphone. The first link to the wikipage actually sends to the landing page, the link being exactly https://it.m.wikipedia.org/wiki/Wikipedia:Comunicato_3_luglio_2018.
Several users have reported this problem, however they weren't really redirected: instead, while searching stuff on google, google redirected them to the blackout landing page. For many (if not all) of this cases we have determined that google is actually showing the wrong link, which points straight to the blackout page. We also filed a task at google's, and I'm not sure if there's something at MW-side that could be done. Also, this problem (as reported by Nemo), only occurs from mobile, while desktop is totally fine. That being said, I'd like to ask Nemo if he can confirm that google links are wrong in his case as well, or if it's a separate issue.
And what would be an alternative to having two autoloaders? Anyway, CI is just a case where this problem pops out, but I guess not the only one.
@dbarratt Looking at the comments above:
@dbarratt I'm not sure how to reproduce this. CI shows this error for REL1_31 branch, see e.g. https://integration.wikimedia.org/ci/job/mwext-testextension-hhvm-jessie/46253/console. I also have it on my local wiki, where equivset isn't included in vendor after updates. A composer update solves the problem, although it's not the solution we're looking for.
Mon, Jul 9
Actually, I think we should get rid of this syntax, with or without the caching parser. This isn't sane at all, and greatly increases the chances of writing a wrong filter. Due to foo& == false, I accidentally made an important itwiki filter stop working for a month, until MusikBot reported it and I could spot a trailing & left by mistake while refactoring. We should probably ask for a query (like in T193894) of use cases in production, fix them and then remove this dangerous syntax.
That's nice! Should we make it voting on CI as soon as it'll pass there?
Sun, Jul 8
I think the above patches will provide a really good test background to start with, and should accomplish the goal of this task. Then, they may be expanded progressively.
Sat, Jul 7
This is what I'm progressively try to add as tests:
- Tests for all AFPUserVisibleException exceptions
- Tests for filter validation
- Tests for generic functions in AbuseFilter class
- Complete tests where filters are created, actions are executed and consequences must be taken.
Apart from the latter, everything is under review right now.
@TerraCodes The ones we've seen for the recent blackout, as I was saying. Itwiki had more ore less three days to decide for a blackout, and there has been some uncertainties until one hour before the effective blackout. Other wikis have had even less time, and in order not to be late they opened short polls (we're talking of a matter of hours, not days), for instance cawiki. Some wikis actually ended up with nothing done because of not enough time left to vote, for instance elwiki. In such a situation, where time is really crucial and should be spent in reaching consensus, communities should have a quick way to enforce their decision. At any rate, this is the classical situation that you can't actually foresee, and for which we need to be ready, no matter how rare such situation could be.
Fri, Jul 6
Yes, more or less. The period may be relatively short, and may also happen without explicit bilateral communication between communities and WMF (like in this case). In any case, the blackout would be enabled by a sysadmin.
The idea is to directly transclude the whole page, whatever it is. BTW, the exact timeline I'm thinking of would be this one:
- The community, with the approval of WMF, decides to blackout
- A patch is created and SWATted as soon as possible. We should adapt the $wgBlackout variable to be used for these cases, so that the patch would only consist of $wgBlackout['Enable'] = true.
- The blackout is now enabled, and sysops can change config (landing page, whitelisted groups, almost whatever we want to add)
- As the community decides that the blackout should end, the on-wiki config is edited adding "enabled": false, and another patch for serverside deletion is planned for SWAT. This patch is deployed, and the blackout officially ends.