babelouest / ulfius

Web Framework to build REST APIs, Webservices or any HTTP endpoint in C language. Can stream large amount of data, integrate JSON data with Jansson, and create websocket services
https://babelouest.github.io/ulfius
GNU Lesser General Public License v2.1
1.08k stars 182 forks source link

File data not accessible in request->map_post_body #223

Closed itzoke closed 2 years ago

itzoke commented 2 years ago

Unable to access file data I'm trying to access file data from a form/multi-part request. The request consists of one JSON and one JPEG. The JSON part is easily accessible, however the JPEG is hiding somewhere.

If I'm going at this the wrong way please let me know. I did look at the code and it seems to me that this should work.

Example

#include <stdio.h>
#include <ulfius.h>                                                                                                                           

#define PORT 8009

int callback_plex(const struct _u_request *request, struct _u_response *response, void *user_data) {
  // print all keys
  const char **keys = u_map_enum_keys(request->map_post_body);
  uint8_t i; 
  for(i = 0; keys[i] != NULL; i++)
    printf("request has key: %s with data length %d\n", keys[i], u_map_get_length(request->map_post_body, keys[i]));

  return(U_CALLBACK_CONTINUE);
} 

int main() {
  struct _u_instance instance;

  if(ulfius_init_instance(&instance, PORT, NULL, NULL) != U_OK) {
    fprintf(stderr, "Failed miserably...\n");
    exit(EXIT_FAILURE);
  } 

//  instance.check_utf8 = 0;

  if(ulfius_add_endpoint_by_val(&instance, "POST", "/test", NULL, 0, &callback_plex, NULL) != U_OK) {
    fprintf(stderr, "Failed to add endpoint...\n");
    exit(EXIT_FAILURE);
  } 

  if(ulfius_start_framework(&instance) != U_OK) {
    fprintf(stderr, "Failed to start framework...\n");
    exit(EXIT_FAILURE);
  } else {
    while(1) {
      pause();
    } 
  } 
  ulfius_stop_framework(&instance);
  ulfius_clean_instance(&instance);
}

A request is sent using curl -F 'payload={"foo":"bar"}' -F 'thumb=@thumb.jpg' http://localhost:8009/test.

The output from the code above is

request has key: payload with data length 14

If I un-comment instance.check_utf8 = 0; the output becomes

request has key: payload with data length 14
request has key: thumb_filename with data length 10

Expected behavior I would expect output similar to

request has key: payload with data length 14
request has key: thumb_filename with data length 10
request has key: thumb with data length XXXX

System:

babelouest commented 2 years ago

Hello @ltzoke ,

I quickly tried your code and couldn't find why it doesn't work yet.

But if you intend to parse binary files in your callback, I suggest to use ulfius_set_upload_file_callback_function instead, it's more safe.

You can also look at the sheep_counter.c example program, which has an upload file callback example, url: http://localhost:7437/static/upload.html

In this example, if you add instance.check_utf8 = 0; in the main() function and comment out ulfius_set_upload_file_callback_function, then in the file content is printed in the console output, which might be unsafe though.

itzoke commented 2 years ago

Hi,

I quickly tried your code and couldn't find why it doesn't work yet.

I interpret that as it should work the way I was expecting it to work.

I did a test with using ulfius_set_upload_file_callback_function for the JPEG part. However with that method I would need to find a clean way to associate the file with the correlating request (the filename is always the same).

My planned approach is to base64 encode the JPEG and embed it in the JSON.

babelouest commented 2 years ago

I think I see what's wrong here.

In your code, you must have set a value to instance.max_post_param_size, which isn't normal, because if max_post_param_size is 0 (default value), then there shouldn't be any limit to a post param value.

I'll post a patch to fix that.

itzoke commented 2 years ago

The posted code is all code there is atm. So I don't see where I would have set instance.max_post_param_size.

I just did a quick check with gdb and it's set to 0.

Reading symbols from ./test...
(gdb) b test.c:35
Breakpoint 1 at 0x1381: file test.c, line 36.
(gdb) run
Starting program: /home/zoke/Development/plexhooks/src/test 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7ffff6cac640 (LWP 750224)]

Thread 1 "test" hit Breakpoint 1, main () at test.c:36
36              pause();
(gdb) print instance 
$1 = {mhd_daemon = 0x5555555756e0, status = 1, port = 8009, network_type = 1, bind_address = 0x0, bind_address6 = 0x0, timeout = 0, 
  nb_endpoints = 1, default_auth_realm = 0x0, endpoint_list = 0x555555575670, default_endpoint = 0x0, default_headers = 0x5555555755a0, 
  max_post_param_size = 0, max_post_body_size = 0, websocket_handler = 0x5555555755d0, file_upload_callback = 0x0, file_upload_cls = 0x0, 
  mhd_response_copy_data = 0, check_utf8 = 0, use_client_cert_auth = 0}
babelouest commented 2 years ago

As I said, not setting instance.max_post_param_size is a bug from ulfius, not your code.

In your code, if you add instance.max_post_param_size = 128*1024; for example, it should work then.

itzoke commented 2 years ago

Adding the line you proposed yields the following

(gdb) b test.c:36
Breakpoint 1 at 0x1389: file test.c, line 37.
(gdb) run
Starting program: /home/zoke/Development/plexhooks/src/test 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7ffff6cac640 (LWP 750710)]

Thread 1 "test" hit Breakpoint 1, main () at test.c:37
37              pause();
(gdb) print instance 
$1 = {mhd_daemon = 0x5555555756e0, status = 1, port = 8009, network_type = 1, bind_address = 0x0, bind_address6 = 0x0, timeout = 0, 
  nb_endpoints = 1, default_auth_realm = 0x0, endpoint_list = 0x555555575670, default_endpoint = 0x0, default_headers = 0x5555555755a0, 
  max_post_param_size = 131072, max_post_body_size = 0, websocket_handler = 0x5555555755d0, file_upload_callback = 0x0, 
  file_upload_cls = 0x0, mhd_response_copy_data = 0, check_utf8 = 0, use_client_cert_auth = 0}
(gdb) cont
Continuing.
[New Thread 0x7ffff64ab640 (LWP 750715)]
request has key: payload with data length 14
request has key: thumb_filename with data length 10
request has key: thumb with data length 8312
[Thread 0x7ffff64ab640 (LWP 750715) exited]
babelouest commented 2 years ago

Does this "request has key: thumb with data length 8312" mean it's working in this case?

itzoke commented 2 years ago

Yes, I validated with the following in the callback

  if(u_map_has_key(request->map_post_body, "thumb")) {                                                              
    FILE *thumb = fopen("/tmp/thumb.jpg", "w");                                                                     
    fwrite(u_map_get(request->map_post_body, "thumb"), u_map_get_length(request->map_post_body, "thumb"), 1, thumb);
    fclose(thumb);                                                                                                                            
  }

and the resulting '/tmp/thumb.jpg' is as expected.

babelouest commented 2 years ago

Good, I think I fixed the initial problem in the code as well, can you check your first code sample with the last commit?

Thanks for reporting the issue!

itzoke commented 2 years ago

It's working. Thanks for fixing the issue.

I would hate to have to do this in some other "lesser" language :p

Also, I didn't see any mention in the docs that instance.check_utf8 need to be set to 0 in order for files to show up. If it was not just me missing it, it may be a good idea to mention it.

babelouest commented 2 years ago

You're right about the documentation, I'll add a note about check_utf8 and binary files uploaded.

itzoke commented 2 years ago

It seems like this fix broke some other stuff :/ Should I create a separate issue for that or do you want to keep going on this one?

babelouest commented 2 years ago

It depends if the broken thing is related to file upload you can write it here if it's a different matter, you can open a new issue

itzoke commented 2 years ago

Pretty closely related

Sample code pretty close to above (only added JSON testing)

#include <stdio.h>
#include <ulfius.h>

#define PORT 8009

int callback_plex(const struct _u_request *request, struct _u_response *response, void *user_data) {
  // print all keys
  const char **keys = u_map_enum_keys(request->map_post_body);
  uint8_t i;
  for(i = 0; keys[i] != NULL; i++)
    printf("request has key: %s with data length %d\n", keys[i], u_map_get_length(request->map_post_body, keys[i]));

  json_error_t json_error;
  json_t *payload = json_loads(u_map_get(request->map_post_body, "payload"), 0, &json_error);

  if(!payload) {
    fprintf(stderr, "JSON error: %s\n", json_error.text);
    fprintf(stderr, "---\n%s\n---\n", u_map_get(request->map_post_body, "payload"));
  }

  return(U_CALLBACK_CONTINUE);                                                                                                                
}  

int main() {
  struct _u_instance instance;

  if(ulfius_init_instance(&instance, PORT, NULL, NULL) != U_OK) {
    fprintf(stderr, "Failed miserably...\n");
    exit(EXIT_FAILURE);
  }

  instance.check_utf8 = 0;
//  instance.max_post_param_size = 128*1024;

  if(ulfius_add_endpoint_by_val(&instance, "POST", "/test", NULL, 0, &callback_plex, NULL) != U_OK) {
    fprintf(stderr, "Failed to add endpoint...\n");
    exit(EXIT_FAILURE);
  }

  if(ulfius_start_framework(&instance) != U_OK) {
    fprintf(stderr, "Failed to start framework...\n");
    exit(EXIT_FAILURE);
  } else {
    while(1) {
      pause();
    }
  }
  ulfius_stop_framework(&instance);
  ulfius_clean_instance(&instance);
}  

Commands to reproduce error

curl -F 'payload={"event":"media.stop","user":true,"owner":true,"Account":{"id":1,"thumb":"https://plex.tv/users/XXXXXXXXXXXXXXXX/avatar?c=XXXXXXXXXX","title":"zoke"},"Server":{"title":"claire","uuid":"XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"},"Player":{"local":true,"publicAddress":"XXX.XXX.XXX.XXX","title":"Firefox","uuid":"XXXXXXXXXXXXXXXXXXXXXXXX"},"Metadata":{"librarySectionType":"artist","ratingKey":"XXXXXX","key":"/library/metadata/XXXXXX","parentRatingKey":"XXXXXX","grandparentRatingKey":"XXXXXX","guid":"plex://track/XXXXXXXXXXXXXXXXXXXXXXXX","parentGuid":"plex://album/XXXXXXXXXXXXXXXXXXXXXXXX","grandparentGuid":"plex://artist/XXXXXXXXXXXXXXXXXXXXXXXX","parentStudio":"Epic","type":"track","title":"XXXXXXXXXX","grandparentKey":"/library/metadata/XXXXXX","parentKey":"/library/metadata/XXXXXX","librarySectionTitle":"Music","librarySectionID":11,"librarySectionKey":"/library/sections/XX","grandparentTitle":"XXXXXXXXXXXXXXX","parentTitle":"XXXXXXXXXXX","summary":"","index":18,"parentIndex":1,"ratingCount":6690,"parentYear":2003,"XXXXX":"/library/metadata/123308/thumb/XXXXXXXXXX","art":"/library/metadata/XXXXXX/art/XXXXXXXXXX","parentThumb":"/library/metadata/XXXXXX/thumb/XXXXXXXXXX","grandparentThumb":"/library/metadata/XXXXXX/thumb/XXXXXXXXXX","grandparentArt":"/library/metadata/XXXXXX/art/XXXXXXXXXX","addedAt":1640342279,"updatedAt":1647309218},"foo":"bar"}' -F 'thumb=@thumb.jpg' http://localhost:8009/test

