modified MIMEParser class for upload of larger files and multiple files #132

squonk11 · 2020-05-16T16:45:59Z

modified MIMEParser class for uploading lager files and multiple files

squonk11 · 2020-05-17T09:04:47Z

I will take care about the remaining issues during the next days. Unfortunately I have a problem running the shell scripts for the tests in my Docker; I think this is related to fact that the shell scripts are having CRLF line endings. I will try to find a fix for this also.

squonk11 · 2020-05-21T06:39:07Z

Now I changed the line endings of the shell scripts to LF and the error message of the missing interpreter "bin/bash^M" is gone and the script starts to be executed in Docker. But now I get the error message ./CI/build_test.sh: line 14: docker: command not found. Do you have an idea what I am missing?

PerMalmberg · 2020-05-21T07:11:57Z

The CI-scripts are meant to be run outside of docker; they will start an instance of the docker-image to run the tests.

squonk11 · 2020-05-21T07:33:49Z

but starting the scripts outside of Docker I can only do under Linux, right? Or is there a trick to do it under Windows also - maybe WSL?

PerMalmberg · 2020-05-21T07:38:02Z

Yes, I'd expect WSL to work.

I'd guess any bash terminal would work, such as Git-bash that comes with Git for Windows.

squonk11 · 2020-05-21T17:16:35Z

unfortunately both do not work. With git bash I get the error message: C:\Program Files\Docker\Docker\resources\bin\docker.exe: Error response from daemon: the working directory 'C:/Program Files/Git/src' is invalid, it needs to be an absolute path. I guess this is somehow related to the my Git installation because the directory C:/Program Files/Git/src really does not exist.
With WSL I get the error message docker: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?. This probably is due to the fact that WSL assumes that Docker is installed under WSL (but it is not).
So, I think I have to continue the try and error method via several commit/push sequences.

PerMalmberg · 2020-05-21T19:18:16Z

Aww, sorry to hear that.

You can ofc simply run the commands that the CI-scripts themselves run from within Docker.

PerMalmberg · 2020-05-22T13:03:08Z

Last! :) @squonk11 :)

PerMalmberg · 2020-05-27T11:24:57Z

@squonk11 Are you happy with the PR in its current state? I've not test run it myself, but I presume you have.

squonk11 · 2020-05-27T17:57:02Z

I did quite a lot of testing with the previous version using lambda functions. This version I did not test so much. I do not expect issues with this version but before accepting the pull request maybe it would be better to do some more testing. I will try to do it over the next weekend and finally add this PR to the file CONTRIBUTORS.md.
Do you have any more comments on the code?

PerMalmberg · 2020-05-27T18:35:28Z

I didn't see any obvious issues with the code.

I was thinking that we could perhaps do some automatic testing using the Linux version and curl inside docker, i.e. do actual calls to the HTTP-server. That way we get reproducability of the tests and can test a large range of cases.

squonk11 · 2020-05-27T19:56:50Z

Automatic testing would be a good idea. Unfortunately I am having problems starting bash scripts from outside Docker. Is it possible to do use these scripts from inside Docker as well? How to start the HTTP-server inside Docker? Additionally I did not use curl before - but I think I could find out how to use it.

PerMalmberg · 2020-05-27T20:14:08Z

The scripts that currently start the CI-test and builds just start a docker instance to run other scripts. The "other" scripts can all be run manually from within docker.

Python might actually be a better idea than curl, as it is easier to build tests in Python than in bash.

Starting the HTTP-server inside docker would require a test application built for testing the HTTP server specifically. The tests we currently have can serve the example folder with all the images, but that's not really useful in automated testing.

I'm thinking that an echo server, i.e. reply back with what is received, could be a useful thing. Then we can automated sending and receiving data from a single byte up to several mega-bytes with little effort.

squonk11 · 2020-05-28T12:52:25Z

So, the Python script should launch a HTTP client which sends the files to the HTTP test-server. The HTTP test-server will then echo the files back to the python script. In order to receive the echoed files the python script needs to also launch a HTTP server which will then receive the files and comapres the contents against the files sent. Is that what you have in mind?
EDIT: to be more precise: it will not "launch" the HTTP client and server but "act" as a HTTP client and server.

PerMalmberg · 2020-05-28T16:36:24Z

No, no need for a HTTP server on the Python side, that can be done over the same client socket that Python uses to send the data. So, POST some data, then receive the answer.

squonk11 · 2020-05-29T08:29:54Z

Will the upload from the Python script be done via requests; e.g. like so:

import requests

url = "http://localhost:5000/"
fin = open('simple_table.pdf', 'rb')
files = {'file': fin}
try:
  r = requests.post(url, files=files)
	print r.text
finally:
	fin.close()

?
How must the Http-Server send the file back to the Python script? I guess it must send it also via a Multipart/Mime protocoll. But how to do that - there is not such a functionality in Smooth, isn't it?

PerMalmberg · 2020-05-29T08:40:21Z

Yes, requests will most likely do the job.

No there isn't. How about this:

Pre-generate a set of random binary data so that each test can be recreated and re-run.
Create a hash of the data
Send the data to Smooth.
Calculate the hash on the received data.
Reply with the hash
Compare the hash. If they don't match, the test fails.

