Upload endpoint with authorization


Was trying to add authorization to the upload endpoint, following the controller here https://shrinerb.com/docs/plugins/upload_endpoint, but it seems not to be working… I’m using rails and shrine 3.2

If i add the following endpoint on routes.rb it works ok.
mount Shrine.upload_endpoint(:cache) => "/upload"

Nevetherless if i add a custom controller with authorization like this (with the same upload on the client)
match '/upload' => 'images#upload', via: :all
I’m getting the message “Upload Not Valid”

This is my controller

class ImagesController < ApplicationController
  def upload
    set_rack_response ImageUploader.upload_response(:cache, request.env)

    def set_rack_response((status, headers, body))
      self.status = status
      self.response_body = body

Checking the method “get_multipart_upload” on upload_endpoint.rb it seems there is a validation where value needs to be a Hash. But the file is a hash only if it’s directly upload with the rack, when using rails this is a “ActionDispatch::Http::UploadedFile” object.

Could this be a bug or i’m using it in the wrong why?

Hi, this does seem like a bug in Shrine. I didn’t expect Rails to convert the Rack uploaded file hash into ActionDispatch::Http::UploadedFile on the Rack level, this will then need to be handled in upload_endpoint.

Would you like to send a PR to fix this? Otherwise I should be able find time for it sometime next week.

Hello! No problem, i thought the conversion was expected as in the rack_file plugin there was this note

Note that this plugin is not needed in Rails applications, as Rails already wraps the Rack uploaded file hash into an ActionDispatch::Http::UploadedFile object

I’m not very good with git and PRs but saw that it works if i replace these lines

  error!(400, "Upload Not Valid") unless value.is_a?(Hash) && value[:tempfile]


with these

   if value.is_a?(Hash) && value[:tempfile]
	return @shrine_class.rack_file(value)
   elsif value.respond_to?(:read) && value.respond_to?(:rewind)  && value.respond_to?(:eof?) && value.respond_to?(:close)
    	return value
   	error!(400, "Upload Not Valid")

As IO abstraction section mention that those are the methods to implement for a IO

But i’m not sure if it’s the right thing to do. Maybe I’m missing something here?

This change looks good to me :+1:

I like that the conditional is using duck-typing, as that will also work with other web frameworks who might implement an ActionDispatch::Http::UploadedFile equivalent. It also means you can use a different IO object in tests, though I don’t know yet how to pull it through into the params :thinking:

Feel free to send the PR, and if you’re stuck with tests, I can help out :wink:

Ok great! i’ll prepare the PR and let you know :slight_smile: