Allow headers to be provided to fetch_url

[brendon]

Closes #498

Allows optional headers to be passed to the request. The interface for headers is a bit weird so we need to loop the passed in headers and then add each one using a hash-like [] interface. It seems they can't be all added at once.

[brendon]

Hi @markevans, would you mind looking at this PR? Let me know if you think it needs further improvement.

[brendon]

Hi @markevans, I'm upgrading the app where I use a fork of dragonfly with this PR applied. I notice you're still actively maintaining the gem so I wondered if you'd consider accepting this PR for inclusion? I'm happy to make any modifications you think necessary.

[brendon]

Hi @markevans, is this something that we could still look to add?

[markevans]

Hi - sorry for the unbelievably slow response on this! 😬

Looks good in principle, however, I don't think we can include this because of the following reason (sorry I didn't think about this before)...

The job steps, including the arguments, get encoded into the url, so for example

job = Dragonfly.app.fetch_url("https://example.com/image")
job.url # "/W1siZnUiLCJodHRwczovL2V4YW1wbGUuY29tL2ltYWdlIl1d/image?sha=033ecdd26d3968ad"

job = Dragonfly.app.fetch_url("https://example.com/image", headers: {"authorization"=> "Bearer abc123"})
job.url # "/W1siZnUiLCJodHRwczovL2V4YW1wbGUuY29tL2ltYWdlIix7ImhlYWRlcnMiOnsiQXV0aG9yaXphdGlvbiI6IkJlYXJlciBhYmMxMjMifX1dXQ/image?sha=4cf8250ff0fa8974"

(note the longer url).

As you can see the url encodes the auth details - this can even be decoded:

Dragonfly::Serializer.json_b64_decode("W1siZnUiLCJodHRwczovL2V4YW1wbGUuY29tL2ltYWdlIix7ImhlYWRlcnMiOnsiQXV0aG9yaXphdGlvbiI6IkJlYXJlciBhYmMxMjMifX1dXQ") # ==> [["fu", "https://example.com/image", {"headers"=>{"Authorization"=>"Bearer abc123"}}]]

For your app a better idea would be to retrieve the image yourself using whichever http library, then if assigning to an activerecord model do it as per https://markevans.github.io/dragonfly/models#using-the-accessors.

To be honest if I were to start again I may not have even included fetch_url - it's probably better to require the user to save an image locally themselves rather than encoding fetch details in the url.

Apologies I didn't really have time to think this through previously

[brendon]

Hi @markevans, absolutely not a problem! :D I'm a gem maintainer myself and know how real paying works takes priority :)

I didn't realise it worked this way. I assume the fetch only happens once at the time the record is assigned and then Dragonfly stores its own copy of the image? This is my use case (ActiveRecord):

module GoogleDriveConcern

  extend ActiveSupport::Concern

  GOOGLE_DRIVE_FILE_URL = 'https://www.googleapis.com/drive/v3/files'

  included do
    before_validation :set_file_from_google_drive, if: -> { google_drive_file_id.present? }
  end

  private

  def set_file_from_google_drive
    self.file = Dragonfly.app.fetch_url "#{GOOGLE_DRIVE_FILE_URL}/#{google_drive_file_id}?alt=media",
      headers: { 'Authorization': "Bearer #{google_drive_oauth_token}" }
    self.file.name = google_drive_file_name
  end

end

The google_drive_... attributes are just in-memory attributes on the model:

  attribute :google_drive_file_id, :string
  attribute :google_drive_file_name, :string
  attribute :google_drive_oauth_token, :string

Is the concern just that someone with access to the job queue could acquire the token?

As you say though, it's easy enough to fetch the file using Faraday or something like that instead and sidestep all of this.

[markevans]

In your case, yes the fetch is only happening when assigned and Dragonfly is storing its own copy (the line self.file = ...) so you don't have to worry about leaking auth details.

But Dragonfly provides extra functionality such that fetch_url can generate its own url, e.g.

url = Dragonfly.app.fetch_url("https://some.url/image").url # --> "/adsfkladfk..."

If you were to visit this url Dragonfly would fetch that url and serve it (you can chain on processing steps etc.). You're not using this functionality and 99% of people probably won't either, but it needs to be taken into account.

However it's not really in Dragonfly's remit to provide extra HTTP functionality beyond v. basic as this can easily be done with Faraday or something as you mentioned.

I think the best bet would probably be to replace the line

self.file = Dragonfly.app.fetch_url(...

with

self.file = SomeLibraryLikeFaraday.get("...

Hope that makes sense

[brendon]

Thanks @markevans, that definitely makes sense. I suspected it would be something like that. Thanks for the detailed explainer :) Above and beyond!

All the best :D