CarrierWave-style upload from URL about refile HOT 9 CLOSED

Hello.
I could help with this. But could you describe desired API?

If i would have Rails model:

class User < ActiveRecord::Base
  attachment :profile_image
end

Would there be accessor remote_profile_image_url as well?

from refile.

Yes, this was kind of what I was thinking. What do you think about the name? The CarrierWave version is decent, right?

One note about this is that it should follow redirects if URL serves a 302 or similar. I can't remember whether CarrierWave does, but I definitely want that to work. IIRC you'll need to do this manually with Net::HTTP, maybe open-uri does it automatically.

from refile.

Yes, the naming is ok. And Refile.configure should allow to set number how many times script follows redirects and timeout in secods for the whole operation.

from refile.

Number of redirects probably doesn't need to be configurable. It's hard-coded to around 20-25 in all browsers, the HTTP spec originally recommended 5, but I think it was changed to "unspecified" in 1.1.

from refile.

since we're going into Refile.configure may want to add the option for http.verify_mode = OpenSSL::SSL::VERIFY_NONE when the URL is https

from refile.

@choonkeat we shouldn't complicate refile with SSL related stuff.
I would try to simplify even more and make the direct setter (.image=) accept many stuff:

• strings that URI.parse accepts
• objects that responds to read (IOStream, File)
• direct strings that has the item's content

For unverified SSL content you could always do:

  object.image = open(some_url, {ssl_verify_mode: OpenSSL::SSL::VERIFY_NONE})

from refile.

Oohh, that's tricky.

I just took a look at how CarrierWave does this, and it seems that it uses open-uri with no option of configuring SSL.

I agree with @cristianbica that we want to keep those kinds of concerns away from Refile, but at the same time I can see the need for having that be configurable. I disagree that we should override image= to accept all kinds of different parameters and then try to use heuristics to figure out if it's a URL or not, that seems very fragile and potentially insecure to me.

As @cristianbica points out, you can always manually download the file and assign it, but maybe we should just decide that if you want the convenience of this feature, we should just decide that the default SSL options are it. VERIFY_NONE seems like an anti-pattern to me anyway.

from refile.

Yeah image= seems bad to me now :) I always felt unsecure with carrierwave or paperclip was what can I pass to image=. It would be great if we document very well this part. 

Example:

image= takes an IO subclass (something that responds to  .read)

image_url= takes an URL passed to open-uri. We could pass to open-uri all the params flattened:

def image_url=(*args)

   open(*args.flatten)

end

So we can solve the ssl stuff with

image_url = [url, openurioptions]

image_string= method that takes the attachment as a string

Cristian Bica

On Fri, Dec 12, 2014 at 7:01 PM, Jonas Nicklas [email protected]
wrote:

Oohh, that's tricky.
I just took a look at how CarrierWave does this, and it seems that it uses open-uri with no option of configuring SSL.
I agree with @cristianbica that we want to keep those kinds of concerns away from Refile, but at the same time I can see the need for having that be configurable. I disagree that we should override image= to accept all kinds of different parameters and then try to use heuristics to figure out if it's a URL or not, that seems very fragile and potentially insecure to me.

As @cristianbica points out, you can always manually download the file and assign it, but maybe we should just decide that if you want the convenience of this feature, we should just decide that the default SSL options are it. VERIFY_NONE seems like an anti-pattern to me anyway.

Reply to this email directly or view it on GitHub:
#13 (comment)

from refile.

I merged @krists implementation of this, which doesn't include configuration for timeouts or SSL, but I think that's better for now.

from refile.