craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.28k stars 635 forks source link

Changing Asset fields on SVG throws loadFromSVG error #7295

Closed gbowne-quickbase closed 3 years ago

gbowne-quickbase commented 3 years ago

Description

We have a text field imageAltText on our assets, and we've made it required. When this field is edited for SVGs, we get an "Unknown error" thrown in production, but in dev mode, we see the error is in Raster::loadFromSVG() and we get the same error whether the image is uploaded locally or to S3. The image renders fine, and passes linting. I originally thought this might be an nystudio107/craft-imageoptimize error, and sent it over to @khalwat https://github.com/nystudio107/craft-imageoptimize/issues/170 and he suggested I post it here

Screen Shots: image

image

Sample SVG: <?xml version="1.0" encoding="UTF-8"?>

Steps to reproduce

  1. Add a text field to the Asset folders
  2. Upload SVG to asset folder
  3. Edit the text field on the asset
  4. Save

Additional info

    • Amazon S3: 1.2.11
    • Canary: 3.0.4
    • Cloudflare: 0.6.0
    • Cookie Consent Banner: 1.2.7
    • Delete Entry Versions: 2.0.0
    • Element API: 2.6.0
    • Feed Me: 4.3.4
    • Field Manager: 2.2.1
    • GeoMate: v1.2.1
    • Icon Picker: 1.1.10
    • ImageOptimize: 1.6.20
    • Linkit: 1.1.12.1
    • Redactor: 2.8.5
    • Redactor Anchors: 1.1.0
    • Retour: 3.1.45
    • SEOmatic: 3.3.26
    • Similar: 1.0.6
    • Simple Sharing: 1.0.8
    • Smith: 1.1.10
    • Sprig: 1.2.0
    • Super Table: 2.6.4
    • Tags: 1.0.7
    • Twigpack: 1.2.7
    • Workflow: 1.5.7
    • XML Sitemap: 1.3.1
brandonkelly commented 3 years ago

I’m not able to reproduce this on a fresh Craft install. Can you search for the error in storage/logs/web.log and post the stack trace?

gbowne-quickbase commented 3 years ago

The more I dive into the web.log the more I wonder why the error is being thrown. My SVGs aren't part of the ImageOptimize config for resizing, but they seem to be running into that flow. Then the error itself is when Craft tries to open the file, so the transaction rolls back. Here's a good chunk of the transaction, with some redactions:

2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][info][yii\db\Command::execute] UPDATE `craft_content` SET `elementId`=978394, `siteId`=1, `title`='Icon a b test', `field_imageAltText`='Test alt text', `field_imageDescription`='Test', `dateUpdated`='2020-12-23 14:17:55' WHERE `id`=48752
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][profile begin][yii\db\Command::execute] UPDATE `craft_content` SET `elementId`=978394, `siteId`=1, `title`='Icon a b test', `field_imageAltText`='Test alt text', `field_imageDescription`='Test', `dateUpdated`='2020-12-23 14:17:55' WHERE `id`=48752
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][profile end][yii\db\Command::execute] UPDATE `craft_content` SET `elementId`=978394, `siteId`=1, `title`='Icon a b test', `field_imageAltText`='Test alt text', `field_imageDescription`='Test', `dateUpdated`='2020-12-23 14:17:55' WHERE `id`=48752
......
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][info][nystudio107\imageoptimize\services\OptimizedImages::shouldCreateVariants] Array
(
)

