cloudinary / cloudinary_gem

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

Invalid Remote Fetch URL if `folder` is set in Storage Service Config #537

Closed brendanmatkin closed 4 months ago

brendanmatkin commented 6 months ago

Bug report for Cloudinary Ruby SDK

Describe the bug in a sentence or two.

For a remote fetch, cloudinary_url and cl_image_path output URL has version v1 and <folder> prepended to the publicID, if folder is set in storage.yml. If no folder is set, output URL works as expected.

It's possible that there is an option I can't figure out that helps with this? If so, maybe a documentation issue?

Issue Type (Can be multiple)

Steps to reproduce

  1. Add a folder to cloudinary storage service config (config/storage.yml)
    cloudinary:
      service: Cloudinary
      folder: <%= Rails.env %> #e.g. "development"

    where config/environment/<env>.rb has line:

    config.active_storage.service = :cloudinary
  2. fetch a remote url
    image_url = cl_image_path(<REMOTE_URL>, type: "fetch", transformation: [{ fetch_format: :auto }])

Error screenshots or Stack Trace (if applicable)

puts image_url
# output (INVALID):
# https://res.cloudinary.com/<my-cloud-name>/image/fetch/f_auto/v1/development/https://<REMOTE_URL>.jpeg
#                                                              ^^^^^^^^^^^^^^^

# expected (VALID) - which you'll get if you remove `folder` from storage config: 
# https://res.cloudinary.com/<my-cloud-name>/image/fetch/f_auto/https://<REMOTE_URL>.jpeg

Operating System

Environment and Libraries (fill in the version numbers)

wissam-khalili commented 5 months ago

Hi @brendanmatkin,

The cl_image_tag method described above generates an HTML image tag. In certain conditions, you might want to generate a transformation URL directly, without the containing image tag. To return only the URL, either use the cl_image_path or cloudinary_url view helper methods, or use the Ruby command: Cloudinary::Utils.cloudinary_url. You read about it here: https://cloudinary.com/documentation/rails_image_manipulation#direct_url_building

I hope you find it useful.

Regards, Wissam

brendanmatkin commented 5 months ago

Thanks @wissam-khalili.

My above issue is true for those commands (cl_image_path and cloudinary_url) as well. None of those build remote fetch URLs correctly.

The Storage Service Config folder should not be included those URL builders for remote fetch (cloudinary does not expect the folder in remote fetch URL, because remote fetches don't have folders).

The only way to make it work is to build the url manually. The bug still exists though even though the workaround is documented.

Here is my manual URL builder if anyone else has the same problem (obviously the transformations are specific to my use case and URL signing is enabled).

def cloudinary_url_manual(url, width, height)
  transformation = "c_lfill,h_#{height},w_#{width}/q_auto/f_auto"
  public_id = escape(url) # can prepend with folder if using
  to_sign = [transformation, public_id].join("/")
  secret = Rails.application.credentials.dig(:cloudinary, :api_secret)
  signature = "s--#{Base64.urlsafe_encode64(Digest::SHA1.digest(to_sign + secret))[0, 8]}--"
  cloud_name = Rails.application.credentials.dig(:cloudinary, :cloud_name)
  "https://res.cloudinary.com/#{cloud_name}/image/fetch/#{[signature, to_sign].join('/')}"
end

def escape(url)
  url.gsub("?", "%3F").gsub("&", "%26").gsub("=", "%3D")
end
const-cloudinary commented 5 months ago

@brendanmatkin there are actually 2 cloudinary_url methods: One is here: https://github.com/cloudinary/cloudinary_gem/blob/088330b0fa3069c8e6b696e69b7191116aa3851c/lib/cloudinary/helper.rb#L235 (which most likely you are calling)

And another one is here: https://github.com/cloudinary/cloudinary_gem/blob/088330b0fa3069c8e6b696e69b7191116aa3851c/lib/cloudinary/utils.rb#L882

The latter one should be good for building fetch URLs. And you can call it like: Cloudinary::Utils.cloudinary_url(<REMOTE_URL>, type: "fetch", transformation: [{ fetch_format: :auto }])

brendanmatkin commented 5 months ago

@const-cloudinary ah ok! I'm still getting the hang of Ruby/Rails, and I'm not sure if I realized I could use the cloudinary_url Ruby utility command in a view. I think I was thinking that the first was only for views and the second was only for controllers (I'm using this in views).

I THINK we tried the second one at some point, but I can't be sure (it might've been under different circumstances - there are 2 apps that use Cloudinary remote fetch). Thanks for clarifying either way! I will try this again soon - hopefully that was my issue.

brendanmatkin commented 5 months ago

p.s. @wissam-khalili thanks also for your help - I misinterpreted that you were showing me 2 different cloudinary_url methods.

Along these same lines, should the non-utility method still be able to work with remote fetch? It seems to take the option, but it definitely doesn't work. If so, this could still be a valid bug, right?

wissam-khalili commented 5 months ago

Hi @brendanmatkin,

Thank you for your feedback. You are right, it's a bug and we will fix it. We will keep you posted.

Thanks, Wissam

brendanmatkin commented 5 months ago

Ok sounds good! Thanks for the engagement and help so far.

wissam-khalili commented 4 months ago

Hi @brendanmatkin ,

Please note that this issue was fixed. Thanks,Wissam

brendanmatkin commented 4 months ago

@wissam-khalili Nice, thanks for updating me!! Appreciate y'all getting on this obscure bug.