onion

onion copied to clipboard

Have a handler for /jobs on PUT request in my server. I use the following to get the request data and parse it as a json string.

    onion_request* c_req = req.c_handler();
    const onion_block *dreq=onion_request_get_data(c_req);
    Json::parse(onion_block_data(dreq))

When I do a PUT as below the onion_block_data(dreq) returns the path to a file like /tmp/onion-g41wWg . The same behavior is exhibited in both cases below. However if I handle the request via a POST handler the below two works correctly.

curl -XPUT -H'Content-Type: application/json' 'http://localhost:3000/jobs' -d @./tools/ack.json
curl -XPUT -H'Content-Type: application/json' 'http://localhost:3000/jobs' -d '
{"some": "json"}
'

As a workaround for now I'm interpreting the file path as a file containing the PUT data.

Edit: When handling data via POST request the curl command is used with -XPOST option.

Sorry for being so late to answer, but I've been thinking how to better solve it.

PUT is used until now for files (mainly webdav), so it made sense to write everything on a file here. Also there is the problem that it can have a lot of data (MB... GB?). So I don't think is good idea by default just placing everything on mem .

So I'm thinking of adding yet another flag to onion_new, something like O_PUT_ON_MEM, which switches the behaviour to what you expected. It has to be here, and can not be by URL because putting the data on mem or file is before dispatching the URL. Also the limits can be or the limit for the post (onion_set_max_post_size) or of the file (onion_set_max_file_size).

Another option may be to create a new function onion_set_max_put_mem_size that internally sets something like this O_PUT_ON_MEM flag, where -1 may mean PUT data go to file.

Any ideas/comments?

Maybe not change anything. Just a piece of documentation of this behavior will suffice.

Btw, I really appreciate the work you have done on this library.

I'm implementing a REST API, and would prefer the same behaviour with both PUT and POST. I do understand the reasoning behind preventing large objects to be temporarily stored in RAM, and I suppose there are different ways to attempt to solve this, with different drawbacks.

Personally for my use case a simple solution would be to have a onion_set_max_put_mem_size() which implicitly stores the objects in RAM, and denies larger sized objects, for both PUT and POST.

Another option would be storing the objects in RAM up to a certain (configurable) size, and fallback to streaming to disk if they grow larger. The lib user would need to be able to understand when reading an onion_block whether it's a reference to file or memory.

The problem with this solution is that then I have or to abstract onion_block to be able to be on disk (maybe with some mmap) or in memory, which will make the normal case of onion_block slower. Or create new API for reading that PUT/POST that does not use onion_block.

I have to think about it, but I think that it will be just onion_set_max_put_mem_size that sets internally a new O_PUT_ON_MEM flag or similar. The problem with this approach is that it is all or nothing: small PUT to memory or huge PUT to disk, but not a mix. A very important note is that normally files are uploaded via POST, not PUT.

Ideally I would much better to use a new HTTP parser (which has been in development for looong time) that would allow the handlers to manage the request data, be it POST or PUT or whatever, so its the handler itself that decides how to manage the incoming data, with nice and backcompatible APIs. But these needs even more effort to make it work.

Any idea how other libraries do it?

Just as a note, Django do it allowing read from the request object (~ the new onion parser approach), although I saw people asking for a File-like object (~ virtualize onion_block aproach), as it does with POST files.

2015-07-23 11:04 GMT+02:00 Fredrik Widlund [email protected]:

Another option would be storing the objects in RAM up to a certain (configurable) size, and fallback to streaming to disk if they grow larger. The lib user would need to be able to understand when reading an onion_block whether it's a reference to file or memory.

— Reply to this email directly or view it on GitHub https://github.com/davidmoreno/onion/issues/104#issuecomment-124027646.

David Moreno Montero

[email protected] +34 658 18 77 17 http://www.coralbits.com/ http://www.coralbits.com

Heh, I'm writing a reactor pattern event library myself, similar to onion, but I don't have the time available mature it enough for production use just now. I've looked at a number of different libraries over the years, and now very recently, and you deserve a big thank you for your software. Really impressive work.

There are some things to be said for flexibility and others for simplicity. For my current use case I really appreciate a higher abstraction layer where I don't have to care about buffering and streaming. I don't care about very large files so onion_set_max_put_mem_size() works just fine and when my handlers are called I have the data ready without the introduction of any unnecessary overhead. Having onion decide where to put the data and give me a hint somehow whether the onion_block content is a filename or the actual data would be fine as well. I'm not sure if I understand why this would make the onion_block abstraction slower?

In the more generic case perhaps the lib user could choose whether to hook into the data receive flow and programatically via some nice simple abstractions stream or buffer the data, or tell onion to handle it for the user.

I've used nxweb for example, but decided not to use it right now partly because the data receive flow was too cumbersome and ad-hoc to make sense.

Also I'm not sure if in general the semantics of PUT and POST are so different when it comes to the size of the objects. Making assumptions there could perhaps be limiting. Webdav perhaps usually uses PUT, but this is discussed for example here (http://greenbytes.de/tech/webdav/rfc5995.html) and in the general case I believe the most important differences are idempotency and whether you add a resource or add to a collection, not how large the object is.

On Thu, Jul 23, 2015 at 3:32 PM, David Moreno Montero < [email protected]> wrote:

The problem with this solution is that then I have or to abstract onion_block to be able to be on disk (maybe with some mmap) or in memory, which will make the normal case of onion_block slower. Or create new API for reading that PUT/POST that does not use onion_block.

I have to think about it, but I think that it will be just onion_set_max_put_mem_size that sets internally a new O_PUT_ON_MEM flag or similar. The problem with this approach is that it is all or nothing: small PUT to memory or huge PUT to disk, but not a mix. A very important note is that normally files are uploaded via POST, not PUT.

Ideally I would much better to use a new HTTP parser (which has been in development for looong time) that would allow the handlers to manage the request data, be it POST or PUT or whatever, so its the handler itself that decides how to manage the incoming data, with nice and backcompatible APIs. But these needs even more effort to make it work.

Any idea how other libraries do it?

Just as a note, Django do it allowing read from the request object (~ the new onion parser approach), although I saw people asking for a File-like object (~ virtualize onion_block aproach), as it does with POST files.

2015-07-23 11:04 GMT+02:00 Fredrik Widlund [email protected]:

Another option would be storing the objects in RAM up to a certain (configurable) size, and fallback to streaming to disk if they grow larger. The lib user would need to be able to understand when reading an onion_block whether it's a reference to file or memory.

— Reply to this email directly or view it on GitHub <https://github.com/davidmoreno/onion/issues/104#issuecomment-124027646 .

David Moreno Montero

[email protected] +34 658 18 77 17 http://www.coralbits.com/ http://www.coralbits.com

— Reply to this email directly or view it on GitHub https://github.com/davidmoreno/onion/issues/104#issuecomment-124101143.