2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][profile begin][nystudio107\imageoptimize\services\OptimizedImages::populateOptimizedImageModel] populateOptimizedImageModel
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][profile begin][nystudio107\imageoptimize\services\OptimizedImages::addVariantImageToModel] addVariantImageToModel
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][trace][nystudio107\imageoptimize\{closure}] Assets::EVENT_GET_ASSET_URL
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][profile begin][nystudio107\imageoptimize\services\Optimize::handleGetAssetUrlEvent] handleGetAssetUrlEvent
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][profile end][nystudio107\imageoptimize\services\Optimize::handleGetAssetUrlEvent] handleGetAssetUrlEvent
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][info][yii\db\Command::query] SELECT `id`, `assetId`, `filename`, `format`, `location`, `volumeId`, `fileExists`, `inProgress`, `error`, `dateIndexed`, `dateUpdated`, `dateCreated`
FROM `craft_assettransformindex`
WHERE ((`volumeId`=10) AND (`assetId`=978394) AND (`location`='_32x32_crop_center-center_line')) AND (`format` IS NULL)
LIMIT 1
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][profile begin][yii\db\Command::query] SELECT `id`, `assetId`, `filename`, `format`, `location`, `volumeId`, `fileExists`, `inProgress`, `error`, `dateIndexed`, `dateUpdated`, `dateCreated`
FROM `craft_assettransformindex`
WHERE ((`volumeId`=10) AND (`assetId`=978394) AND (`location`='_32x32_crop_center-center_line')) AND (`format` IS NULL)
LIMIT 1
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][profile end][yii\db\Command::query] SELECT `id`, `assetId`, `filename`, `format`, `location`, `volumeId`, `fileExists`, `inProgress`, `error`, `dateIndexed`, `dateUpdated`, `dateCreated`
FROM `craft_assettransformindex`
WHERE ((`volumeId`=10) AND (`assetId`=978394) AND (`location`='_32x32_crop_center-center_line')) AND (`format` IS NULL)
LIMIT 1
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][info][nystudio107\imageoptimize\services\OptimizedImages::addVariantImageToModel] URL created: /uploads/case-studies/_32x32_crop_center-center_line/icon-a-b-test.svg
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][profile begin][nystudio107\imageoptimize\services\OptimizedImages::generatePlaceholders] generatePlaceholders
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][info][nystudio107\imageoptimize\services\OptimizedImages::generatePlaceholders] generatePlaceholders for: nystudio107\imageoptimize\models\OptimizedImage Object
(
    [optimizedImageUrls] => Array
        (
            [32] => /uploads/case-studies/_32x32_crop_center-center_line/icon-a-b-test.svg
        )

    [optimizedWebPImageUrls] => Array
        (
            [32] => /uploads/case-studies/_32x32_crop_center-center_line/icon-a-b-test.svg.webp
        )

.....   

2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][profile begin][nystudio107\imageoptimize\services\Placeholder::createTempPlaceholderImage] createTempPlaceholderImage
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][info][nystudio107\imageoptimize\services\Placeholder::createTempPlaceholderImage] Creating temporary placeholder image for asset
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][trace][yii\db\Transaction::rollBack] Roll back transaction
2020-12-23 09:17:55 [-][58487][prqc272ch66kme5pi2d6us2tmt][error][craft\errors\ImageException] Imagine\Exception\RuntimeException: An image could not be created from the given input in /user-dir/sites/quickbase3/vendor/pixelandtonic/imagine/src/Gd/Imagine.php:214
Stack trace:
#0 /var/www/site/vendor/pixelandtonic/imagine/src/Gd/Imagine.php(108): Imagine\Gd\Imagine->doLoad('<?xml version="...', Object(Imagine\Image\Metadata\MetadataBag))
#1 /var/www/site/vendor/craftcms/cms/src/image/Raster.php(509): Imagine\Gd\Imagine->load('<?xml version="...')
#2 /var/www/site/vendor/craftcms/cms/src/services/Images.php(197): craft\image\Raster->loadFromSVG('<?xml version="...')
#3 /var/www/site/vendor/nystudio107/craft-imageoptimize/src/services/Placeholder.php(271): craft\services\Images->loadImage('/user-dir/s...', true, 300)
#4 /var/www/site/vendor/nystudio107/craft-imageoptimize/src/services/Placeholder.php(245): nystudio107\imageoptimize\services\Placeholder->createImageFromPath('/user-dir/s...', 300, 300, 75, Array)
#5 /var/www/site/vendor/nystudio107/craft-imageoptimize/src/services/Placeholder.php(223): nystudio107\imageoptimize\services\Placeholder->createImageFromAsset(Object(craft\elements\Asset), 300, 300, 75, Array)
#6 /var/www/site/vendor/nystudio107/craft-imageoptimize/src/services/OptimizedImages.php(437): nystudio107\imageoptimize\services\Placeholder->createTempPlaceholderImage(Object(craft\elements\Asset), 1, Array)
#7 /var/www/site/vendor/nystudio107/craft-imageoptimize/src/services/OptimizedImages.php(550): nystudio107\imageoptimize\services\OptimizedImages->generatePlaceholders(Object(craft\elements\Asset), Object(nystudio107\imageoptimize\models\OptimizedImage), 1)
#8 /var/www/site/vendor/nystudio107/craft-imageoptimize/src/services/OptimizedImages.php(164): nystudio107\imageoptimize\services\OptimizedImages->addVariantImageToModel(Object(craft\elements\Asset), Object(nystudio107\imageoptimize\models\OptimizedImage), Object(craft\models\AssetTransform), Array, 1)
#9 /var/www/site/vendor/nystudio107/craft-imageoptimize/src/services/OptimizedImages.php(248): nystudio107\imageoptimize\services\OptimizedImages->populateOptimizedImageModel(Object(craft\elements\Asset), Array, Object(nystudio107\imageoptimize\models\OptimizedImage), false)
#10 /var/www/site/vendor/nystudio107/craft-imageoptimize/src/fields/OptimizedImages.php(223): nystudio107\imageoptimize\services\OptimizedImages->upd
ateOptimizedImageFieldData(Object(nystudio107\imageoptimize\fields\OptimizedImages), Object(craft\elements\Asset))
#11 /var/www/site/vendor/craftcms/cms/src/base/Element.php(3196): nystudio107\imageoptimize\fields\OptimizedImages->afterElementSave(Object(craft\elements\Asset), false)
#12 /var/www/site/vendor/craftcms/cms/src/elements/Asset.php(1841): craft\base\Element->afterSave(false)
#13 /var/www/site/vendor/craftcms/cms/src/services/Elements.php(2454): craft\elements\Asset->afterSave(false)
#14 /var/www/site/vendor/craftcms/cms/src/services/Elements.php(755): craft\services\Elements->_saveElementInternal(Object(craft\elements\Asset), true, false, true)
#15 /var/www/site/vendor/craftcms/cms/src/controllers/AssetsController.php(251): craft\services\Elements->saveElement(Object(craft\elements\Asset))
#16 [internal function]: craft\controllers\AssetsController->actionSaveAsset()
#17 /var/www/site/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array(Array, Array)
#18 /var/www/site/vendor/yiisoft/yii2/base/Controller.php(180): yii\base\InlineAction->runWithParams(Array)
#19 /var/www/site/vendor/craftcms/cms/src/web/Controller.php(190): yii\base\Controller->runAction('save-asset', Array)
#20 /var/www/site/vendor/yiisoft/yii2/base/Module.php(528): craft\web\Controller->runAction('save-asset', Array)
#21 /var/www/site/vendor/craftcms/cms/src/web/Application.php(274): yii\base\Module->runAction('assets/save-ass...', Array)
#22 /var/www/site/vendor/craftcms/cms/src/web/Application.php(577): craft\web\Application->runAction('assets/save-ass...', Array)
#23 /var/www/site/vendor/craftcms/cms/src/web/Application.php(253): craft\web\Application->_processActionRequest(Object(craft\web\Request))
#24 /var/www/site/vendor/yiisoft/yii2/base/Application.php(386): craft\web\Application->handleRequest(Object(craft\web\Request))
#25 /var/www/site/web/index.php(21): yii\base\Application->run()
#26 {main}
khalwat commented 3 years ago

@gbowne-quickbase I added a try/catch to cause the above to no longer throw an exception in: https://github.com/nystudio107/craft-imageoptimize/commit/ebd78565c3b0226150411d95e6ec3ba082f51a0c

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-imageoptimize": "dev-develop as 1.6.21",

Then do a composer update

I still suspect this is something environmental, because it shouldn't be throwing an error when trying to rasterize an SVG, but we can at least catch it.

gbowne-quickbase commented 3 years ago

@khalwat Before installing your update, If I turn off the optimize for that directory, it's fine. I do have SVG set to NOT transform, but that toggle doesn't seem to do anything. image I'm going to try your update now.

khalwat commented 3 years ago

Yeah it's not creating transforms there, it's creating the placeholder images. You can turn that off too, but you probably use them if you're lazy loading images.

gbowne-quickbase commented 3 years ago

@khalwat Your 1.6.21 definitely fixed it for me.

khalwat commented 3 years ago

Yeah I think it's not a fix so much as a paving over, in other words, it should be able to rasterize an SVG. Or maybe there is an issue with that particular SVG?

Either way, I think it's good that we're now doing a try/catch here but I'm just making you aware there may be underlying issues if you ever do need to rasterize an SVG.

gbowne-quickbase commented 3 years ago

That's all fair! Thanks for the quick asphalt patch on this.

andris-sevcenko commented 3 years ago

Just wanted to chime in on this and say that rasterizing an SVG can get really ugly, depending on the SVG. We've seen some SVGs get mangled beyond recognition when rasterizing with Imagick.

brandonkelly commented 3 years ago

@khalwat Thanks for fixing!