curl -F 'payload={"event":"media.stop","user":true,"owner":true,"Account":{"id":1,"thumb":"https://plex.tv/users/XXXXXXXXXXXXXXXX/avatar?c=XXXXXXXXXX","title":"zoke"},"Server":{"title":"claire","uuid":"XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"},"Player":{"local":true,"publicAddress":"XXX.XXX.XXX.XXX","title":"Firefox","uuid":"XXXXXXXXXXXXXXXXXXXXXXXX"},"Metadata":{"librarySectionType":"artist","ratingKey":"XXXXXX","key":"/library/metadata/XXXXXX","parentRatingKey":"XXXXXX","grandparentRatingKey":"XXXXXX","guid":"plex://track/XXXXXXXXXXXXXXXXXXXXXXXX","parentGuid":"plex://album/XXXXXXXXXXXXXXXXXXXXXXXX","grandparentGuid":"plex://artist/XXXXXXXXXXXXXXXXXXXXXXXX","parentStudio":"Epic","type":"track","title":"XXXXXXXXXX","grandparentKey":"/library/metadata/XXXXXX","parentKey":"/library/metadata/XXXXXX","librarySectionTitle":"Music","librarySectionID":11,"librarySectionKey":"/library/sections/XX","grandparentTitle":"XXXXXXXXXXXXXXX","parentTitle":"XXXXXXXXXXX","summary":"","index":18,"parentIndex":1,"ratingCount":6690,"parentYear":2003,"XXXXX":"/library/metadata/123308/thumb/XXXXXXXXXX","art":"/library/metadata/XXXXXX/art/XXXXXXXXXX","parentThumb":"/library/metadata/XXXXXX/thumb/XXXXXXXXXX","grandparentThumb":"/library/metadata/XXXXXX/thumb/XXXXXXXXXX","grandparentArt":"/library/metadata/XXXXXX/art/XXXXXXXXXX","addedAt":1640342279,"updatedAt":1647309218}}' -F 'thumb=@thumb.jpg' http://localhost:8009/test

Output of sample program

request has key: payload with data length 1367
request has key: thumb_filename with data length 10
request has key: thumb with data length 8312
request has key: payload with data length 1355
request has key: thumb_filename with data length 10
request has key: thumb with data length 8312
JSON error: end of file expected near '"foo"'
---
{"event":"media.stop","user":true,"owner":true,"Account":{"id":1,"thumb":"https://plex.tv/users/XXXXXXXXXXXXXXXX/avatar?c=XXXXXXXXXX","title":"zoke"},"Server":{"title":"claire","uuid":"XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"},"Player":{"local":true,"publicAddress":"XXX.XXX.XXX.XXX","title":"Firefox","uuid":"XXXXXXXXXXXXXXXXXXXXXXXX"},"Metadata":{"librarySectionType":"artist","ratingKey":"XXXXXX","key":"/library/metadata/XXXXXX","parentRatingKey":"XXXXXX","grandparentRatingKey":"XXXXXX","guid":"plex://track/XXXXXXXXXXXXXXXXXXXXXXXX","parentGuid":"plex://album/XXXXXXXXXXXXXXXXXXXXXXXX","grandparentGuid":"plex://artist/XXXXXXXXXXXXXXXXXXXXXXXX","parentStudio":"Epic","type":"track","title":"XXXXXXXXXX","grandparentKey":"/library/metadata/XXXXXX","parentKey":"/library/metadata/XXXXXX","librarySectionTitle":"Music","librarySectionID":11,"librarySectionKey":"/library/sections/XX","grandparentTitle":"XXXXXXXXXXXXXXX","parentTitle":"XXXXXXXXXXX","summary":"","index":18,"parentIndex":1,"ratingCount":6690,"parentYear":2003,"XXXXX":"/library/metadata/123308/thumb/XXXXXXXXXX","art":"/library/metadata/XXXXXX/art/XXXXXXXXXX","parentThumb":"/library/metadata/XXXXXX/thumb/XXXXXXXXXX","grandparentThumb":"/library/metadata/XXXXXX/thumb/XXXXXXXXXX","grandparentArt":"/library/metadata/XXXXXX/art/XXXXXXXXXX","addedAt":1640342279,"updatedAt":1647309218}}"foo"`
---
^[[?1;2c

It seems that when the second JSON is shorter than the first one we have some strays from the first. This does not happen when testing with short JSON strings ({"foo":"bar","bar":"foo"}/{"foo":"bar"} was tested)

It seems there is a missing free or at the very least a null termination.

babelouest commented 2 years ago

Yup, this change broke my code too, so I saw it too, but you were first :-)

I force added a null character at the end of the struct _u_map.values, and fixed another side bug. Can you take a look?

itzoke commented 2 years ago

Looks good to me. Sample application works as expected.

Thanks again.