-
-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
unexpected line in Next(): #1688
Comments
@bussiere what version of ipfs (git sha if you can), OS and archictecture? |
Also, is there anything weird about the gallery? such as uf8 file names or strange file types |
Ubuntu 14.04 go 1.5 amd64 And yes in the gallerie it may have some french character as ç or japanese one. Thanks |
That's strange because if i try to add it folder by folder it seems to works : |
@bussiere do you mind zipping up a set of files that can repro the issue? |
yep the zip is here : QmfYQRPWworETuBudtayhkmCAMxmz8LV3YXm3KXwytyrfM and i've isolated the error ::
thanks |
multipart blowing up? |
Which version is the gobuilder one? 0.3.7 or 0.3.8-dev? — On Thu, Sep 17, 2015 at 5:07 PM, Johan Kiviniemi [email protected]
|
The gobuilder version I downloaded came from the URL below and was downloaded approximately at 5:00 PM EST today. THe .zip archive has a timestamp of "Sept 16 02:41", and it identifies itself as This version string is identical to the string produced by the working version of IPFS that doesn't reproduce this issue https://gobuilder.me/get/github.com/ipfs/go-ipfs/cmd/ipfs/ipfs_master_linux-amd64.zip |
SIgh, ok. i worried we'd run into a case like this soon enough. we need to start putting git commit short hashes into the version string ( @whyrusleeping thoughts here? |
I'm getting the same error. I pulled a fresh version a few minutes ago with I tried to attach a file that causes the error, but github doesn't seem to like it either. Here's the base64 encoded gzip of it: To recreate the file, do:
I experimented a bit with how much I could 'simplify' the file while still getting the error, in order to try to narrow down what's happening. The following seems not to affect whether the error occurs:
But the following does:
Replacing an unusual character with another unusual character sometimes affects it, and sometimes doesn't. I haven't worked out the pattern. I've identified the following characters as 'unusual':
I assume there are many others, which I haven't bothered to look for. |
Could this be related to #1582 ? |
Possible, but I don't see how bad UTF-8 handling would explain the fact that the crash goes away when I change the leading characters from |
To be honest I'm more surprised that ipfs doesn't crash on binary files more often given the broken json codec. |
@davidar broken json codec? i may be missing some context here |
@whyrusleeping the issue I just linked, with the json encoding corrupting binary data |
how common is this for you @sonatagreen ? |
It's only happened to me once so far.
|
Ran into it again today, so twice now. One-liner to reproduce:
|
i ran into this today. i didn't save the file that triggered this error |
I ran into the same issue when experimenting with IPFS. I started to dig into it a bit and the interesting thing is that it appears to depend on file size. I can reproduce this behaviour for any file of size n*4032-1 (where n is an integer > 0).
The hex dump of testfile:
The hex dump of testfile_cat:
Considering the "boundary-line" of the mime-multipart thing appears to be of length 62, together with two characters \r and \n this adds up to 64. Add this to the magic number 4032 (see above) and you have 4096. This happens to be the same as the peek buffer size in go's multipart package (https://golang.org/src/mime/multipart/multipart.go). I haven't had the time to pinpoint the exact cause, but I suspect the cause of this bug is hiding in the complexity of (the peek buffer part of) this multipart package. Also considering the diff between the original and the retrieved file: it is 4 bytes longer, exactly '\r\n--'. Perhaps the boundary-line is not properly detected in these cases? Anyway, perhaps this is another data-point that can help in resolving this. |
@mhe great! could you contribute a sharness test case so we can make it pass? |
@jbenet Sure, sounds like an interesting exercise :) I've attached a sharness test to this comment (I thought an PR would be a bit overkill). |
Ok thanks @mhe -- we should make a PR with a fix that includes that test case. |
I assume this is fixed as of |
@travisperson ah yes! thanks, i can close this now. |
I'am currently manipulating pictures and gallery of pictures :
Seems kind of backslash plague with \n\
The text was updated successfully, but these errors were encountered: