Files (stl/gcode) corrupted on upload

[CapnBry]

I was having a problem with binary STL files not wanting to be sliced by Slic3r because "File size mismatch". I looked at the uploaded STL file and sure enough it had two extra bytes on the end of it \r\n.

I started digging in and all files which I've uploaded using the relatively new "temp upload file rename" tornado server code have extra \r\n at the end of them. This isn't a problem with gcode files or text STL files because the extra data is just ignored.

I tracked this down to src/octoprint/server/util/tornado.py:UploadStorageFallbackHandler. What's happening is it is looking for the multipart delimiter and applying everything between the delimiters as "data". However, mime states that the boundary has to start on a new line so the actual data is appended by the sender with \r\n then the multipart boundary on the next line. This is true for all content types and looks like this:

XXXXXXXX
--mimeboundary

The code in the tornado handler looks for the boundary and splits the data off properly but doesn't remove the newline at the end. The file will contain the delimiter which is actually the start of the mime boundary. The simple fix is this

diff --git a/src/octoprint/server/util/tornado.py b/src/octoprint/server/util/tornado.py
index cecc8f6..9d70cf0 100644
--- a/src/octoprint/server/util/tornado.py
+++ b/src/octoprint/server/util/tornado.py
@@ -194,7 +194,8 @@ class UploadStorageFallbackHandler(tornado.web.RequestHandler):
                end_of_header = None
                if delimiter_loc != -1:
                        # found the delimiter in the currently available data
-                       data, self._buffer = data[0:delimiter_loc], data[delimiter_loc:]
+                       delimeter_data_end = delimiter_loc if delimiter_loc == 0 else delimiter_loc - 2
+                       data, self._buffer = data[0:delimeter_data_end], data[delimiter_loc:]
                        end_of_header = self._buffer.find("\r\n\r\n")
                else:
                        # make sure any boundary (with single or double ==) contained at the end of chunk does not get

But you need to make sure that the request bodies are being rebuilt properly for other multipart types (which I believe have extra \r\n in them currently). I think they are but I am sure you have a better way of looking than mine, which is to print() data to the console. I am so bad at Python I should be ashamed.

[GitIssueBot]

Hi @CapnBry,

It looks like there is some information missing from your ticket that will be needed in order to process it properly. Please take a look at the Contribution Guidelines and the page How to file a bug report on the project wiki, which will tell you exactly what your ticket has to contain in order to be processable.

If you did not intend to report a bug, please take special note of the title format to use as described in the Contribution Guidelines.

I'm marking this one now as needing some more information. Please understand that if you do not provide that information within the next two weeks (until 2014-12-29 15:30) I'll close this ticket so it doesn't clutter the bug tracker.

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

[CapnBry]

I love cookies. I truly do.

1. What were you doing?
   Minding my own business when I suddenly the paparazzi were everywhere. I was trying to upload a file to Octoprint at the time. I didn't realize how famous it would make me.
2. What did you expect to happen?
   The file uploaded to be a copy of my local file without all the bells and whistles.
3. What happened instead?
   The file got bells and whistles. Not the chr(7) kind of bell, but the chr(13) and chr(10) kind. These characters were appended to the end of the uploaded file.
4. Branch & Commit or Version:
   comrefactoring branch a0a6f95 revisison (latest as of right now), but this is also present on the devel branch.
5. Printer model & used firmware incl. version
   None printer. A nonebot.
6. Link to octoprint.log on gist.github.com or pastebin.com:
   No u.
7. Link to contents of terminal tab on gist.github.com or pastebin.com:
   Why u no want no log file! This is internal, yo.

[foosel]

<3 That's an awesome bug report! Will take a closer look. To be honest, I'm always scared of fumbling around with that stuff. Remember that horrible Heisenbug that you unearthed and I hunted down and killed back in August? That was also there...

[CapnBry]

I do remember! I never would have found that bug. This is wicked straight-forward as far as reproducing goes. Just hexdump a file before upload and after and you'll see it has the extra \r\n on it.

It makes sense looking at the code too, it reads everything to the mime barrier but it is always preceded by \r\n which the code doesn't remove so whatever the German equivalent of "Bob's your uncle" is. I was just concerned about the rebuilding of the message now without the extra line delimiters maybe affecting other code where you've come to expect it there. I've been using it all week and have played around changing settings (which would also be POSTed right?) and it all seems to be ok. You know your internals better than I do though. Sometimes I can't figure out how some bit of code magically runs out of nowhere by design. Because: FUS RO...CTOPRINT!

[foosel]

There was still an issue when writing the body back in case of file uploads which caused the included form parameters to not be written out correctly (because here suddenly a \r\n was missing, I wonder why ;)) so I had to fix that but other than that your patch worked flawlessly! You would only have noticed that issue by using the fileupload API directly, because then it just concatenated all parameters into one since the CRLF was missing in front of the boundary. Adding that back fixed it just fine.

So big thanks for spotting this! Now the md5sums of uploaded files match their original ones too :D

[CapnBry]

Freakin' sweet. See I knew you'd find that my quick fix broke something else. We're the best team, something is broken, I break something else, you fix them both. TEAMWORK

[foosel]

One day we really need to have a drink together to celebrate this ;)