U4-7223 - GetCropUrl outputs html entities in url when used in css

Created by Rune Strand 12 Oct 2015, 07:24:41 Updated by Shannon Deminick 02 Dec 2016, 00:27:30

'''''What did you do?''''' I used the image cropper to create css background images:

<style>
    .mobile {
         background-image: url(@Model.Content.GetCropUrl("image", "mobile"));
    }
    .desktop {
        background-image: url(@Model.Content.GetCropUrl("image", "desktop"));
    }
</style>

'''''What did you expect to happen?''''' I expected the url to be: /media/1001/boat.jpeg?crop=0.27083333333333343,0.0000000000000001263187085796,0,0&cropmode=percentage&width=1000&height=500&rnd=130891115360000000

'''''What actually happened?''''' Ampersands where rendered as html entities so & turned into &amp; resulting in this url: /media/1001/boat.jpeg?crop=0.27083333333333343,0.0000000000000001263187085796,0,0&amp;cropmode=percentage&amp;width=1000&amp;height=500&amp;rnd=130891115360000000

If you hit url directly you get a ysod (see attached image).

Got it working by using Html.Raw() so:

<style>
    .mobile {
         background-image: url(@Html.Raw(Model.Content.GetCropUrl("image", "mobile")));
    }
    .desktop {
        background-image: url(@Html.Raw(Model.Content.GetCropUrl("image", "desktop")));
    }
</style>

This was not a problem prior to 7.3 so I'm guessing something has changed in Image Processor. If I use an older version of Image Processor (DLL's from 7.2.8) this does not occur.

1 Attachments

Comments

James South 12 Oct 2015, 10:04:30

Are you sure that the urls are not always generated with encoded ampersands?

ImageProcessor doesn't generate those urls, that's all Umbraco.

What you're seeing is a difference in the way urls are parsed between the old v3 version of ImageProcessor.Web and the v4 version.

In v3 and below I was incorrectly url decoding the full url when parsing querystring values. This led to all sorts of issues with urls for images from 3rd party websites like Facebook and also led to issues with the watermarking functionality and some of the numeric parsers. I have a much better parsing engine running now that correctly handles all these types and leaves the decoding to ASP.

The reason the code you had worked before was a side-effect of my previously buggy code allowing it. You should be generating your urls without HTML encoding the ampersands.

Hope that helps.


Rune Strand 12 Oct 2015, 10:12:48

Thanks for replying so fast.

You are absolutely right. Urls are generated in the same way in older versions (7.2.8 anyway). Guess the Image Cropper needs a service check to play nicely with Image Processor again.


Rune Strand 12 Oct 2015, 10:14:37

We need to address this as sites will break (or at the very least be missing images) // @pploug @Shandem


Claus Jensen 22 Oct 2015, 11:55:37

I have investigated this a bit and it is actually working as intended. By default the razor view engine will (and should) url-encode every string it outputs unless you specifically ask it not to by wrapping in @Html.Raw().

I tested this behavior in a 7.2.8 just to confirm and can ensure you it works just the same way (razor has done this for as far as I remember).

There's two ways of getting around this - one is: we output a HtmlString instead of a plain string (HtmlStrings will not be encoded). This would make the above work - but it is unfortunately a method signature change which we consider a breaking change.

The other option is to enhance the HtmlHelper to be able to do GetCropUrl() instead - which is the fix I've submitted here.

So instead you would do this to have it output correctly:

<style>
    .mobile {
         background-image: url(@Html.GetCropUrl(Model.Content, "image", "mobile"));
    }
    .desktop {
        background-image: url(@Html.GetCropUrl(Model.Content, "image", "desktop"));
    }
</style>

PR for this fix: https://github.com/umbraco/Umbraco-CMS/pull/832


Rune Strand 23 Oct 2015, 06:23:08

@Claus.Jensen So is this the way to use the image cropper going forward (as in should we get docs updated to reflect this)? Is it only for this specific scenario or can you still use the old way to use GetCropUrl or is it depending on where you would use it?

And just to clarify, upgrading to 7.3.1 wouldn't actually solve the problem, right? You would still need to go in and edit templates to use @Html.GetCropUrl().


Claus Jensen 23 Oct 2015, 06:55:46

@rune Yep - I don't really see how this would ever have worked in the past since this is not really something we have changed. It is a part of the razor view engine that if you output something as plain text through a razor helper method it will encode it with the &amp;-thing (I guess it's a security thing to prevent accidentally allowing injection of scripts).

Do you have a link to our documentation where this is used?

And yep - we can't make a fix that will just "solve" the problem since that would require us to change what is returned from GetCropUrl() to a HtmlString instead of a plain text string. It would technically work for this specific case since razor views are compiled runtime, but we could potentially be breaking functionality in sites expecting that GetCropUrl() returns a string and not a HtmlString.


Rune Strand 23 Oct 2015, 07:13:22

@Claus.Jensen Well, my initial bug report might have been a bit off.

You are right, GetCropUrl() has always returned the string with &amp;. Thing is, this used to work because we used an older version of Image Processor prior to 7.3 (See James Souths reply above). So it was a perfect storm of dodgy output and really forgiving parsing but it worked.

Documentation for the [https://our.umbraco.org/documentation/Getting-Started/Backoffice/Property-Editors/Built-in-Property-Editors/Image-Cropper, Image Cropper]. I'll be happy to update the docs, just need to know exactly what's what :)


Shannon Deminick 05 Jan 2016, 13:56:37

Re-fixing this issue in 7.3.5 with UrlHelper extensions, obsoleting the HtmlHelper extensions, see here: https://github.com/umbraco/UmbracoDocs/pull/262#issuecomment-169001480


Alejandro Ocampo 23 Nov 2016, 13:05:11

@Shandem Any reason why this extension is not implemented in the ImageCropperTemplateExtensions? So instead of only use @Url.GetCropUrl(mediaItem, width:300, htmlEncode:false) when can also use @mediaItem.GetCropUrl("CropAlias", htmlEncode:false).


Shannon Deminick 02 Dec 2016, 00:27:30

All GetCropUrl extension methods are on the UrlHelper where they are supposed to be. All GetCropUrl extension methods that exist on the model or html helper are obsoleted and should not be used


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty:

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.3.0

Due in version: 7.3.1, 7.3.5

Sprint: Sprint 1

Story Points:

Cycle: