totaljs / framework

Node.js framework
http://www.totaljs.com
Other
4.36k stars 450 forks source link

multipart parser #671

Closed sersh88 closed 5 years ago

sersh88 commented 6 years ago

Sometimes I see this string as filename: form-data; name="file4"; filename="

Looks like somthing wrong with multipart parser.

And it is possible to use third party multipart parser like formidable with total.js ?

petersirka commented 6 years ago

Hi @sersh88, Total.js uses formidable parser... Can you send me some script for testing?

sersh88 commented 6 years ago

Well, this is difficult, becouse this error appears not every time ( I have thousands of uploads per day) and I don't understand what need's to be logged to catch this.

petersirka commented 6 years ago

OK, and what is the real problem?

sersh88 commented 6 years ago

Uploads are done with php script using curl. Requests contains both files and some post fields. For some files in this request the filename is wrong - its name is like form-data; name="file4"; filename=" Before I used php to handle this uploads and everything worked fine.

petersirka commented 6 years ago

What's the size and type of file? I'll try to simulate something similar.

sersh88 commented 6 years ago

the size is different - 5-300kb The type is jpg

sersh88 commented 6 years ago

In php file fields are created like this:

$data['file' . $_i] = \curl_file_create(realpath($file));
sersh88 commented 6 years ago

The amount of files in one request - about 5-10

petersirka commented 6 years ago

I'll try to simulate it today. I'll give you feedback.

petersirka commented 6 years ago

I have tested uploading of 20 000 files:


sersh88 commented 6 years ago

Yes, I upload multiple files with one field of additional data. I have tried also to simulate hundreds of uploads of same files - also no problem. Looks like it happens with particular files and data. I am trying to catch this error and copy these files for test, will be back later.

sersh88 commented 6 years ago

Well, catched this by now: In internal.js in function onHeaderValue the function gets not full buffer I guess. I put some logging before this function:

log.push(buffer.slice(start - 50, end + 100).toString(ENCODING));
var header = buffer.slice(start, end).toString(ENCODING);

And this log showed: ---------------e278e3845310\r\nContent-Disposition: form-data; name="file5"; filename="/www/fasttube/backend/app/cache/upl So, after var header = buffer.slice(start, end).toString(ENCODING); header becomes: form-data; name="file5"; filename="/www/fasttube/backend/app/cache/upl And after calling header = parse_multipart_header(header); : [ 'file5', 'form-data; name="file5"; filename="' ] So, filename becomes 'form-data; name="file5"; filename="

What do you think? Why buffer could be truncated? Where to log more? Again - this happens not every time.

petersirka commented 6 years ago

Thank you. Are your data still same?

Content-Disposition: form-data; name="file5"; filename="/www/fasttube/backend/app/cache/upl...."
petersirka commented 6 years ago

Are you sure that your sender can't send bad data?

sersh88 commented 6 years ago

Not sure, need to check. But I send with php curl. And this error appears only maybe for 1 of 500 uploads. Maybe network error, I don't know. Will try to find the way to check.

petersirka commented 6 years ago

I need to simulate it... As I wrote I tested more than 20000 uploaded 300 kB files without any problem. Let me know.

petersirka commented 6 years ago

Any news?

sersh88 commented 6 years ago

Give me please couple of days to debug this. Was very busy with other projects. Problem still here.

petersirka commented 6 years ago

OK, no problem.

sersh88 commented 6 years ago

Still testing... Error happening one - two times per day, so still not understand why. Changed nginx configuration about timeouts, maybe becouse of them.

sersh88 commented 5 years ago

I give up. Too difficult for me to debug it. Still getting this error randomly. What I discovered, is in internal.js function parser.onHeaderValue gets cutted buffer sometimes, like this: form-data; name="file1"; filename="/www/fasttube/backend/app/cache/uploads/2018-11-07-1541 And should get buffer like this: form-data; name="file3"; filename="/www/fasttube/backend/app/cache/uploads/2018-11-07-1541607576-5be310982beef/t_8.jpg The logic of how it cuts header values is too difficult for me to examine. As I understand, it may be formidable bug.

petersirka commented 5 years ago

There can be a problem in some character. Are you sure the filename doesn't contain any : colon or \n? Are you sure upload data are really correct?

Can you create a small examples with upload/download? I need to capture this problem and then I'll fix it.

sersh88 commented 5 years ago

No, filename doesn't contain any unusual chars. And I tried exactly same uploads repeated thousand times without error. I mean I catched the errored upload, copied all the files of this upload, all fields and headers and then simulated same upload thousands times - no error. So, looks like this error occures only when different uploads comes simultaniously. I also upgraded node, upgraded nginx - not helped. I will try another multipart parser.

sersh88 commented 5 years ago

Made a separate script for uploading task with vanilla node http server + multiparty parser. Looks like this helped, no errors so far. So, I think it's formidable or total.js issue, but I don't know how to track it. And one feature request - please, make it possible to handle route customly. I know about controller.custom(), but in this case it is too late, because total.js already parsed body and headers with 'upload' flag. Maybe add 'custom' flag which will mean don't do anything, let the script handle req and res as with vanilla http server.

petersirka commented 5 years ago

I understand. I'll try to upload 100K files again from multiple sources. I'll give you a feedback.

petersirka commented 5 years ago

I have performed several test and no issue present again. I'm closing this issue for now.