yegor256 / takes

True Object-Oriented Java Web Framework without NULLs, Static Methods, Annotations, and Mutable Objects
https://www.takes.org
MIT License
806 stars 197 forks source link

RqMultipart is not supporting multiple file transfer #141

Closed ghost closed 5 years ago

ghost commented 9 years ago

There is no code to produce valid request collections on multiple file transfer.

yegor256 commented 9 years ago

@d-molotchko I'm not sure I follow.. could you please explain more and provide some Java code sample that fails at the moment

ghost commented 9 years ago

@yegor256 Quote from http://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.2 is

If multiple files are to be returned as the result of a single form entry, they should be returned as "multipart/mixed" embedded within the "multipart/form-data".

But I can't reproduce this behavior with chrome 40 and iceweasel 31.4

yegor256 commented 9 years ago

@d-molotchko I still don't understand where is our bug. How can I reproduce it? Show me the Java code please.

ghost commented 9 years ago

I can't generate request with nested sections in my browsers, maybe this behavior is out of date. There is syntetic test for nested sections

public final class RqMultipartTest {

    /**
     * RqMultipart can parse body.
     * @throws IOException If some problem inside
     */
    @Test
    public void parsesHttpBody() throws IOException {
        final Request req = new RqFake(
            Arrays.asList(
                "POST /h?a=3 HTTP/1.1",
                "Host: www.example.com",
                "Content-Type: multipart/form-data; boundary=AaB03x",
                "Content-Length: 10000"
            ),
            Joiner.on("\r\n").join(
                "--AaB03x",
                "Content-Disposition: form-data; name=\"address\"",
                "",
                "440 N Wolfe Rd, Sunnyvale, CA 94085",
                "--AaB03x",
                // @checkstyle LineLength (1 line)
                "Content-Disposition: form-data; name=\"files\"",
                "Content-Type: multipart/mixed; boundary=BbC04y",
                "",
                "--BbC04y",
                "Content-Disposition: file; filename=\"file1.txt\"",
                "Content-Type: text/plain",
                "",
                "first file content",
                "--BbC04y",
                "Content-Disposition: file; filename=\"file2.gif\"",
                "Content-Type: text/plain",
                "",
                "second file content",
                "--BbC04y--",
                "--AaB03x--"
            )
        );
        final RqMultipart multi = new RqMultipart(req);
        final Iterator<Request> it = multi.part("files").iterator();
        MatcherAssert.assertThat(
            new RqPrint(
                it.next()
            ).printBody(),
            Matchers.equalTo("first file content")
        );
        MatcherAssert.assertThat(
                new RqPrint(
                    it.next()
                ).printBody(),
                Matchers.equalTo("second file content")
            );
    }
}
yegor256 commented 9 years ago

@d-molotchko looks like we need a new class RqMultipartMixed for parsing these requests and it would work like this:

Request files = multi.part("files").iterator().next();
Request first = new RqMultipartMixed(files).part("file1.txt").iterator().next();
Request second = new RqMultipartMixed(files).part("file2.gif").iterator().next();

make sense?

ghost commented 9 years ago

@yegor256 maybe like this?

Request files = multi.part("files").iterator().next();
Iterator<Request> it = new RqMultipartMixed(files).iterator();
Request first = it.next();
Request second = it.next();

There are no file names in compile time .

yegor256 commented 9 years ago

@d-molotchko yes, right

yegor256 commented 9 years ago

@d-molotchko I think that in order to reproduce this request in a browser we should use multiple attribute (see http://www.w3.org/TR/html-markup/input.file.html):

<input type="file" name="foo" multiple />
ghost commented 9 years ago

@yegor256 I try this

<INPUT type="file" name="file[]" multiple >

but fail

yegor256 commented 9 years ago

@d-molotchko what you mean by "fail"? what is the request you're getting from the browser?

ghost commented 9 years ago

"fail" is "I didn't get multipart/mixed content from browser". Chrome and Iceweasel returns multiple files without multipart/mixed section.

yegor256 commented 9 years ago

@d-molotchko can you post an entire request here?

ghost commented 9 years ago
POST / HTTP/1.1
Host: localhost
Connection: keep-alive
Content-Length: 4204
Content-Type: multipart/form-data; boundary=----WebKitFormBoundaryYAyERkBAWwbSXVxh

------WebKitFormBoundaryYAyERkBAWwbSXVxh
Content-Disposition: form-data; name="submit-name"

John
------WebKitFormBoundaryYAyERkBAWwbSXVxh
Content-Disposition: form-data; name="file[]"; filename="RqMethodTest.java"
Content-Type: text/x-java

------WebKitFormBoundaryYAyERkBAWwbSXVxh
Content-Disposition: form-data; name="file[]"; filename="RqWithoutHeaderTest.java"
Content-Type: text/x-java

------WebKitFormBoundaryYAyERkBAWwbSXVxh--
yegor256 commented 9 years ago

@d-molotchko maybe this multipart/mixed packaging is outdated and shouldn't be supported by us? Why did you decide to report this issue? How did you get stuck on it?

ghost commented 9 years ago

@yegor256 maybe, I don't know. I decide to report this issue because refactor RqMultipart code and see request example on link above. HTML 4.01 is pretty actual version of standard. Any way, you decide what to do with it. Leave as is. New issue is created in few minutes.

yegor256 commented 9 years ago

@d-molotchko let's keep this ticket open for some time and see whether anyone else will report a similar problem. For now I don't see a reason to implement all this, since Chrome is not using this multipart/mixed content type.

paulodamaso commented 5 years ago

@d-molotchko I'm closing this one as no one reported a similar problem in the last two years.

0crat commented 5 years ago

Job gh:yegor256/takes#141 is not assigned, can't get performer

0crat commented 5 years ago

This job is not in scope