Reading Multi part error

[seaweedfs]

Reading Multi part error

[devinrsmith]

This may be b/c my master and volumes where different versions... updating to latest will update soon.

[devinrsmith]

Updated everything to 0.68 still has issues, going back to 0.67.

[chrislusf]

There is a very recent change on parsing multipart uploads.

[chrislusf]

Related to this? #56

[devinrsmith]

Back to 0.67, things working again.

[yourchanges]

@devinrsmith Could you PLEASE give the request payload or the example to show how the multipart uploads failed?

[yourchanges]

@devinrsmith by the way, the tons of the error logs is needed too.

And our product env run the master bench very well without any issues.

[devinrsmith]

I've rolled back to 0.67 for the time, but I'll see if I can't get a dump of what was sent over the wire later. It should be a pretty standard multipart post I think. I don't think the error logs will help too much as it's just the same thing repeated over and over.

https://gist.github.com/devinrsmith/a55b55e2ac12d50c4559

[yourchanges]

There is a way to cause into "dead-for" loop, the request payload didn't have a 'filename' properties in dispositionParams parsed from the Content-Disposition. Or you just submit a non-multipart form request to server?

for example:

------------gL6KM7cH2ae0Ij5ae0KM7ae0ae0gL6
Content-Disposition: form-data; name="file"; [filename="IMG_1840.JPG" ---miss ]
Content-Type: application/octet-stream

@devinrsmith could you give the some client javascript code(or the command line) to show how to submit the form

[chrislusf]

Right. Previous version allows empty file names.

[yourchanges]

@chrislusf but it seems not RFC(See https://www.ietf.org/rfc/rfc1867.txt ) RECOMMAND if the filename is empty, in the java world "commons-upload.jar" also check the filename to judge a multi-part item DO a file item

And mentioned in #56 , the "Previous version" it will just save the normal form item value into storage(for example: a user name value, even it's not a file), and I think this should be avoid.

So the weedfs should aways fetch the upload file data from a NAMED form item such as : "file"

[chrislusf]

The previous version supports bare blob. This is because weedfs is more a key~blob store, not necessarily a file with filename.

I suggest we still support uploads with empty filename.

[devinrsmith]

Yep, this must be it. I'm not setting filename.

[yourchanges]

But I think the weedfs is a HTTP file server too, it runs well, and it should handle these clients such as jquery uploadify tool's request.

"support uploads with empty filename" is fine, but "save the normal form item value into storage" should be avoid too.

Lets improve it

[chrislusf]

Not saying we should not support other tools. But we still can support empty file names.

[yourchanges]

@chrislusf the improved code: needle.go L64-L84:

    part, fe := form.NextPart()
for {
    if fe != nil {
        glog.V(0).Infoln("Reading Multi part [ERROR]", fe)
        e = fe
        return
    }

    if part.FileName() != "" {
        break //found the first <file type> multi-part
    }

    part2, fe := form.NextPart()

    //no more multi-part items, we take the last multi-part item as the target
    if fe == io.EOF {
        break
    } else {
        part = part2
    }
}

[EDIT: the solution does not work, because the internal reader has been moved to next cased the ioutil.ReadAll(part) method failed]

[chrislusf]

Haven't tested it but looks good. Send a pull request please.

[yourchanges]

ok, I'll do it later, it's 03:14 am, Jan 16 in China, I'm going to sleep now.

[chrislusf]

@devinrsmith Maybe you can confirm the fix?

[devinrsmith]

Has a binary been compiled? I can check it out tomorrow.

[chrislusf]

No compiled binary published yet. But I can send you the binary if you need it. Closing this. My local test seems good.