Problem/Motivation

There are security problems when using serialized arrays in PHP code.
JSON data storage is a requirement of Drupal 10 (https://www.drupal.org/node/3192484), and the D9 version will use it soon.

Proposed resolution

Convert to JSON data storage.

Remaining tasks

  • Convert to JSON data storage.
  • Update custom update scripts.
  • Add an update script to update existing configuration.
  • Test coverage for the config update script.
  • Add an update script to update stored entity data.
  • Test coverage for the entity update script.
  • Dig into the test failure from #22, which suggests an API breakage somewhere.

User interface changes

None.

API changes

TBD

Data model changes

TBD

CommentFileSizeAuthor
#29 metatag-n3273408-29.patch21.84 KBdamienmckenna
#29 metatag-n3273408-29.interdiff.txt1.89 KBdamienmckenna
#27 metatag-n3273408-27.patch21.63 KBdamienmckenna
#27 metatag-n3273408-27.interdiff.txt1.05 KBdamienmckenna
#24 metatag-n3273408-24.patch22.24 KBdamienmckenna
#24 metatag-n3273408-24.interdiff.txt616 bytesdamienmckenna
#22 metatag-n3273408-22.patch21.51 KBdamienmckenna
#22 metatag-n3273408-22.interdiff.txt7.17 KBdamienmckenna
#18 metatag-n3273408-18.patch17.84 KBdamienmckenna
#18 metatag-n3273408-18.interdiff.txt1.6 KBdamienmckenna
#16 metatag-n3273408-16.patch16.35 KBdamienmckenna
#16 metatag-n3273408-16.interdiff.txt3.1 KBdamienmckenna
#14 metatag-n3273408-14.patch14.37 KBdamienmckenna
#14 metatag-n3273408-14.interdiff.txt654 bytesdamienmckenna
#12 metatag-n3273408-12.patch14.37 KBdamienmckenna
#12 metatag-n3273408-12.interdiff.txt396 bytesdamienmckenna
#10 metatag-n3273408-10.patch14.41 KBdamienmckenna
#10 metatag-n3273408-10.interdiff.txt9.53 KBdamienmckenna
#7 metatag-n3273408-7.patch5.15 KBdamienmckenna
#7 metatag-n3273408-7.interdiff.txt434 bytesdamienmckenna
#5 metatag-n3273408-5.patch4.84 KBdamienmckenna

Comments

DamienMcKenna created an issue. See original summary.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Parent issue: #3272768: Convert to JSON data storage (D9) »
Related issues: +#3272768: Convert to JSON data storage (D9)
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Parent issue: » #3273409: Plan for Metatag 7.x-2.0
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Active » Needs review
StatusFileSize
new metatag-n3273408-5.patch4.84 KB

WIP patch. MetatagCoreNodeTest passes locally, hopefully everything else will too.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Version: 8.x-1.x-dev » 7.x-1.x-dev
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes
StatusFileSize
new metatag-n3273408-7.interdiff.txt434 bytes
new metatag-n3273408-7.patch5.15 KB

metatag_metatags_save() wasn't updated.

Might need some more test coverage for this..

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Related issues: +#3323254: Add test coverage for metatag_metatags_save()

Test coverage for metatag_metatags_save() will be done in #3323254: Add test coverage for metatag_metatags_save().

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Assigned: Unassigned » damienmckenna
Issue summary: View changes

Next off: update scripts.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new metatag-n3273408-10.interdiff.txt9.53 KB
new metatag-n3273408-10.patch14.41 KB

Test coverage.

Status: Needs review » Needs work

The last submitted patch, 10: metatag-n3273408-10.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new metatag-n3273408-12.interdiff.txt396 bytes
new metatag-n3273408-12.patch14.37 KB

Copy pasta.

Status: Needs review » Needs work

The last submitted patch, 12: metatag-n3273408-12.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new metatag-n3273408-14.interdiff.txt654 bytes
new metatag-n3273408-14.patch14.37 KB

More copy pasta.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes
Status: Needs review » Needs work

Next off: review the defaults configuration system, make sure it works, write an update script if needed.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new metatag-n3273408-16.interdiff.txt3.1 KB
new metatag-n3273408-16.patch16.35 KB

This should cover the config changes.

Status: Needs review » Needs work

The last submitted patch, 16: metatag-n3273408-16.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new metatag-n3273408-18.interdiff.txt1.6 KB
new metatag-n3273408-18.patch17.84 KB

This resolves the i18n issues locally.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes

Last step - test coverage for the config update script.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Needs work
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue tags: +Needs change record

We also need a change record because of the API change over the configuration object.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new metatag-n3273408-22.interdiff.txt7.17 KB
new metatag-n3273408-22.patch21.51 KB

Test coverage for the config update, and some fixes to it.

Status: Needs review » Needs work

The last submitted patch, 22: metatag-n3273408-22.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new metatag-n3273408-24.interdiff.txt616 bytes
new metatag-n3273408-24.patch22.24 KB

Hopefully this will fix the test failures.

Though I don't honestly understand why #22 failed.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes
Status: Needs review » Needs work

The only thing left is to dig into the test failure in #22, which suggests an API breakage somewhere. And the change record.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

WIP change notice: https://www.drupal.org/node/3325825

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new metatag-n3273408-27.interdiff.txt1.05 KB
new metatag-n3273408-27.patch21.63 KB

Rolling back the change in the Xss test file, just to see if it'll work this time?

Status: Needs review » Needs work

The last submitted patch, 27: metatag-n3273408-27.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new metatag-n3273408-29.interdiff.txt1.89 KB
new metatag-n3273408-29.patch21.84 KB

This should fix the test failures.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes

I think that's it!

  • DamienMcKenna committed cf9f0faa on 7.x-2.x 
    Issue #3273408 by DamienMcKenna: Convert to JSON data storage.
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Needs review » Fixed
Issue tags: -Needs change record

Committed to the new 7.x-2.x branch!

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Assigned: damienmckenna » Unassigned

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.