Cannot camput bytes - get Unexpected EOF error #642
Comments
|
I haven't checked yet, but the .tmpxxx suffix suggests me that it could be an incomplete blob, hence the error ? |
|
@mpl What does an incomplete blob mean? I thought a blob was just bytes - shouldn't I be able to |
|
@dbp disregard that last comment, lemme look into it. Still, camput blob is not meant to do what I'm guessing you want to do here. Regardless, you're right that you should be able to give any file to camput blob without crashing it. It's somewhat similar to issue #248 |
|
Just to follow up a little - I did a little digging (I've never written any Go code, so forgive any silly misunderstandings). I tracked the error down to happening within the call to copy into the tempfile here: Looking at the stack at that point, in order to figure out what Which means the reader is a few wrappers around the This is starting to sound suspiciously like issue #4.... Or maybe some new variant of it (it looks like the unexpected EOF error is coming from the I don't understand multipart encoding well, so I'm not sure if I'll be able to make much sense out of what is wrong, but by setting And then errors out at the end of that function. |
|
yes, I arrived to the same suspicions. I'm investigating the stdlib. |
|
I believe I found the bug, see: |
|
@mpl Thanks! |
|
@dbp thank you for reporting it. |
The case fixed by this change happens when, in func (pr partReader) Read, the Peek happens to read so that peek looks like: "somedata\r\n--Boundary\r" peekBufferSeparatorIndex was returning (-1, false) because it didn't find the trailing '\n'. This was wrong because: 1) It didn't match the documentation: as "\r\n--Boundary" was found, it should return the index of that pattern, not -1. 2) It lead to an nCopy cut such as: "somedata\r| |\n--Boundary\r" instead of "somedata| |\r\n--Boundary\r" which made the subsequent Read miss the boundary, and eventually end with a "return 0, io.ErrUnexpectedEOF" case, as reported in: perkeep/perkeep#642 Change-Id: I1ba78a741bc0c7719e160add9cca932d10f8a615 Reviewed-on: https://go-review.googlesource.com/15269 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
|
fixed at 821b54921a3cba5d853b531d4b03527c01bfc9b4 in Go tree. |
|
Wait, shouldn't we vendor mime/multipart into Camlistore for now? And then open a "Things to fix in Camlistore when Go depends on Go 1.6" bug? |
|
ugh, yes, sorry. Since I've switched to gotip to fix that bug I had forgotten we're supposed to run on Go 1.5. |
|
@bradfitz I'll confirm tomorrow the reason, but I'm having some trouble getting Camlistore to use the vendored version. I can confirm that I vendored it properly because when I add a dummy var to the source of the vendored code, I can set/access that dummy var, say in camlistored.go (and adding a "mime/multipart" import in camlistored.go). However, it seems like, since in pkg/blobserver/handlers/upload.go we don't use the multipart pkg directly, but we instead get a Reader through "multipart, err := req.MultipartReader()", in that case we get the buggy code from Go 1.5, because req from net/http was built against the mime/multipart of Go 1.5. At least that's my suspicion. |
|
@bradfitz I think I was able to confirm my suspicion above, i.e. that since we're getting the multipart Reader from net/http, we're getting the (buggy) one from Go 1.5. Suggestion? We don't want to vendor net/http as well now, do we ? |
|
Oh, I see. Gross. |
|
I flagged the issue as a potential Go 1.5.2 backport. We'll see how others feel. |
|
So we have to wait for Go 1.5.2? No other workaround possible? In this state camlistore is pretty useless to me... |
|
@sherter uh, just use go tip ? Or even easier, just patch your Go 1.5 source tree as in https://go-review.googlesource.com/15269 ; it's a 3 chars fix... Also, I don't think the odds of hitting that bug are very large, as it requires a file of a very specific size. You could just not upload such files if that occurs. |
|
I hit this many times when trying to sync between camlistore servers (I On Tuesday, October 13, 2015, mpl notifications@github.com wrote:
|
|
Fair enough. But my first suggestions still stand :-) |
|
@mpl Definitely. And it's also perfectly usable for me. I just have to use just one server :) |
At a2119aca7dc82dc5b5cd40b1a2f56e82323da002 in go tip because we want the bugfix at 821b54921a3cba5d853b531d4b03527c01bfc9b4 We could legitimately vendor as "vendor/mime/multipart" and shadow the stdlib's but we do it in future for clarity. Issue perkeep#642 Change-Id: Ifddbd4c9120936b8acc2f6ae31a97b1831b99f34
…rt Reader We redefine a MultipartReader(*http.Request) function to use instead of req.MultipartReader, so we can get a (bugfixed) multipart Reader from our vendored mime/multipart, instead of the buggy one from the stdlib's. Fixes issue perkeep#642 Change-Id: I6a205bff915632d4ee77547e6e26bc0af99665e9
revert changes afe28eb and 666d525 because Go 1.6 is out. Issues perkeep#642 and perkeep#644 Change-Id: Ide422b5164576c77d72061bb05ea0984c68d55e4
I see an old, fixed issue (#4) that says that there used to be an issue where files without linefeeds would trigger this error. I'm not sure if it's the same issue (I see this error both with 0.8 and HEAD, built with go 1.5.1 on mac), but trying to add a blob with the contents at http://s3.dbp.io/misc/camli-eof-2015-10-2 (I'm not sure what this is - it came out of another camliserver, presumably it is a part of an image) causes the following error on the server:
and correspondingly on the client:
I noticed this error because I was trying to sync from a blobserver on another computer to this one, and I was getting lots of these errors in syncing (and the result of the sync was messed up - lots of permanodes with no claims, etc). So I grabbed one sha (to try to figure out what went wrong), and checked and it existed on the old server, and I could
camgetit, which is where the file I linked to above came from. But trying tocamput blobit causes the above error.The text was updated successfully, but these errors were encountered: