Tue, Aug 11
Untagging Platform Team, retag us if code review is necessary. We put related task T259962 into our External Code Review column.
Thu, Aug 6
Wed, Aug 5
I tried a few things and was unable to reproduce this. @Ablum010777, can you update to the latest 1.35 code and see if this still occurs?
From the stack trace:
General rule: our unit tests should test contracts, not implementations. In other words, test what the class/interface/handler is supposed to do, rather than using special knowledge about how it accomplishes it. (Note: testing failures is fine, failing according to contract is super important)
Tue, Aug 4
Two deployment notes, by the way:
All steps completed. How do we confirm it works as intended?
Mon, Aug 3
Thu, Jul 30
We need to
- cherry-pick the upstream patch to mathjax https://gerrit.wikimedia.org/r/c/mediawiki/services/mathjax/+/606727
- publish a new version of mathoid-mathjax to npm, https://gerrit.wikimedia.org/r/c/mediawiki/services/mathjax/+/606732
- update the dependency in mathjax-node to the new mathoid release publish that new version to npm, https://gerrit.wikimedia.org/r/c/mediawiki/services/mathjax-node/+/606742/1/package.json
- and eventually update the dependency in mathoid. https://gerrit.wikimedia.org/r/c/mediawiki/services/mathoid/+/606760
Wed, Jul 29
6 occurrences seen in logstash on 2020-07-29.
Seen in logstash on 2020-07-29:
See in logstash on 2020-07-29:
Tue, Jul 28
I'm no train log triage expert, so I could be misinterpreting. But looking back over the last couple of months, I'm seeing this in multiple mediawiki versions, and I'm not seeing an enormous spike in recent frequency. Maybe the last few days have been slightly higher, but not by orders of magnitude. So from a train conductor's perspective, I'm not seeing anything worthy of train blocking.
If I'm reading this correctly, the error is occurring in 1.36.0-wmf.1, but 9bd04f784566 didn't hit until 1.36.0-wmf.2. Let me know if I'm not looking at that correctly.
Will look at this sometime today if someone doesn't beat me to it. Daniel is out this week. Adding @Pchelolo and @DannyS712 (who have also recently worked on Revision code) and @CCicalese_WMF (who is currently on Platform Team Clinic Duty).
Mon, Jul 27
Sure thing. I am not yet familiar with backporting upstream mathjax changes, so (as @Physikerwelt said) this will take some extra time for learning. But that's knowledge I need to acquire anyway, so should be time well spent.
Fri, Jul 17
Jul 15 2020
Jul 14 2020
@tstarling , do you have any thoughts here? (Note to other people on this task - Tim is on vacation until July 20, so an answer may be delayed.)
@JTannerWMF , we are trying to triage this and it is unclear what is needed. Review on the patch, or does something need to be done to RESTBase?
@Pchelolo , any thoughts here?
Removing tag that was auto-added by Herald. Retag if there's work for CPT here.
Please retag if there's work for CPT on this.
hooks.txt already references Hooks.md. It seems like we can simply eliminate everything from hooks.txt except the first sentence.
Status note: related tasks T233963: Add serialization options to RESTBagOStuff and T234779: Removed deprecated "legacy" serialization type from RESTBagOStuff are complete. RESTBagOStuff now supports (only) PHP and JSON serialization types, with (optional) hmac protection. It is in use on production wikis of all groups to store sessions in Kask, using PHP serialization and hmac protection.
Jul 13 2020
Jul 2 2020
Looking a bit more, RevisionStore::constructSlotRecords (the function that throws the exception in question) already logs related diagnostic info via $this->logger->info(), so I followed that same pattern for the new log message with revision id.
Jun 30 2020
I do not believe this an actual security concern from a technical/code perspective. However, it may be a concern from a policy/legal perspective.
Jun 25 2020
Yep, that syntax would create the desired structure.
This sounds like the kind of thing that doesn't matter very much, until it does. Then it matters a lot, and is difficult to fix quickly.
Jun 24 2020
Already in CPT External Code Reviews, so untagging CPT so that it moves out of the inbox.
Is this done?
Adding @apaskulin as a subscriber.
Untagging CPT as this may not actually be an issue. Retag us if needed.
Declining. Reopen if you think this is important.
Already assigned to @hnowlan , so moving to Green Team board.
@DannyS712 , moved this to CPT watching, but please reach out to us if we can help.
@tstarling , does this make any sense to you?
Unclear how this would have worked on group0. We will look into it further to see what we can learn.