cloudinary / cloudinary_gem

Cloudinary GEM for Ruby on Rails integration
https://cloudinary.com
420 stars 285 forks source link

Spaces in Font's name are not escaped #458

Open tejanium opened 3 years ago

tejanium commented 3 years ago

Cloudinary supports all of Google's fonts https://support.cloudinary.com/hc/en-us/articles/203352832-What-is-the-list-of-supported-fonts-for-text-overlay-transformation- and some of them have spaces on them, for example, "Open Sans"

But unlike Text Transformation where spaces are escaped https://github.com/cloudinary/cloudinary_gem/blob/28a9ee65d24c6bac0654919a5a76de95cf356510/lib/cloudinary/utils.rb#L399 font family aren't https://github.com/cloudinary/cloudinary_gem/blob/28a9ee65d24c6bac0654919a5a76de95cf356510/lib/cloudinary/utils.rb#L452

resulting in an invalid URL

[1] pry(main)> Cloudinary::VERSION
=> "1.20.0"
[2] pry(main)> url = Cloudinary::Utils.cloudinary_url("coffee_cup.jpg", transformation: [
  {overlay: {font_family: "Open Sans", text: "Text with spaces", font_size: 10}}
])
=> "http://res.cloudinary.com/cookpad/image/upload/l_text:Open Sans_10:Text%20with%20spaces/coffee_cup.jpg"
[3] pry(main)> URI.parse url
URI::InvalidURIError: bad URI(is not URI?): "http://res.cloudinary.com/cookpad/image/upload/l_text:Open Sans_10:Text%20with%20spaces/coffee_cup.jpg"
tommyg-cld commented 1 year ago

@tejanium firstly apologies for the delay in getting back here, this got lost in our backlog.

Regarding the customer font, you can just URL encode the space before making the request and it will work e.g. https://res.cloudinary.com/cookpad/image/upload/l_text:Open%20Sans_60:Text%20with%20spaces/sample.jpg

let me know if resolves your issue?

tejanium commented 1 year ago

Thank you for getting back to me

Regarding the customer font, you can just URL encode the space before making the request and it will work e.g. res.cloudinary.com/cookpad/image/upload/l_text:Open%20Sans_60:Text%20with%20spaces/sample.jpg

Yes, this is what we ended up doing. But I think this should be part of the library.

tommyg-cld commented 1 year ago

@tejanium no problem and fair point, let me raise it internally and hopefully we will have a fix soon.

tommyg-cld commented 1 year ago

also as a workaround, for now, you could escape the space when it comes to fonts with spaces like font_family: "Open%20Sans"

tejanium commented 1 year ago

Yes, that's what we did

        def safe_font_family
          ERB::Util.url_encode(font_family)
        end
tommyg-cld commented 1 year ago

so this is in our list to fix but no ETA currently but will keep you posted.