This would allow us to test everything from 1 byte to megabytes.

squonk11 · 2020-05-29T08:47:40Z

The hash will be one hash value calculated over all files to be send (in case of multiple files)? The hash value will then be returned via the HTTP response?

squonk11 · 2020-06-12T12:21:13Z

Now I have the first version of the test ready.
I used the StringResponse to reply with the JSON string. In Python this string gets converted to a Python dict. The hashes of the files are then compared to the hashes returned from the HTTP server and a success or error message is printed.
I tested several times with the 1000 icon files and no error ocured. The only message I see is again that the watchdog on one CPU is triggered due to not being serviced. But the transmission still completes.
Shall I upload this as first version for discussion into this PR since it is still open?

PerMalmberg · 2020-06-12T12:59:50Z

Yes please do that. Did you try with larger files too?

…

On Fri, 12 Jun 2020, 14:21 Lothar, ***@***.***> wrote: Now I have the first version of the test ready. I used the StringResponse to reply with the JSON string. In Python this string gets converted to a Python dict. The hashes of the files are then compared to the hashes returned from the HTTP server and a success or error message is printed. I tested several times with the 1000 icon files and no error ocured. The only message I see is again that the watchdog on one CPU is triggered due to not being serviced. But the transmission still completes. Shall I upload this as first version for discussion into this PR since it is still open? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#132 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAU2LLF4FPBN7JKSJZYWKMTRWIMUPANCNFSM4NC7UQLA> .

squonk11 · 2020-06-12T14:06:25Z

yes, I tried also with lager files; e.g. three files: 8MB, 10MB and 1kB
I will provide the update soon.

PerMalmberg · 2020-06-12T20:00:36Z

I tried to run this in as a Linux version, but that unfortunately results in a crash.

[1] 31201 abort (core dumped) ./http_files_upload_test.elf

I'll have to debug this and see what goes wrong.

squonk11 · 2020-06-12T20:15:04Z

Dies it crash during or after file(s) Transmission? Am 12. Juni 2020 22:00:49 MESZ schrieb Per Malmberg <[email protected]>:

…

I tried to run this in as a Linux version, but that unfortunately results in a crash. `[1] 31201 abort (core dumped) ./http_files_upload_test.elf` I'll have to debug this and see what goes wrong. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #132 (comment)

-- Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.

PerMalmberg · 2020-06-12T20:55:19Z

As soon as transmission starts. Fist file.

…

On Fri, 12 Jun 2020, 22:15 Lothar, ***@***.***> wrote: Dies it crash during or after file(s) Transmission? Am 12. Juni 2020 22:00:49 MESZ schrieb Per Malmberg < ***@***.***>: >I tried to run this in as a Linux version, but that unfortunately >results in a crash. > >`[1] 31201 abort (core dumped) ./http_files_upload_test.elf` > >I'll have to debug this and see what goes wrong. > >-- >You are receiving this because you were mentioned. >Reply to this email directly or view it on GitHub: >#132 (comment) -- Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#132 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAU2LLAGLVOAER2XKKTOHKLRWKEFNANCNFSM4NC7UQLA> .

squonk11 · 2020-06-12T22:44:57Z

I checked the sources again. Maybe there could be two weak points:

in UploadResponder::start_of_request() I am using sodium_malloc(). Maybe a stub is needed for this?
In MIMEParser::parse() I define status as static. Maybe it must be a private member variable of MIMEParser?

Are you using the secure or the insecure server for the test?

PerMalmberg · 2020-06-13T13:49:44Z

I used the insecure server. It's most likely related to the sodium stuff, not sure why though. Adding gdb to the container right now to debug it.

A static local in parse() is no good since it then is is shared between instances.

PerMalmberg · 2020-06-13T14:51:51Z

First crash solved - sodium_init() must be called prior to using the library.

Another problem that then immediately showed it self - in end_of_request(), the call to free(pState) must be changed to sodium_free().

This clearly shows why it is beneficial to perform most of the work on a Linux system - the debugging facilities are so much better.

I'll be testing this a bit and will push the changes to you branch (if I can).

Changed free() to sodium_free(). Made static function variable into class-member to avoid collisions with multiple instances. General code review.

PerMalmberg · 2020-06-13T15:53:35Z

I'm done testing now, just waiting for the CI scripts to run locally. I see that you changed it to a local variable. I'll merge that into my local copy and push my changes to this PR.

squonk11 · 2020-06-13T16:05:07Z

o.k. thanks. Then I will check again.
Sure, testing under a "real" OS is much easier because you have all tools available. Usually I am working on µCs where this is not possible - even the logging to a terminal (like here in esp-idf) is not possible. Then debugging becomes very ugly...

…branch

PerMalmberg · 2020-06-13T16:31:41Z

Just pushed my changes, have a look.

Yes, uCs are a pain to debug, which is why I made Smooth so that it would be possible to use all the cool toys available :)

squonk11 · 2020-06-13T16:36:44Z

yes, great idea!
Do you know if I just have to do git pull in order to receive your changes?

