Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
5 of 10 tasks
brendanmatkin opened this issue Feb 15, 2024 · 9 comments · Fixed by #548
Closed
5 of 10 tasks

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

brendanmatkin opened this issue Feb 15, 2024 · 9 comments · Fixed by #548

Comments

@brendanmatkin
Copy link

Bug report for Cloudinary Ruby SDK

  • Before proceeding, please update to latest version and test if the issue persists

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)

  • Build - Cannot install or import the SDK
  • Performance - Performance issues
  • Behaviour - Functions are not working as expected (such as generate URL)
  • Documentation - Inconsistency between the docs and behaviour
  • Other (Specify)

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

  • Linux
  • Windows
  • macOS
  • All
    (probably all but only tested with these two)

Environment and Libraries (fill in the version numbers)

  • Cloudinary Ruby SDK version - 1.28.0
  • Ruby Version - 3.1.2p20
  • Rails Version - 7.0.7.2
  • ActiveStorage - 7.0.7.2
@wissam-khalili
Copy link

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
Copy link
Author

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
Copy link
Contributor

@brendanmatkin there are actually 2 cloudinary_url methods:
One is here:

def cloudinary_url(source, options = {})
(which most likely you are calling)

And another one is here:

def self.cloudinary_url(public_id, options = {})

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
Copy link
Author

@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
Copy link
Author

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
Copy link

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
Copy link
Author

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

@wissam-khalili
Copy link

Hi @brendanmatkin ,

Please note that this issue was fixed.
Thanks,Wissam

@brendanmatkin
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants