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

Feature request:upload handler #1

Closed
bao-qian opened this issue Aug 8, 2013 · 20 comments
Closed

Feature request:upload handler #1

bao-qian opened this issue Aug 8, 2013 · 20 comments

Comments

@bao-qian
Copy link

bao-qian commented Aug 8, 2013

Hi,
Could you support mg_upload wrapper and callbacks.upload ?

@bao-qian
Copy link
Author

bao-qian commented Aug 8, 2013

UPDATE:
I'm trying to implement it, but when I write code like this in Request.cpp. The mg_upload will always return 0.
Any suggestion?

mg_upload(request.connection, "/tmp");

@Gregwar
Copy link
Owner

Gregwar commented Aug 9, 2013

Hi

Is the form you are using to post the file good? (It has to be multipart)

@bao-qian
Copy link
Author

bao-qian commented Aug 9, 2013

Thanks for your response.
Yes, I use multipart to send it.
Here is the command to sent file:
curl --form "test=@stage1.png" http://127.0.0.1:8080
It can do the job when I use mongoose.c directly.
Here is my code:

Request.cpp:

...
int Request::upload(string destination_dir)
{
    return mg_upload(connection, destination_dir.c_str());
}

Request.h

...
int upload(string destination_dir);
...

upload.cpp

#include <unistd.h>
#include <stdlib.h>
#include <signal.h>
#include <mongoose/Server.h>
#include <mongoose/WebController.h>
using namespace std;
using namespace Mongoose;

class MyController : public WebController
{
    public: 
        void upload(Request &request, StreamResponse &response)
        {
            response << "Upload test begin " << endl;
            response << "Number of file uploaded:" << request.upload(".") << endl;
        }
        void setup()
        {
            addRoute("POST", "/", MyController, upload);
        }
};
int main()
{
    MyController myController;
    Server server(8080);
    server.registerController(&myController);
    server.start(); 
    while(1)
        sleep(10);
}

Here is the output:

Upload test begin 
Number of file uploaded:0

Any idea?

@Gregwar
Copy link
Owner

Gregwar commented Aug 10, 2013

Ok

I guess this is because I systematically read all the post data in the request, so mg_upload is not fed anymore by mg_read

we'll have to change this behaviour to support post files

@Gregwar
Copy link
Owner

Gregwar commented Aug 10, 2013

Line 22 of Request.cpp should also test if the request is multipart for e.g, and avoid reading post in this case

(I cant work ok this since I am not near a computer now)

@bao-qian
Copy link
Author

Yes, you are right.
I just tested PUT method, and it's working!!
I think we need get a universal way to handle data send by client, because not only POST method can send file.

@Gregwar
Copy link
Owner

Gregwar commented Aug 10, 2013

Why not, but if we test the Content-type request header to check for a boundary like mg_upload we could detect multipart requests

@bao-qian
Copy link
Author

Since there are two ways to send data:
application/x-www-form-urlencoded and multipart/form-data
This may be better:
Request.cpp

...
// Downloading send data
ostringstream sendData;
// if (method == "POST") {
const char * content_type = mg_get_header(connection, "Content-Type");
        if (content_type!= NULL && strcmp(content_type, "application/x-www-form-urlencoded") == 0){
        int 
        char post[
        while (n = mg_read(connection, post, sizeof(post))) {
            sendData.write(post, n);
        }
    }
data = sendData.str();
...

Also I noticed that you request_info->http_headers array manually to get header, but mongoose.c already offered a convenient function: mg_get_header.

@bao-qian
Copy link
Author

Hi, I have implemented the upload feature, including upload callback(same as you did for begin_request).
I will create a pull request later.

@Gregwar
Copy link
Owner

Gregwar commented Aug 10, 2013

I still don't imagine what is the best way to put the file upload in the binding,

I dont think the callback is really important, we could indeed have a method to save the uploaded file somewhere

@bao-qian
Copy link
Author

the mg_upload can decide the location. But If you don't have that call
back, you can't get the file name.

I still don't imagine what is the best way to put the file upload in the
binding,

I dont think the callback is really important, we could indeed have a
method to save the uploaded file somewhere


Reply to this email directly or view it on
GitHubhttps://github.com//issues/1#issuecomment-22437831
.

@Gregwar
Copy link
Owner

Gregwar commented Aug 10, 2013

You're right

However, I'm not really happy with the way mongoose handle the naming of
the uploaded files, it look like it's always using the client filename

Allowing the conttoller developper to choose the name would be better,
maybe we could implement a method to rename the file to an arbitrary
location
Le 10 août 2013 13:54, "happlebao" notifications@github.com a écrit :

the mg_upload can decide the location. But If you don't have that call
back, you can't get the file name.

I still don't imagine what is the best way to put the file upload in the
binding,

I dont think the callback is really important, we could indeed have a
method to save the uploaded file somewhere


Reply to this email directly or view it on
GitHub<
https://github.com/Gregwar/mongoose-cpp/issues/1#issuecomment-22437831>
.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1#issuecomment-22438506
.

@bao-qian
Copy link
Author

If we want to rename the file, I think it's hard to handle multiple files in one request.
And since the aim of this project is offer a c++ wrapper of mongoose, we need the same functionality as mongoose.
Mongoose has callback, then we need callback too.

@Gregwar
Copy link
Owner

Gregwar commented Aug 10, 2013

Yes, we will have to use the callback

Maybe we could create a method that call mg_upload, then the callback
will give us the name of files and we could access it as a
vector where UploadedFile is a class encapsulating the name
and some methods to rename it, remove it etc.
Le 10 août 2013 14:21, "happlebao" notifications@github.com a écrit :

If we want to rename the file, I think it's hard to handle multiple files
in one request.
And since the aim of this project is offer a c++ wrapper of mongoose, we
need the same functionality as mongoose.
Mongoose has callback, then we need callback too.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1#issuecomment-22438876
.

@bao-qian
Copy link
Author

But if we do that, then this project is no longer a just simple binding of mongoose.

@Gregwar
Copy link
Owner

Gregwar commented Aug 10, 2013

It's already more than that (think of sessions, controllers and routing for
instance)

The goal is not just to wrap mongoose but to provide a developer-friendly
API for writing applications with a webserver embedded

Mongoose is tiny and compiles very fast, which makes it cool but using it
in pure-C style tastes like dealing with buffers of char[], sprintf and
other things that I'm not happy to use in a C++ app :-)

We are not just providing a simple binding but also some additional helpers
to make easier the building of an embedded webapp
Le 10 août 2013 16:46, "happlebao" notifications@github.com a écrit :

But if we do that, then this project is no longer a just simple binding of
mongoose.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1#issuecomment-22440999
.

@Gregwar
Copy link
Owner

Gregwar commented Aug 10, 2013

Maybe I should change this repo description to something like "C++ web-application library based on mongoose"

Gregwar added a commit that referenced this issue Aug 10, 2013
@Gregwar
Copy link
Owner

Gregwar commented Aug 10, 2013

I've just commited a simple upload system

I think we should now add possibilities for do things with uploaded files like move/rename them

@Gregwar
Copy link
Owner

Gregwar commented Aug 10, 2013

(see /upload in the main.cpp example)

@bao-qian
Copy link
Author

Thanks a lot.

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

2 participants