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

Upload endpoint only uploading to 'cache' folder #77

Closed
imfaisalkh opened this issue Aug 17, 2016 · 6 comments
Closed

Upload endpoint only uploading to 'cache' folder #77

imfaisalkh opened this issue Aug 17, 2016 · 6 comments

Comments

@imfaisalkh
Copy link

imfaisalkh commented Aug 17, 2016

Hey again!

I was able to resolve the previous issue (detail posted there). Now, with the given upload endpoint /attachments/images/cache/upload any uploaded file returns a 200 status in the inspector but when I check my s3 bucket it is only uploaded in the cache folder and store folder is empty.

Plus, since I'm trying to implement fineuploader library, it requires a specific key in the returned JSON response. Currently, I'm getting this response:

{"id":"4a4191c6c43f54c0a1eb2cf482fb3543.PNG","storage":"cache","metadata":{"filename":"IMG_0105.PNG","size":114333,"mime_type":"image/png","width":640,"height":1136}}

Whereas, the fineuploader expects the property{"success":true} in the response, how can I add this value in the response?

I'm not sure if these issues are related to shrine gem or aws, since I'm receiving a different kind of 200 response with this example's upload method (which utilize jquery-file-upload). However if it is not related to this gem please let me know.

@janko
Copy link
Member

janko commented Aug 17, 2016

Now, with the given upload endpoint /attachments/images/cache/upload any uploaded file returns a 200 status in the inspector but when I check my s3 bucket it is only uploaded in the cache folder and store folder is empty.

Yes, this is for security. If the direct upload endpoint could upload directly to your main storage (store), an attacker could upload an arbitrary number of orphan files to your main storage. And on storages like S3 it's difficult/slow to find and delete orphan files. But you can allow uploading directly to store if you still want to, check the direct_upload plugin docs.

The response of the uploaded cached file should be sent as the attachment (e.g. assigned to hidden attachment field), and then when it's attached to the record it will be uploaded to permanent storage.

Whereas, the fineuploader expects the property{"success":true} in the response, how can I add this value in the response?

I think the best way is to create a Rack middleware that modifies the response, and attach it to the UploadEndpoint (probably best to do it in the initializer):

Shrine.plugin :direct_upload

class FineUploaderResponse
  def initialize(app)
    @app = app
  end

  def call(env)
    status, headers, body = @app.call(env)

    if status == 200
      data = JSON.parse(body[0])
      data["success"] = true
      body[0] = data.to_json
    end

    [status, headers, body]
  end
end

Shrine::UploadEndpoint.use FineUploaderResponse

I'm not sure if these issues are related to shrine gem or aws

If you are using S3 for storage, you could also explore the possibility of uploading directly to S3. FineUploader has built-in support for that, and it shouldn't be difficult to hook it to Shrine's presign route.

@janko janko closed this as completed Aug 17, 2016
@janko
Copy link
Member

janko commented Aug 17, 2016

Note that for usage questions it's much better to use the Shrine Google group, and use GitHub issues only for bugs 😉

@imfaisalkh
Copy link
Author

Thanks for your answer.

The response of the uploaded cached file should be sent as the attachment (e.g. assigned to hidden attachment field), and then when it's attached to the record it will be uploaded to permanent storage.

Now, I got it working exactly as you described here.

I think the best way is to create a Rack middleware that modifies the response, and attach it to the UploadEndpoint (probably best to do it in the initializer):

The code you provided seems to corrupt the JSON response somehow. Nevertheless, I've opened up a ticket on stackoverflow:

Note that for usage questions it's much better to use the Shrine Google group, and use GitHub issues only for bugs 😉

I was not aware of this group and will look into it definitey. Thanks again.

@openscript
Copy link

I faced exact the same problem. After we changed the body of the response, the Content-Length in the header needs to be updated or the browser will cut the body off. This looks like this then:

class FineUploaderResponse
  def initialize(app)
    @app = app
  end

  def call(env)
    status, headers, body = @app.call(env)

    if status == 200
      data = JSON.parse(body[0])
      data['success'] = 'true'
      body[0] = data.to_json
      headers['Content-Length'] = body[0].length
    end

    [status, headers, body]
  end
end

Shrine::UploadEndpoint.use FineUploaderResponse

@janko
Copy link
Member

janko commented Aug 19, 2016

@openscript Aaah, thanks a lot! For some reason I assumed that the Rack::ContentLength middleware will take care of that automatically, but Roda already assigns the Content-Length before it gets to that Rack::ContentLength, which won't change the Content-Length if it was already set.

@janko
Copy link
Member

janko commented Dec 23, 2016

Just noting here for future readers that Content-Length should by updated with String#bytesize instead of String#length, so that responses with non-ASCII characters aren't trimmed down by the browser.

class FineUploaderResponse
  def initialize(app)
    @app = app
  end

  def call(env)
    status, headers, body = @app.call(env)

    if status == 200
      data = JSON.parse(body[0])
      data['success'] = 'true'
      body[0] = data.to_json
      headers['Content-Length'] = body[0].bytesize # <===
    end

    [status, headers, body]
  end
end

Shrine::UploadEndpoint.use FineUploaderResponse

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

No branches or pull requests

3 participants