multipart/form-data about angelo HOT 13 OPEN

@pachacamac hello! last summer, i was able to help a google summer of code student through adding rudimentary support for both sessions and multipart to reel. it's not merged yet, and still a bit rough, but you can try it:

Gemfile:

gem 'reel', github: 'pulkit4tech/reel', branch: 'gsoc16'

Angelo app class file:

require 'reel/request/multipart'

# ...

  post '/' do
    halt 400 unless request.multipart?
    request.multipart.each do |filename, mp|
      puts "received file: '#{filename}'"
      puts "#{mp[:data].size} bytes, #{mp[:data].path}"
    end
    redirect '/'
  end

from angelo.

@Adam-Stomski I looked through reel, sinatra, and rack code... Looks like rack handles this, merging a tempfile into the params in this case. I'm now confused as to whether this type of thing should be in reel or angelo (though I think it should definitely be in one). I'm guessing that it "works" if using reel-rack. @tarcieri @digitalextremist thoughts?

from angelo.

Putting it in reel would mean people wouldn't have to re-invent it. But I
haven't actually looked into the reel API so I don't know how much sense
that makes.

On Thu, Feb 19, 2015 at 11:14 AM, kenichi nakamura <[email protected]

wrote:

@Adam-Stomski https://github.com/Adam-Stomski I looked through reel,
sinatra, and rack code... Looks like rack handles this, merging a tempfile
into the params in this case. I'm now confused as to whether this type of
thing should be in reel or angelo (though I think it should definitely be
in one). I'm guessing that it "works" if using reel-rack. @tarcieri
https://github.com/tarcieri @_de thoughts?

—
Reply to this email directly or view it on GitHub
#47 (comment).

from angelo.

@tommay I thought I had looked at it quite a bit, but realized that I completely missed the Reel::StreamResponse and Reel::EventStream classes when added SSE and chunked support to Angelo. Also, reel doesn't do params, so I'm wondering what would even be the approach there re: rack-style params[:file] => Tempfile

from angelo.

I could go either way on this one. Reel is trying to provide a lightweight, below-Rack level abstraction, but if you really think it makes sense to add it to Reel I'd be ok with that.

from angelo.

This is something I have code for, per celluloid/reel#149 ... @Adam-Stomski I believe mulitipart form-data ought to be in Reel ... because no, it does not work properly with Rack either. That is what originally got me started modifying Reel a very long time ago - no multipart form-data support. I will work on the referenced ticket and have it reviewed to make sure it behaves the way we're discussing.

One thing I did notice though, is that form-data is always parsed in Rack which means non-multipart requests get an ever-so-slight performance dip, whereas in how I've used multipart with Reel so far, I believe I treat it more like a WebSocket is, which is to grab the data if it is expected to be multipart at an endpoint level, not in the request handler itself.

from angelo.

FWIW, @ixti is working on this sort of thing in https://github.com/httprb/form_data.rb

from angelo.

@tarcieri I remember you mentioning that, and I checked it out. The server-side handling of multipart requests seems undocumented. The gem seems geared toward acting as a client posting requests right now, but I'm sure when I did further I'll find a parser. This is the one I've been using myself:

https://github.com/danabr/multipart-parser

If @ixti's gem is going to be more active, I'll switch over to that myself also, and in fact remove my multipart form-data bolt on, and pull this into Reel in a way @kenichi can see works with angelo.

from angelo.

yeah, i don't see any server-side parsing in that http-form_data gem... what am i missing?

from angelo.

Nonexistent at the moment, but potentially something @ixti might be interested in adding

from angelo.

Maybe I'll work with @ixti to add support for parsing on the server-side.

@kenichi, do you have specific hopes for multipart support for your DSL? I also have a Reel-based DSL, and I'd rather not custom tailor the implementation of multipart to mine... or Sinatra/Rack either.

Question for @kenichi and @tarcieri: is it acceptable to use a "hijack" style handling of multipart form-data, in this sense -- when we hijack a websocket in the Rack approach to websockets, we're at a certain endpoint and anticipate a certain behavior/state of the browser. We're never touching every single HTTP request to make it compatible with websocket requests, and the same ought to be true of multipart requests with form-data ... agreed? To me, this question alone answers enough to build requirements for Reel handling its own multipart form-data, regardless of what parser we use.

from angelo.

@digitalextremist i have no specific hopes for multipart support in angelo; however, i do agree now that it should be handled in reel, so our respective DSLs can build off that. As for reel-rack support, if it doesn't work now, I see no problem in it continuing to not work :) but i think maybe an off-switch that reel-rack can set so rack can attempt to do it's thing may be a good idea.

my thoughts, as not fully formed as they are, lean towards some methods added to Reel::Request like in your issue. request.multipart? seems a great start to me. i'd like to do more research into the way rack does this with the tempfile and everything - see if there are issues with that approach that we can avoid. i'd also be super interested in @tenderlove's thoughts on how the_metal / rack-2 is going to approach this.

from angelo.

So there is still no solution? Any idea/best practice on how to handle file uploads with angelo?

from angelo.