Skip to content
New issue

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

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

Already on GitHub? # to your account

encode_path on url #3

Open
allaire opened this issue May 28, 2017 · 3 comments · May be fixed by #5
Open

encode_path on url #3

allaire opened this issue May 28, 2017 · 3 comments · May be fixed by #5

Comments

@allaire
Copy link

allaire commented May 28, 2017

Hi, I had to monkey path to add encode_path to the url method, like this:

module AzureEncodingPatch
  include CarrierWave::Utilities::Uri

  def url(options = {})
    path = ::File.join @uploader.azure_container, encode_path(@path)
    if @uploader.asset_host
      "#{@uploader.asset_host}/#{path}"
    else
      uri = @connection.generate_uri(path)
      if sign_url?(options)
        @signer.signed_uri(uri, false, { permissions: 'r',
                                         resource: 'b',
                                         start: 1.minute.ago.utc.iso8601,
                                         expiry: expires_at}).to_s
      else
        uri.to_s
      end
    end
  end
end

Otherwise, images with spaces in it were failing. Should I create a PR?

@allaire
Copy link
Author

allaire commented May 28, 2017

@krismichalski URI.encode could also be used I suppose!

From what I see, it seems to be pretty standard to use the encoded_path, at least in the fog storage:

https://github.com/carrierwaveuploader/carrierwave/blob/540a84c606353a878d6574740e44f7970b754463/lib/carrierwave/storage/fog.rb#L335

@krismichalski
Copy link
Owner

Hi @allaire, great catch!
Yes, if you could create PR with encode_path patch it would be great.
It would be even more awesome if you could add a test case for files with spaces in them.
Unfortunately, I no longer have access to Azure, so I cannot develop it by myself.

@allaire
Copy link
Author

allaire commented Jun 5, 2017

@krismichalski Good, will try to get this done soonish!

@jmadureira jmadureira linked a pull request Apr 22, 2020 that will close this issue
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants