Feature/http multipart post

[patrickjahns]

Some fixes for issues noticed in #694

[robotiko]

Tested all flows:
it just seems to fail in extreme cases:
-If file to upload is bigger than 1MB, it will give a successfully upload and list it in the file list but fails to open (Esp8266 1MB boundary??)
-Large file names trimm the name and extension but work ok and can be read.
-reuploading a file will rewrite the content if there is enough space to upload it (it doesn't check ahead that is a rewrite).

-In two cases I got file corruption of a previously and correctly uploaded content, after a new big upload (1MB boundary?). ** I will do further testing to track down this, as the only relevant issue **

post log.txt

Minor detail as example, any error is displayed in console but not on the upload page

[patrickjahns]

-If file to upload is bigger than 1MB, it will give a successfully upload and list it in the file list but fails to open (Esp8266 1MB boundary??)

That is a limitation of spiffs - while we can guess the probably free space - we can never surely tell if it will fit since spiffs also requires space for metadata
https://github.com/pellepl/spiffs/wiki/Using-spiffs#checking-filesystem

Large file names trimm the name and extension but work ok and can be read.

Also spiffs limitation - 31chars is the maximum (including the extension) - for this scenario it might be good to define the default behavior (show error page saying "filename too large")

reuploading a file will rewrite the content if there is enough space to upload it (it doesn't check ahead that is a rewrite).

That is proper default behavior - it is up to the sming user to tell if the upload is accepted or not in the callback.

In two cases I got file corruption of a previously and correctly uploaded content, after a new big upload (1MB boundary?). ** I will do further testing to track down this, as the only relevant issue **

That would be good - I suspect it's also related to spiffs - it would be great if you can track this down.
Thanks for including a log - would be great if you can limit it to the parts that caused issue - it's hard for me to look at 27000 lines without knowing where the issue happened.

[hreintke]

@patrickjahns : @robotiko :
Quick first reply.

If file to upload is bigger than 1MB, it will give a successfully upload and list it in the file list but fails to open (Esp8266 1MB boundary??)

That is a limitation of spiffs - while we can guess the probably free space - we can never surely tell if it will fit since spiffs also requires space for metadata

Spiffs/filewrite should give an error return when a filewrite doesn't fit on the FS. That is independent on the size of the file currently being written. -> Also small files can get the FS filled.
It might be related to the possible mismatch in sizing of the spiffy created FS and the mounted FS in Sming (Solved in SmingRTOS 😄 ). In rboot application it is aligned by using mount_manual.

Also spiffs limitation - 31chars is the maximum (including the extension) - for this scenario it might be good to define the default behavior (show error page saying "filename too large")

There is an application callback, if the filename is to long before the callback, the application can update it to a correct filename. If it is to long after the callback, the upload should be cancelled (skipped) and reported as an erroneous download when the httprequest is complete.

[patrickjahns]

@hreintke

Spiffs/filewrite should give an error return when a filewrite doesn't fit on the FS. That is independent on the size of the file currently being written. -> Also small files can get the FS filled.
It might be related to the possible mismatch in sizing of the spiffy created FS and the mounted FS in Sming (Solved in SmingRTOS 😄 ). In rboot application it is aligned by using mount_manual.

Thats what I meant its a limitation/issue with spiffs - for the upload scenario if the file write doesnt return an error or the free space is wrongly announced the upload part cant know it

There is an application callback, if the filename is to long before the callback, the application can update it to a correct filename. If it is to long after the callback, the upload should be cancelled (skipped) and reported as an erroneous download when the httprequest is complete.

more thoughts on this

• it is good that the application can intercept the behavior
• filename length check will be skipped, if the callback handles the file writing and not the default handler
• i`d prefer cancelling the request and displaying a 4xx page

different things that came to my mind:
It would probably be a good idea to provide access to httpresponse in the uploadhandler and have the upload handler return true/false (or a status code) to indicate if an error occured in the callback and if we should drop the request.

This is related to a different thought I was having: if we want to secure the upload endpoint with http basic auth - it is currently only possible after the upload finished and we are in the normal callback. But for security purposes we should intercept the upload after the headers have been parsed and before we actually open+write to a file

[hreintke]

i`d prefer cancelling the request and displaying a 4xx page

The http_response class should not write a response. It is up to the application to react to the "outside world" We just provide the information : The upload was aborted.

This is related to a different thought I was having: if we want to secure the upload endpoint with http basic auth - it is currently only possible after the upload finished and we are in the normal callback

The httprequest is available in the httpupload callback. With that information the application can decide. (correct ?)

[patrickjahns]

The httprequest is available in the httpupload callback. With that information the application can decide. (correct ?)

Yes the request is available

But my remarks are, with the current flow:

• C: send request with header (and maybe some data)
• SMING: parse header
• SMING: parse body -> we find multipart
• SMING: hand over to application upload
• APP Upload callback: check for password/username in header -> not allowed and set to skip upload
• C: send more data until finished
• SMING: parse data until client finishes
• SMING: hand over to application callback
• APP: Request is not allowed - send not allowed response
• SMING: return the response

This way even if someone is not allowed to upload data - it is still forced to first accept all client packets before we actually react and close the connection

It would be better if the APP: has access to response object; sets the response to not allowed; tells Sming to not process it further but send the response/error code at that very moment

(Similar to the process we internally do when we receive a bad request)

[slaff]

Post parameters with multipart encoding should be implemented with the upcoming Http refactoring (#1112).