Page MenuHomePhabricator
Create Task
Maniphest T263851

Disallow dynamic property access on ParserOutput
Open, Needs TriagePublic
Actions

Description

Per mediawiki 1.20 the way for extensions to attach custom data to the ParserOutput was to declare a dynamic property. Now the mechanism exists, ParserOutput::getExtensionData/setExtensionData to do this properly.

We need to find all the places in the deployed extensions where dynamic property access is still happening, replace usages with a fallback on __get, wait for ParserCache expiration period, drop support for it and trigger an error if an attempt is made to use dynamic properties on ParserOutput.

It seems like a good way of tracking the usages of this is by grepping for PhanUndeclaredProperty suppression.

Details

Show related patches Customize query in gerrit

Related Objects
Search...

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
ssastry awarded a token.Sep 25 2020, 2:55 PM
gerritbot added a comment.Sep 25 2020, 3:17 PM

Change 630190 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Allow back getting/setting dynamic properties on ParserOutput.

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

gerritbot added a project: Patch-For-Review.Sep 25 2020, 3:17 PM
gerritbot added a comment.Sep 25 2020, 3:40 PM

Change 630191 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/extensions/ProofreadPage@master] Use ParserOutput::setExtensionData instead of dynamic property access

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

• Pchelolo added a comment.EditedSep 25 2020, 3:48 PM

Checking all the deployed extensions/core:

gerritbot added a comment.Sep 25 2020, 4:05 PM

Change 630208 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/extensions/LiquidThreads@master] Replace use of dynamic property in ParserOutput for extension data.

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

gerritbot added a comment.Sep 25 2020, 4:23 PM

Change 630210 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/extensions/GeoCrumbs@master] Remove fallback for ParserOutput->mCustomData

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

gerritbot added a comment.Sep 25 2020, 4:40 PM

Change 630190 merged by jenkins-bot:
[mediawiki/core@master] Allow back getting/setting dynamic properties on ParserOutput.

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

gerritbot added a comment.Sep 25 2020, 4:40 PM

Change 630191 merged by jenkins-bot:
[mediawiki/extensions/ProofreadPage@master] Use ParserOutput::setExtensionData instead of dynamic property access

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

gerritbot added a comment.Sep 25 2020, 4:40 PM

Change 630208 merged by jenkins-bot:
[mediawiki/extensions/LiquidThreads@master] Replace use of dynamic property in ParserOutput for extension data.

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

gerritbot added a comment.Sep 25 2020, 4:48 PM

Change 630212 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/extensions/GeoData@master] Unsuppress PhanUndeclaredProperty, reason for it was removed

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

gerritbot added a comment.Sep 25 2020, 4:58 PM

Change 630210 merged by jenkins-bot:
[mediawiki/extensions/GeoCrumbs@master] Remove fallback for ParserOutput->mCustomData

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

gerritbot added a comment.Sep 25 2020, 4:59 PM

Change 630214 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/extensions/Insider@master] Drop fallback to reading ParserOutput:mCustomData

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

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.11; 2020-09-29).Sep 25 2020, 5:00 PM
gerritbot added a comment.Sep 25 2020, 5:17 PM

Change 630216 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/extensions/TemplateSandbox@master] Unsuppress PhanUndeclaredProperty, reason for it removed

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

gerritbot added a comment.Sep 25 2020, 5:32 PM

Change 630212 merged by jenkins-bot:
[mediawiki/extensions/GeoData@master] Unsuppress PhanUndeclaredProperty, reason for it was removed

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

gerritbot added a comment.Sep 25 2020, 5:37 PM

Change 630216 merged by jenkins-bot:
[mediawiki/extensions/TemplateSandbox@master] Unsuppress PhanUndeclaredProperty, reason for it removed

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

• Pchelolo mentioned this in rETSA249d9a441a75: Unsuppress PhanUndeclaredProperty, reason for it removed.Sep 25 2020, 5:46 PM
gerritbot added a comment.Sep 25 2020, 5:52 PM

Change 630214 merged by jenkins-bot:
[mediawiki/extensions/Insider@master] Drop fallback to reading ParserOutput:mCustomData

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

• Pchelolo changed the task status from Open to Stalled.Sep 25 2020, 6:19 PM

Ok, now nothing in the deployed extensions or core writes to the dynamic properties in the ParserOutput, however LiquidThreads and ProofreadPage still fallback to the property since it could be coming from the ParserCache.

Now we need to wait for at least 1 month before removing the fallbacks to make sure ParserCache has expired, after that drop the last two fallbacks and either start emitting the deprecate warning or throwing an error if the dynamic property is accessed.

ssastry moved this task from Needs Triage to Non-Parsing-Team Tasks on the Parsoid board.Sep 26 2020, 3:04 AM
Nikerabbit subscribed.Sep 26 2020, 5:51 AM

SemanticMediaWiki also uses dynamic properties. We started getting a bunch of logspam in translatewiki.net yesterday after we updated to get the security fixes. We had to revert the patch locally.

• Pchelolo changed the task status from Stalled to Open.EditedSep 28 2020, 3:06 PM
• Pchelolo edited projects, added Platform Team Workboards (Green); removed Patch-For-Review, Platform Engineering.
In T263851#6495718, @Nikerabbit wrote:

SemanticMediaWiki also uses dynamic properties. We started getting a bunch of logspam in translatewiki.net yesterday after we updated to get the security fixes. We had to revert the patch locally.

With the latest latest version, with partially removed deprecation, you should not be getting logspam for dynamic properties, only direct access to static properties.

I can fix SemanticMediawiki as well.

• Pchelolo claimed this task.Sep 28 2020, 3:07 PM
• Pchelolo moved this task from Backlog to Doing on the Platform Team Workboards (Green) board.
• Pchelolo added a comment.Sep 28 2020, 8:47 PM

https://github.com/SemanticMediaWiki/SemanticMediaWiki/pull/4850 should address SemanticMediawiki

• Pchelolo moved this task from Doing to Blocked on the Platform Team Workboards (Green) board.Sep 28 2020, 8:48 PM

Blocking until ParserCache has expired and we can drop fallbacks in remaining extensions.

Nikerabbit added a comment.Sep 29 2020, 2:12 PM

With the latest latest version, with partially removed deprecation, you should not be getting logspam for dynamic properties, only direct access to static properties.

Thanks! We will test this on Wednesday.

• Pchelolo added a comment.Sep 29 2020, 2:13 PM

@Nikerabbit Also, the semantic mediawiki PR above should fix it on their side. If you see some more logspam, please save it somewhere, I'd go through it and fix all issues

gerritbot added a comment.Sep 30 2020, 11:54 PM

Change 631318 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] Reject ParserCache entries from the last wmf.11 deployment

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

gerritbot added a project: Patch-For-Review.Sep 30 2020, 11:54 PM
Krinkle added projects: Wikimedia-Incident, Wikimedia-production-error.EditedSep 30 2020, 11:57 PM
Krinkle subscribed.

See T264257: Fix ParserOutput corruption wmf.10 -> wmf.11

Krinkle added a parent task: T263177: 1.36.0-wmf.11 deployment blockers.Sep 30 2020, 11:57 PM
Krinkle removed a parent task: T263177: 1.36.0-wmf.11 deployment blockers.
Krinkle added a subtask: T264257: Fix ParserOutput corruption wmf.10 -> wmf.11.Oct 1 2020, 12:00 AM
Krinkle removed projects: Wikimedia-production-error, Wikimedia-Incident, Patch-For-Review.
gerritbot added a comment.Oct 1 2020, 12:03 AM

Change 631320 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] Reject ParserCache entries from the last wmf.11 deployment (2)

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

gerritbot added a project: Patch-For-Review.Oct 1 2020, 12:03 AM
gerritbot added a comment.Oct 1 2020, 12:14 AM

Change 631320 merged by jenkins-bot:
[operations/mediawiki-config@master] Reject ParserCache entries from the last wmf.11 deployment (2)

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

• Pchelolo closed subtask T264257: Fix ParserOutput corruption wmf.10 -> wmf.11 as Resolved.Oct 1 2020, 7:31 PM
• Pchelolo moved this task from Blocked to Ready on the Platform Team Workboards (Green) board.Nov 2 2020, 2:46 PM

Ok, now nothing in the deployed extensions or core writes to the dynamic properties in the ParserOutput, however LiquidThreads and ProofreadPage still fallback to the property since it could be coming from the ParserCache. Now we need to wait for at least 1 month before removing the fallbacks to make sure ParserCache has expired, after that drop the last two fallbacks and either start emitting the deprecate warning or throwing an error if the dynamic property is accessed.

1 month has passed, we can remove the fallback.

gerritbot added a comment.Nov 5 2020, 2:36 PM

Change 639534 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/extensions/ProofreadPage@master] Remove fallback to undeclared ParserOutput property

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

gerritbot added a comment.Nov 5 2020, 2:40 PM

Change 639535 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/extensions/LiquidThreads@master] Remove fallback to dynamic ParserOutput properties

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

• Pchelolo moved this task from Ready to Waiting for Review on the Platform Team Workboards (Green) board.Nov 5 2020, 10:32 PM
gerritbot added a comment.Nov 10 2020, 1:50 AM

Change 640281 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Hard-deprecate all public paroperty access on CacheTime and ParserOutput.

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

gerritbot added a comment.Nov 10 2020, 2:46 PM

Change 639535 merged by jenkins-bot:
[mediawiki/extensions/LiquidThreads@master] Remove fallback to dynamic ParserOutput properties

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

gerritbot added a comment.Nov 10 2020, 2:46 PM

Change 639534 merged by jenkins-bot:
[mediawiki/extensions/ProofreadPage@master] Remove fallback to undeclared ParserOutput property

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

ReleaseTaggerBot edited projects, added MW-1.36-notes (1.36.0-wmf.18; 2020-11-17); removed MW-1.36-notes (1.36.0-wmf.11; 2020-09-29).Nov 10 2020, 3:00 PM
• Pchelolo moved this task from Waiting for Review to Blocked on the Platform Team Workboards (Green) board.Nov 16 2020, 5:16 PM
• Pchelolo closed this task as Resolved.Nov 19 2020, 8:49 PM
• Pchelolo moved this task from Blocked to Done on the Platform Team Workboards (Green) board.
gerritbot added a comment.Nov 19 2020, 9:10 PM

Change 640281 merged by jenkins-bot:
[mediawiki/core@master] Hard-deprecate all public property access on CacheTime and ParserOutput.

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

ReleaseTaggerBot edited projects, added MW-1.36-notes (1.36.0-wmf.20; 2020-12-01); removed MW-1.36-notes (1.36.0-wmf.18; 2020-11-17).Nov 19 2020, 10:00 PM
hashar mentioned this in T269235: [X8ehRwpAICgAALHNCUsAAACI] /wiki/clamor ErrorException from line 157 of /srv/mediawiki/php-1.36.0-wmf.20/extensions/CategoryTree/includes/CategoryTreeHooks.php: ParserOutput::mCategoryTreeTag dynamic property write access deprecated [Called from CategoryTreeHooks::parserHook].EditedDec 2 2020, 2:28 PM
hashar subscribed.

A couple others spotted during the train: T269235 and T269236

ErrorException from line 157 of /srv/mediawiki/php-1.36.0-wmf.20/extensions/CategoryTree/includes/CategoryTreeHooks.php: ParserOutput::mCategoryTreeTag dynamic property write access deprecated [Called from CategoryTreeHooks::parserHook]

[X8elNgpAAL8AASHtlA8AAADE] /wiki/Category:German_pronunciation_of_prepositions ErrorException from line 4021 of /srv/mediawiki/php-1.36.0-wmf.20/includes/parser/Parser.php: ParserOutput::mNoGallery public write access deprecated [Called from Parser::handleDoubleUnderscore]

hashar mentioned this in T269236: [X8elNgpAAL8AASHtlA8AAADE] /wiki/Category:German_pronunciation_of_prepositions ErrorException from line 4021 of /srv/mediawiki/php-1.36.0-wmf.20/includes/parser/Parser.php: ParserOutput::mNoGallery public write access deprecated [Called from Parser::handleDoubleUnderscore].Dec 2 2020, 2:35 PM
hashar added subtasks: T269235: [X8ehRwpAICgAALHNCUsAAACI] /wiki/clamor ErrorException from line 157 of /srv/mediawiki/php-1.36.0-wmf.20/extensions/CategoryTree/includes/CategoryTreeHooks.php: ParserOutput::mCategoryTreeTag dynamic property write access deprecated [Called from CategoryTreeHooks::parserHook], T269236: [X8elNgpAAL8AASHtlA8AAADE] /wiki/Category:German_pronunciation_of_prepositions ErrorException from line 4021 of /srv/mediawiki/php-1.36.0-wmf.20/includes/parser/Parser.php: ParserOutput::mNoGallery public write access deprecated [Called from Parser::handleDoubleUnderscore].
• Pchelolo reopened this task as Open.Dec 2 2020, 2:54 PM