PerMalmberg · 2020-06-13T16:41:55Z

Yes a pull should be all you need.

…

On Sat, 13 Jun 2020, 18:36 Lothar, ***@***.***> wrote: yes, great idea! Do you know if I just have to do git pull in order to receive your changes? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#132 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAU2LLDBREEKJAD4Y62DFODRWOTKPANCNFSM4NC7UQLA> .

squonk11 · 2020-06-13T17:11:12Z

I tested it again and it seems to work - after adjusting the url in upload.py.
Interesting to see your modifications in upload.py - for me it was more or less the first Python script I made.
Are there any other changes needed from my side? Maybe adding PR132 to CONTRIBUTORS.md?

PerMalmberg · 2020-06-13T17:21:23Z

Yes, please add it to the contributions file, then I'll accept this PR.

PerMalmberg · 2020-06-13T17:57:13Z

Merged! Thank you for your contribution @squonk11, great work.

squonk11 · 2020-06-14T13:26:01Z

Thank you also to you - without your support things might have been much more difficult. BTW: the last change to CONTRIBUTORS.md got lost somehow... Am 13. Juni 2020 19:57:25 MESZ schrieb Per Malmberg <[email protected]>:

…

Merged! Thank you for your contribution @squonk11, great work. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #132 (comment)

-- Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.

PerMalmberg · 2020-06-14T19:00:07Z

Huh, how odd. I'll fix that.

PerMalmberg · 2020-06-14T19:09:48Z

@squonk11 Dunno if the commit was advertised to you, but it was just a typo. It's fixed in master now.

squonk11 added 7 commits May 16, 2020 18:42

modified MIMEParser class for upload of larger files and multiple files

5ed625d

modifications for uncrustify

eb4b86c

next try to uncrustify MIMEParser.cpp

c50e11c

uncrustify UploadResponder.*

83fe6da

correction to MIMEParserTest in linux_unit_tests

b8aa59e

fix on MIMEParserTest.cpp

a5c816c

some more fixes in MIMEParserTest.cpp

5559ab7

squonk11 added 5 commits May 22, 2020 10:53

corrections to MIMEParserTest.cpp

e3a565c

uncrustified MIMEParserTest.cpp

2ed62b4

next modification to MIMEParserTest.cpp

95d8385

next

c37e006

last?

595b1d3

squonk11 added 3 commits June 12, 2020 16:25

added http_files_upload_test to tests

b3773db

fixed compilation error in UploadResponder.h

6268fbe

fixed some more warnings which were treated as errors

a9b643e

squonk11 and others added 2 commits June 13, 2020 17:50

status in MIMEParser::parse is now private member variable (was static)

c94bf9e

Added missing initialization of libsodium.

69b8f28

Changed free() to sodium_free(). Made static function variable into class-member to avoid collisions with multiple instances. General code review.

uncrustify...

30ef412

PerMalmberg added 2 commits June 13, 2020 18:23

Merge branch 'mybranch' of https://github.com/squonk11/Smooth into my…

1df6754

…branch

Don't cast to const iterator, use cbegin() instead.

2a4ce7c

added PR#132 to CONTRIBUTORS.md

6047cea

PerMalmberg merged commit c6e1a1d into PerMalmberg:master Jun 13, 2020

modified MIMEParser class for upload of larger files and multiple files #132

modified MIMEParser class for upload of larger files and multiple files #132

Conversation

squonk11 commented May 16, 2020

squonk11 commented May 17, 2020

squonk11 commented May 21, 2020

PerMalmberg commented May 21, 2020

squonk11 commented May 21, 2020

PerMalmberg commented May 21, 2020

squonk11 commented May 21, 2020

PerMalmberg commented May 21, 2020

PerMalmberg commented May 22, 2020

PerMalmberg commented May 27, 2020

squonk11 commented May 27, 2020

PerMalmberg commented May 27, 2020

squonk11 commented May 27, 2020

PerMalmberg commented May 27, 2020

squonk11 commented May 28, 2020 • edited Loading

PerMalmberg commented May 28, 2020

squonk11 commented May 29, 2020

PerMalmberg commented May 29, 2020

squonk11 commented May 29, 2020

squonk11 commented Jun 12, 2020

PerMalmberg commented Jun 12, 2020 via email

squonk11 commented Jun 12, 2020

PerMalmberg commented Jun 12, 2020

squonk11 commented Jun 12, 2020 via email

PerMalmberg commented Jun 12, 2020 via email

squonk11 commented Jun 12, 2020

PerMalmberg commented Jun 13, 2020

PerMalmberg commented Jun 13, 2020

PerMalmberg commented Jun 13, 2020

squonk11 commented Jun 13, 2020

PerMalmberg commented Jun 13, 2020

squonk11 commented Jun 13, 2020

PerMalmberg commented Jun 13, 2020 via email

squonk11 commented Jun 13, 2020

PerMalmberg commented Jun 13, 2020

PerMalmberg commented Jun 13, 2020

squonk11 commented Jun 14, 2020 via email

PerMalmberg commented Jun 14, 2020

PerMalmberg commented Jun 14, 2020

squonk11 commented May 28, 2020 •

edited

Loading