Skip to content

costume duplication makes potentially-unsafe changes to asset objects #3099

Open
@cwillisf

Description

@cwillisf

Expected Behavior

The assetId for a costume object should match its costume data, or be removed.
The assetId for an asset object should match its contents, or be removed.

Actual Behavior

Duplicating a costume can in theory lead to a costume object containing an asset ID which does not match its costume data. This was an important part of the root cause of the SVG question-mark issue in April 2021.

Steps to Reproduce

  1. Edit a vector costume in the paint editor. This creates a new asset object and calculates a new asset ID for it.
  2. Duplicate the costume
    • in duplicateCostume we shallow-copy the costume object using Object.assign. We now have two costume objects which refer to the same asset object.
    • duplicateCostume then calls loadCostume
    • in some cases loadCostume modifies the contents of the asset object. In these cases it updates the asset ID of the costume being loaded.
    • In the duplication case, if the asset is modified, loadCostume updates the asset ID in the new duplicate copy but does NOT update the asset ID of the other costume sharing the now-modified asset object.
  3. If loadCostume modified the contents of the duplicate costume's asset object, then the original costume now holds an asset ID which does not match its costume data. The duplicate costume's asset ID is accurate.

Note that in practice this currently does not cause problems because currently loadCostume generally does not cause changes in already-loaded costumes.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions