Add new HttpMultipartFormData, used for parsing a MimeMultipart and extracting either text or binar…

Add new HttpMultipartFormData, used for parsing a MimeMultipart and extracting either text or binary data.

BUG=
R=sgjesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=22414

[Anders Johnsen, 2013-05-06 11:03:06]

This is an early review of this change, to get a bit of API feedback.

[Søren Gjesse, 2013-05-06 12:28:10]

The API looks fine.

https://codereview.chromium.org/14796015/diff/2001/sdk/lib/io/http_multipart_...
File sdk/lib/io/http_multipart_form_data.dart (right):

https://codereview.chromium.org/14796015/diff/2001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data.dart:7: 
Some documentation is needed here.

https://codereview.chromium.org/14796015/diff/2001/sdk/lib/io/http_multipart_...
File sdk/lib/io/http_multipart_form_data_impl.dart (right):

https://codereview.chromium.org/14796015/diff/2001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data_impl.dart:24: if (contentTransferEncoding !=
null) {
Throw here for now?

https://codereview.chromium.org/14796015/diff/2001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data_impl.dart:32: _stream = _stream
Use the content type charset for decoding if provided.

https://codereview.chromium.org/14796015/diff/2001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data_impl.dart:34: .expand((data) {
What if the HTML entites cross data boundaries?

https://codereview.chromium.org/14796015/diff/2001/tests/standalone/io/http_m...
File tests/standalone/io/http_multipart_test.dart (right):

https://codereview.chromium.org/14796015/diff/2001/tests/standalone/io/http_m...
tests/standalone/io/http_multipart_test.dart:3: // BSD-style license that can be
found in the LICENSE file.
Add testing with short read/write.

// VMOptions=
// VMOptions=--short_socket_read
// VMOptions=--short_socket_write
// VMOptions=--short_socket_read --short_socket_write

https://codereview.chromium.org/14796015/diff/2001/tests/standalone/io/mime_m...
File tests/standalone/io/mime_multipart_parser_test.dart (right):

https://codereview.chromium.org/14796015/diff/2001/tests/standalone/io/mime_m...
tests/standalone/io/mime_multipart_parser_test.dart:283: // Without initial
CRLF. One body part less.
This comment looks wrong.

[Anders Johnsen, 2013-05-06 13:38:16]

PTAL

https://codereview.chromium.org/14796015/diff/2001/sdk/lib/io/http_multipart_...
File sdk/lib/io/http_multipart_form_data.dart (right):

https://codereview.chromium.org/14796015/diff/2001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data.dart:7: 
On 2013/05/06 12:28:10, Søren Gjesse wrote:
> Some documentation is needed here.

Done.

https://codereview.chromium.org/14796015/diff/2001/sdk/lib/io/http_multipart_...
File sdk/lib/io/http_multipart_form_data_impl.dart (right):

https://codereview.chromium.org/14796015/diff/2001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data_impl.dart:24: if (contentTransferEncoding !=
null) {
On 2013/05/06 12:28:10, Søren Gjesse wrote:
> Throw here for now?

Done.

https://codereview.chromium.org/14796015/diff/2001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data_impl.dart:32: _stream = _stream
On 2013/05/06 12:28:10, Søren Gjesse wrote:
> Use the content type charset for decoding if provided.

Done.

https://codereview.chromium.org/14796015/diff/2001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data_impl.dart:34: .expand((data) {
On 2013/05/06 12:28:10, Søren Gjesse wrote:
> What if the HTML entites cross data boundaries?

That's what the 'null' check is for. It returns null if it's on the boundary,
and thus we store it for next chunk.

https://codereview.chromium.org/14796015/diff/2001/tests/standalone/io/http_m...
File tests/standalone/io/http_multipart_test.dart (right):

https://codereview.chromium.org/14796015/diff/2001/tests/standalone/io/http_m...
tests/standalone/io/http_multipart_test.dart:3: // BSD-style license that can be
found in the LICENSE file.
On 2013/05/06 12:28:10, Søren Gjesse wrote:
> Add testing with short read/write.
> 
> // VMOptions=
> // VMOptions=--short_socket_read
> // VMOptions=--short_socket_write
> // VMOptions=--short_socket_read --short_socket_write

Done.

https://codereview.chromium.org/14796015/diff/2001/tests/standalone/io/mime_m...
File tests/standalone/io/mime_multipart_parser_test.dart (right):

https://codereview.chromium.org/14796015/diff/2001/tests/standalone/io/mime_m...
tests/standalone/io/mime_multipart_parser_test.dart:283: // Without initial
CRLF. One body part less.
On 2013/05/06 12:28:10, Søren Gjesse wrote:
> This comment looks wrong.

Done.

[Søren Gjesse, 2013-05-06 13:46:34]

lgtm

https://codereview.chromium.org/14796015/diff/9001/sdk/lib/io/http_multipart_...
File sdk/lib/io/http_multipart_form_data.dart (right):

https://codereview.chromium.org/14796015/diff/9001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data.dart:11: * it can be used.
What code?

https://codereview.chromium.org/14796015/diff/9001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data.dart:19: * The parsed [:Content-Type:]
heaver of the [:HttpMultipartFormData:].
heaver?

https://codereview.chromium.org/14796015/diff/9001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data.dart:26: * This field is always present. Use
this extract e.g. name and filename
Use this -> Use this to

https://codereview.chromium.org/14796015/diff/9001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data.dart:26: * This field is always present. Use
this extract e.g. name and filename
name -> name (form field name)

https://codereview.chromium.org/14796015/diff/9001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data.dart:26: * This field is always present. Use
this extract e.g. name and filename
filename -> filename (client provided name of uploaded file)

[Anders Johnsen, 2013-05-06 14:07:55]

https://codereview.chromium.org/14796015/diff/9001/sdk/lib/io/http_multipart_...
File sdk/lib/io/http_multipart_form_data.dart (right):

https://codereview.chromium.org/14796015/diff/9001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data.dart:11: * it can be used.
On 2013/05/06 13:46:34, Søren Gjesse wrote:
> What code?

Done.

https://codereview.chromium.org/14796015/diff/9001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data.dart:19: * The parsed [:Content-Type:]
heaver of the [:HttpMultipartFormData:].
On 2013/05/06 13:46:34, Søren Gjesse wrote:
> heaver?

Done.

https://codereview.chromium.org/14796015/diff/9001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data.dart:26: * This field is always present. Use
this extract e.g. name and filename
On 2013/05/06 13:46:34, Søren Gjesse wrote:
> Use this -> Use this to

Done.

https://codereview.chromium.org/14796015/diff/9001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data.dart:26: * This field is always present. Use
this extract e.g. name and filename
On 2013/05/06 13:46:34, Søren Gjesse wrote:
> name -> name (form field name)

Done.

https://codereview.chromium.org/14796015/diff/9001/sdk/lib/io/http_multipart_...
sdk/lib/io/http_multipart_form_data.dart:26: * This field is always present. Use
this extract e.g. name and filename
On 2013/05/06 13:46:34, Søren Gjesse wrote:
> filename -> filename (client provided name of uploaded file)

Done.

[Anders Johnsen, 2013-05-06 14:13:07]

Committed patchset #4 manually as r22414 (presubmit successful).

[nweiz, 2013-05-08 18:06:18]

On 2013/05/06 11:03:06, Anders Johnsen wrote:
> This is an early review of this change, to get a bit of API feedback.

If you change a name (e.g. MimeParserException -> MimeMultipartException),
please update external references to that name in the repo. This change broke
some of pub's error-handling logic.