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

multipart/form-data #47

Open
Adam-Stomski opened this issue Feb 13, 2015 · 13 comments
Open

multipart/form-data #47

Adam-Stomski opened this issue Feb 13, 2015 · 13 comments

Comments

@Adam-Stomski
Copy link

Does angelo support file uploads? Im trying to send file from Rails apllication to angelo server with form like

<form method="POST" enctype="multipart/form-data" action="http://127.0.0.1:4567/images">
  <div>
    <input name="image" type="file" />
  </div>
  <div>
    <input type="submit" value="Submit" />
  </div>
</form>

server code:

class SampleServer < Angelo::Base
  post '/images' do
    puts request.inspect
    puts params
  end
end

and log:

I, [2015-02-13T11:18:10.885547 #4130]  INFO -- : 127.0.0.1 - - "POST /images HTTP/1.1" 200 5
#<Reel::Request POST /images HTTP/1.1 @headers={"Host"=>"127.0.0.1:4567", "Connection"=>"keep-alive", "Content-Length"=>"3437206", "Cache-Control"=>"max-age=0", "Accept"=>"text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8", "Origin"=>"http://localhost:3000", "User-Agent"=>"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.111 Safari/537.36", "Content-Type"=>"multipart/form-data; boundary=----WebKitFormBoundaryMMXfV2Ron4mihT2p", "Referer"=>"http://localhost:3000/", "Accept-Encoding"=>"gzip, deflate", "Accept-Language"=>"en-US,en;q=0.8"}>
{}
@kenichi
Copy link
Owner

kenichi commented Feb 19, 2015

@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?

@tommay
Copy link
Collaborator

tommay commented Feb 19, 2015

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).

@kenichi
Copy link
Owner

kenichi commented Feb 19, 2015

@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

@tarcieri
Copy link

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.

@digitalextremist
Copy link
Contributor

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.

@tarcieri
Copy link

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

@digitalextremist
Copy link
Contributor

@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.

@kenichi
Copy link
Owner

kenichi commented Feb 20, 2015

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

@tarcieri
Copy link

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

@digitalextremist
Copy link
Contributor

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.

@kenichi
Copy link
Owner

kenichi commented Feb 21, 2015

@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.

@pachacamac
Copy link

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

@kenichi
Copy link
Owner

kenichi commented Jan 15, 2017

@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

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

6 participants