Page MenuHomePhabricator
Create Task
Maniphest T305158

Tighten up ParserOutput::setPageProperty() to only allow string values; add ::setIndexedPageProperty() for numeric values
Open, Needs TriagePublic
Actions

Description

See the discussion in T301915#7821799: although ParserOutput will properly store any JSON-serializable value, the deferred LinksUpdate task will stringify everything before storing it in the database. Further, the sort key will be null unless the value is int|float|bool.

This is a dangerous pattern, as many implicit conversions in PHP will convert between numeric strings (strings which "look like" numbers) and int|float. For example, storing a numeric string as an array_key will convert it to a number. It is too easy to either (a) add bogus sort keys to things like title strings that just sometimes happen to be numbers (and which were stored as array keys and thus numeric-ified), or (b) store a numeric string instead of an actual number and thus get the sort key dropped (see T350224).

It would be best to split this into two methods:

  • ::setIndexedPageProperty() which only takes int|float and coerces its values to numbers --- at least until PHP 8 when we can use the union type as a type hint
    • This can also allow specification of sort key independent from value, as hinted by a note in PagePropsTable (Txxxx)
  • ::setUnindexedPageProperty() which only takes string and ensures the values won't be inadvertently indexed

Details

Subject Repo Branch Lines +/-
Bump wikimedia/parsoid to 0.20.0-a2 mediawiki/vendor master +279 -121
Ensure pages are indexed by quality level mediawiki/extensions/ProofreadPage master +2 -2
[CMC] Add ::setUnsortedPageProperty and ::setNumericPageProperty mediawiki/services/parsoid REL1_42 +136 -20
[CMC] Add ::setUnsortedPageProperty and ::setNumericPageProperty mediawiki/services/parsoid master +136 -20
[CMC] Add ::setUnindexedPageProperty and ::setIndexedPageProperty mediawiki/services/parsoid master +111 -20
ParserOutput: clarify that “indexed” refers to value mediawiki/core REL1_42 +5 -3
ParserOutput::setUnindexedPageProperty(): use empty string as default value mediawiki/core REL1_42 +4 -3
ParserOutput: clarify that “indexed” refers to value mediawiki/core master +5 -3
ParserOutput::setUnindexedPageProperty(): use empty string as default value mediawiki/core master +4 -3
Add ParserOutput::setIndexedPageProperty(); deprecate numeric properties mediawiki/core REL1_42 +174 -48
Add ParserOutput::setIndexedPageProperty(); deprecate numeric properties mediawiki/core master +174 -48
ParserOutput::setPageProperty(): Emit deprecation warning for non-scalar values mediawiki/core master +15 -2
[GlobeCoordinateKartographerDataUpdaterTest] Use valid page property values mediawiki/extensions/Wikibase master +2 -2
ParserOutput::setPageProperty(): Update documentation mediawiki/core master +29 -15
Page properties should always be strings mediawiki/extensions/Wikibase master +34 -33
Deprecate the use of non-string page property values mediawiki/core master +24 -5
Page properties should always be strings mediawiki/extensions/WikibaseLexeme master +6 -6
Ensure page properties are cast to string before comparison mediawiki/extensions/Wikibase master +1 -1
Page properties should always be strings mediawiki/extensions/PagedTiffHandler master +4 -4
Page properties should always be strings mediawiki/extensions/LiquidThreads master +3 -3
Page properties should always be strings mediawiki/extensions/WikidataPageBanner master +2 -2
Page properties should always be strings mediawiki/extensions/Score master +3 -2
Page properties should always be strings mediawiki/extensions/JsonConfig master +2 -2
Page properties should always be strings mediawiki/extensions/Kartographer master +2 -2
Page properties should always be strings mediawiki/extensions/Cite master +2 -2
Page properties should always be strings mediawiki/extensions/ProofreadPage master +4 -1
[doc only] Improve the documentation re non-string page property values mediawiki/core master +14 -7
Show related patches Customize query in gerrit

Related Objects
Search...

Event Timeline

cscott created this task.Mar 31 2022, 3:01 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 31 2022, 3:01 PM
cscott mentioned this in T301915: Before 1.38: ParserOutput::getPageProperty() should return null, not false.Mar 31 2022, 3:02 PM
gerritbot added a comment.Mar 31 2022, 3:11 PM

Change 775881 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] [doc only] Improve the documentation re non-string page property values

https://gerrit.wikimedia.org/r/775881
gerritbot added a project: Patch-For-Review.Mar 31 2022, 3:11 PM
gerritbot added a comment.Mar 31 2022, 3:55 PM

Change 775881 merged by jenkins-bot:

[mediawiki/core@master] [doc only] Improve the documentation re non-string page property values

https://gerrit.wikimedia.org/r/775881
ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.6; 2022-04-04).Mar 31 2022, 4:00 PM
Maintenance_bot removed a project: Patch-For-Review.Mar 31 2022, 4:10 PM
gerritbot added a comment.Mar 31 2022, 5:31 PM

Change 775901 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Wikibase@master] WIP: Only store strings as page property values

https://gerrit.wikimedia.org/r/775901
gerritbot added a project: Patch-For-Review.Mar 31 2022, 5:31 PM
gerritbot added a comment.Mar 31 2022, 5:36 PM

Change 775902 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Cite@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775902
gerritbot added a comment.Mar 31 2022, 5:39 PM

Change 775903 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/JsonConfig@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775903
gerritbot added a comment.Mar 31 2022, 5:43 PM

Change 775905 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Kartographer@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775905
gerritbot added a comment.Mar 31 2022, 5:45 PM

Change 775882 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] WIP: Deprecate the use of non-string page property values

https://gerrit.wikimedia.org/r/775882
gerritbot added a comment.Mar 31 2022, 6:00 PM

Change 775907 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/LiquidThreads@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775907
gerritbot added a comment.Mar 31 2022, 6:02 PM

Change 775908 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/PagedTiffHandler@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775908
gerritbot added a comment.Mar 31 2022, 6:05 PM

Change 775909 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/ProofreadPage@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775909
gerritbot added a comment.Mar 31 2022, 6:07 PM

Change 775911 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Score@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775911
gerritbot added a comment.Mar 31 2022, 6:17 PM

Change 775915 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/WikidataPageBanner@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775915
gerritbot added a comment.Mar 31 2022, 7:24 PM

Change 775909 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775909
gerritbot added a comment.Mar 31 2022, 11:37 PM

Change 776016 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/WikibaseLexeme@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/776016
gerritbot added a comment.Apr 1 2022, 12:03 AM

Change 776019 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Wikibase@master] Ensure page properties are cast to string before comparison

https://gerrit.wikimedia.org/r/776019
gerritbot added a comment.Apr 4 2022, 10:47 PM

Change 775902 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775902
gerritbot added a comment.Apr 5 2022, 3:07 PM

Change 775905 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775905
ReleaseTaggerBot edited projects, added MW-1.39-notes (1.39.0-wmf.7; 2022-04-11); removed MW-1.39-notes (1.39.0-wmf.6; 2022-04-04).Apr 5 2022, 4:00 PM
gerritbot added a comment.Apr 5 2022, 5:58 PM

Change 775903 merged by jenkins-bot:

[mediawiki/extensions/JsonConfig@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775903
gerritbot added a comment.Apr 5 2022, 6:01 PM

Change 775911 merged by jenkins-bot:

[mediawiki/extensions/Score@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775911
gerritbot added a comment.Apr 5 2022, 6:04 PM

Change 775915 merged by jenkins-bot:

[mediawiki/extensions/WikidataPageBanner@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775915
gerritbot added a comment.Apr 7 2022, 5:45 AM

Change 775907 merged by jenkins-bot:

[mediawiki/extensions/LiquidThreads@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775907
gerritbot added a comment.Apr 7 2022, 5:47 AM

Change 775908 merged by jenkins-bot:

[mediawiki/extensions/PagedTiffHandler@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/775908
gerritbot added a comment.Apr 7 2022, 9:30 PM

Change 776019 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Ensure page properties are cast to string before comparison

https://gerrit.wikimedia.org/r/776019
gerritbot added a comment.Apr 7 2022, 9:30 PM

Change 776016 merged by jenkins-bot:

[mediawiki/extensions/WikibaseLexeme@master] Page properties should always be strings

https://gerrit.wikimedia.org/r/776016
Lucas_Werkmeister_WMDE subscribed.Apr 8 2022, 9:09 AM

This change makes no sense to me. The database table is specifically optimized for numerical values with pp_sortkey – why should we cast properties that semantically represent numbers and will be used as numbers into strings?
Lucas_Werkmeister_WMDE mentioned this in T350224: [LEX] pp_sortkey is null for wb-claims, wbl-forms and wbl-senses on many Lexemes.Nov 1 2023, 10:05 AM
Michael subscribed.Nov 10 2023, 3:49 PM
cscott renamed this task from Tighten up ParserOutput::setPageProperty() to only allow string values to Tighten up ParserOutput::setPageProperty() to only allow string values; add ::setIndexedPageProperty() for numeric values.Feb 16 2024, 2:54 PM
cscott updated the task description. (Show Details)
cscott added a parent task: T357783: Allow page properties to independently specify value and sort key.Feb 16 2024, 2:59 PM
gerritbot added a comment.Feb 16 2024, 3:29 PM

Change 1004146 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] ParserOutput::setPageProperty(): Update documentation

https://gerrit.wikimedia.org/r/1004146
gerritbot added a comment.Feb 16 2024, 3:29 PM

Change 1004147 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] ParserOutput::setPageProperty(): Emit deprecation warning for non-scalar values

https://gerrit.wikimedia.org/r/1004147
gerritbot added a comment.Feb 16 2024, 3:29 PM

Change 1004148 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] Add ParserOutput::setIndexedPageProperty(); deprecate numeric properties

https://gerrit.wikimedia.org/r/1004148
gerritbot added a comment.Feb 16 2024, 3:30 PM

Change 775882 abandoned by C. Scott Ananian:

[mediawiki/core@master] Deprecate the use of non-string page property values

Reason:

Abandoned in favor of I8a39a7c90341dfee932aa819c9a0a637a8782f69; see updated T305158.

https://gerrit.wikimedia.org/r/775882
gerritbot added a comment.Feb 16 2024, 4:41 PM

Change 1004195 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/extensions/Wikibase@master] [GlobeCoordinateKartographerDataUpdaterTest] Use valid page property values

https://gerrit.wikimedia.org/r/1004195
gerritbot added a comment.Feb 20 2024, 4:41 PM

Change 775901 abandoned by C. Scott Ananian:

[mediawiki/extensions/Wikibase@master] Page properties should always be strings

Reason:

Abandoned in favor of Ib36787d04c0ca713587dc8b814ca1c5a827f6f72

https://gerrit.wikimedia.org/r/775901
gerritbot added a comment.Feb 20 2024, 5:23 PM

Change 1004146 merged by jenkins-bot:

[mediawiki/core@master] ParserOutput::setPageProperty(): Update documentation

https://gerrit.wikimedia.org/r/1004146
ReleaseTaggerBot added a project: MW-1.42-notes (1.42.0-wmf.20; 2024-02-27).Feb 22 2024, 11:01 AM
gerritbot added a comment.Mar 11 2024, 5:08 PM

Change 1004195 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] [GlobeCoordinateKartographerDataUpdaterTest] Use valid page property values

https://gerrit.wikimedia.org/r/1004195
gerritbot added a comment.Mar 11 2024, 5:08 PM

Change 1004147 merged by jenkins-bot:

[mediawiki/core@master] ParserOutput::setPageProperty(): Emit deprecation warning for non-scalar values

https://gerrit.wikimedia.org/r/1004147
ReleaseTaggerBot edited projects, added MW-1.42-notes (1.42.0-wmf.22; 2024-03-12); removed MW-1.42-notes (1.42.0-wmf.20; 2024-02-27).Mar 11 2024, 7:00 PM
gerritbot added a comment.Apr 10 2024, 5:44 PM

Change #1004148 merged by jenkins-bot:

[mediawiki/core@master] Add ParserOutput::setIndexedPageProperty(); deprecate numeric properties

https://gerrit.wikimedia.org/r/1004148
Maintenance_bot removed a project: Patch-For-Review.Apr 10 2024, 6:30 PM
gerritbot added a comment.Apr 10 2024, 9:46 PM

Change #1018798 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/services/parsoid@master] [CMC] Add ::setUnindexedPageProperty and ::setIndexedPageProperty

https://gerrit.wikimedia.org/r/1018798
gerritbot added a project: Patch-For-Review.Apr 10 2024, 9:46 PM
gerritbot added a comment.Apr 11 2024, 2:06 PM

Change #1018970 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@REL1_42] Add ParserOutput::setIndexedPageProperty(); deprecate numeric properties

https://gerrit.wikimedia.org/r/1018970
gerritbot added a comment.Apr 11 2024, 2:11 PM

Change #1018970 abandoned by C. Scott Ananian:

[mediawiki/core@REL1_42] Add ParserOutput::setIndexedPageProperty(); deprecate numeric properties

Reason:

Botched the cherry-pick

https://gerrit.wikimedia.org/r/1018970
gerritbot added a comment.Apr 11 2024, 2:14 PM

Change #1018970 restored by C. Scott Ananian:

[mediawiki/core@REL1_42] Add ParserOutput::setIndexedPageProperty(); deprecate numeric properties

https://gerrit.wikimedia.org/r/1018970
gerritbot added a comment.Apr 11 2024, 2:47 PM

Change #1018970 merged by jenkins-bot:

[mediawiki/core@REL1_42] Add ParserOutput::setIndexedPageProperty(); deprecate numeric properties

https://gerrit.wikimedia.org/r/1018970
gerritbot added a comment.Apr 11 2024, 4:00 PM

Change #1019071 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] ParserOutput::setUnindexedPageProperty(): use empty string as default value

https://gerrit.wikimedia.org/r/1019071
gerritbot added a comment.Apr 12 2024, 10:09 AM

Change #1019258 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/core@master] ParserOutput: clarify that “indexed” refers to value

https://gerrit.wikimedia.org/r/1019258
gerritbot added a comment.Apr 12 2024, 4:10 PM

Change #1019179 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@REL1_42] ParserOutput::setUnindexedPageProperty(): use empty string as default value

https://gerrit.wikimedia.org/r/1019179
gerritbot added a comment.Apr 12 2024, 4:12 PM

Change #1019300 had a related patch set uploaded (by C. Scott Ananian; author: Lucas Werkmeister (WMDE)):

[mediawiki/core@REL1_42] ParserOutput: clarify that “indexed” refers to value

https://gerrit.wikimedia.org/r/1019300
gerritbot added a comment.Apr 12 2024, 4:30 PM

Change #1019071 merged by jenkins-bot:

[mediawiki/core@master] ParserOutput::setUnindexedPageProperty(): use empty string as default value

https://gerrit.wikimedia.org/r/1019071
gerritbot added a comment.Apr 12 2024, 4:34 PM

Change #1019258 merged by jenkins-bot:

[mediawiki/core@master] ParserOutput: clarify that “indexed” refers to value

https://gerrit.wikimedia.org/r/1019258
ReleaseTaggerBot added a project: MW-1.43-notes (1.43.0-wmf.1; 2024-04-16).Apr 12 2024, 5:00 PM
gerritbot added a comment.Apr 12 2024, 10:26 PM

Change #1019360 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/services/parsoid@master] [CMC] Add ::setUnsortedPageProperty and ::setSortedPageProperty

https://gerrit.wikimedia.org/r/1019360
gerritbot added a comment.Apr 15 2024, 9:21 AM

Change #1019179 merged by jenkins-bot:

[mediawiki/core@REL1_42] ParserOutput::setUnindexedPageProperty(): use empty string as default value

https://gerrit.wikimedia.org/r/1019179
gerritbot added a comment.Apr 15 2024, 9:21 AM

Change #1019300 merged by jenkins-bot:

[mediawiki/core@REL1_42] ParserOutput: clarify that “indexed” refers to value

https://gerrit.wikimedia.org/r/1019300
ReleaseTaggerBot edited projects, added MW-1.42-notes; removed MW-1.42-notes (1.42.0-wmf.22; 2024-03-12).Apr 15 2024, 10:00 AM
gerritbot added a comment.Apr 15 2024, 9:11 PM

Change #1018798 abandoned by C. Scott Ananian:

[mediawiki/services/parsoid@master] [CMC] Add ::setUnindexedPageProperty and ::setIndexedPageProperty

Reason:

Abandoned in favor of Ied9948e10e49113dc0082726b16759e0676444f8

https://gerrit.wikimedia.org/r/1018798
gerritbot added a comment.Apr 16 2024, 7:29 PM

Change #1019360 merged by jenkins-bot:

[mediawiki/services/parsoid@master] [CMC] Add ::setUnsortedPageProperty and ::setNumericPageProperty

https://gerrit.wikimedia.org/r/1019360
gerritbot added a comment.Apr 16 2024, 7:48 PM

Change #1020340 had a related patch set uploaded (by Arlolra; author: C. Scott Ananian):

[mediawiki/services/parsoid@REL1_42] [CMC] Add ::setUnsortedPageProperty and ::setNumericPageProperty

https://gerrit.wikimedia.org/r/1020340
gerritbot added a comment.Apr 16 2024, 8:05 PM

Change #1020340 merged by jenkins-bot:

[mediawiki/services/parsoid@REL1_42] [CMC] Add ::setUnsortedPageProperty and ::setNumericPageProperty

https://gerrit.wikimedia.org/r/1020340
gerritbot added a comment.Apr 17 2024, 3:32 AM

Change #1004193 had a related patch set uploaded (by Subramanya Sastry; author: C. Scott Ananian):

[mediawiki/extensions/ProofreadPage@master] Ensure pages are indexed by quality level

https://gerrit.wikimedia.org/r/1004193
gerritbot added a comment.Apr 17 2024, 6:08 AM

Change #1004193 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Ensure pages are indexed by quality level

https://gerrit.wikimedia.org/r/1004193
ReleaseTaggerBot edited projects, added MW-1.43-notes (1.43.0-wmf.2; 2024-04-23); removed MW-1.43-notes (1.43.0-wmf.1; 2024-04-16).Apr 17 2024, 7:01 AM
gerritbot added a comment.Apr 19 2024, 11:26 PM

Change #1022258 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.20.0-a2

https://gerrit.wikimedia.org/r/1022258
gerritbot added a comment.Mon, Apr 22, 3:30 AM

Change #1022258 merged by jenkins-bot:

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.20.0-a2

https://gerrit.wikimedia.org/r/1022258
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