Missed a few.

• Pchelolo moved this task from Done to Doing on the Platform Team Workboards (Green) board.Dec 2 2020, 2:55 PM
gerritbot added a comment.Dec 2 2020, 2:55 PM

Change 644844 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] ParserOutput: temporary undeprecate getting dynamic properties.

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

gerritbot added a comment.Dec 2 2020, 3:02 PM

Change 644848 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Deprecate ParserOutput dynamic properties read

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

gerritbot added a comment.Dec 2 2020, 4:11 PM

Change 644844 merged by jenkins-bot:
[mediawiki/core@master] ParserOutput: temporary undeprecate getting dynamic properties.

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

ReleaseTaggerBot edited projects, added MW-1.36-notes (1.36.0-wmf.21; 2020-12-08); removed MW-1.36-notes (1.36.0-wmf.20; 2020-12-01).Dec 2 2020, 5:00 PM
gerritbot added a comment.Dec 2 2020, 6:00 PM

Change 644817 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@wmf/1.36.0-wmf.20] ParserOutput: temporary undeprecate getting dynamic properties.

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

• Pchelolo moved this task from Doing to Blocked on the Platform Team Workboards (Green) board.Dec 2 2020, 6:59 PM
gerritbot added a comment.Dec 2 2020, 10:07 PM

Change 644817 merged by jenkins-bot:
[mediawiki/core@wmf/1.36.0-wmf.20] ParserOutput: temporary undeprecate getting dynamic properties.

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

ReleaseTaggerBot edited projects, added MW-1.36-notes (1.36.0-wmf.20; 2020-12-01); removed MW-1.36-notes (1.36.0-wmf.21; 2020-12-08).Dec 2 2020, 11:00 PM
• Pchelolo closed subtask T269235: [X8ehRwpAICgAALHNCUsAAACI] /wiki/clamor ErrorException from line 157 of /srv/mediawiki/php-1.36.0-wmf.20/extensions/CategoryTree/includes/CategoryTreeHooks.php: ParserOutput::mCategoryTreeTag dynamic property write access deprecated [Called from CategoryTreeHooks::parserHook] as Resolved.Dec 2 2020, 11:23 PM
• Pchelolo closed subtask T269236: [X8elNgpAAL8AASHtlA8AAADE] /wiki/Category:German_pronunciation_of_prepositions ErrorException from line 4021 of /srv/mediawiki/php-1.36.0-wmf.20/includes/parser/Parser.php: ParserOutput::mNoGallery public write access deprecated [Called from Parser::handleDoubleUnderscore] as Resolved.
gerritbot added a comment.Dec 7 2020, 2:08 PM

Change 646101 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Hard-deprecate all public property access on CacheTime and ParserOutput.

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

gerritbot added a comment.Dec 7 2020, 3:05 PM

Change 644848 abandoned by Ppchelko:
[mediawiki/core@master] Deprecate ParserOutput dynamic properties read

Reason:
Superseded by I70d6feb1c995a0a0f763b21261141ae8ee6dc570

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

MBinder_WMF edited projects, added Parsoid (Tracking); removed Parsoid.Dec 10 2020, 8:12 PM
Krinkle mentioned this in T270228: "Error: Unsupported operand types" from ParserOptionsTest::testMatches on php74.Dec 16 2020, 3:46 AM
• Pchelolo moved this task from Blocked to Waiting for Review on the Platform Team Workboards (Green) board.Jan 4 2021, 4:39 PM
gerritbot added a comment.Oct 13 2021, 6:08 PM

Change 646101 merged by jenkins-bot:

[mediawiki/core@master] Hard-deprecate all public property access on CacheTime and ParserOutput.

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

ReleaseTaggerBot added a project: MW-1.38-notes (1.38.0-wmf.5; 2021-10-19).Oct 13 2021, 7:00 PM
hashar unsubscribed.Jan 6 2022, 9:20 AM
Aklapper removed • Pchelolo as the assignee of this task.Jul 2 2022, 10:06 AM

Removing inactive assignee (Platform Engineering: Please unassign tasks of previous team members.)

Pppery removed a project: Patch-For-Review.Oct 25 2023, 4:17 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits