Issue 218993016: Make MultipartRequest more closely adhere to browsers' behavior.

Issue 218993016: Make MultipartRequest more closely adhere to browsers' behavior. (Closed)

Created:
6 years, 8 months ago by nweiz

Modified:
6 years, 8 months ago

Reviewers:
Bob Nystrom

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Make MultipartRequest more closely adhere to browsers' behavior. R=rnystrom@google.com BUG=17899 Committed: https://code.google.com/p/dart/source/detail?r=34631

Patch Set 2 : code review #

Created: 6 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+78 lines, -12 lines)			Patch
M	pkg/http/CHANGELOG.md	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	pkg/http/lib/src/multipart_request.dart	View	1	5 chunks	+18 lines, -10 lines	0 comments	Download
M	pkg/http/test/multipart_test.dart	View		2 chunks	+58 lines, -2 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Bob Nystrom

Couple of nits then LGTM. https://codereview.chromium.org/218993016/diff/1/pkg/http/lib/src/multipart_request.dart File pkg/http/lib/src/multipart_request.dart (right): https://codereview.chromium.org/218993016/diff/1/pkg/http/lib/src/multipart_request.dart#newcode162 pkg/http/lib/src/multipart_request.dart:162: // `\r\n`; URL-encode `"`; ...

6 years, 8 months ago (2014-04-01 19:45:26 UTC) #2

nweiz

Committed patchset #2 manually as r34631 (presubmit successful).

6 years, 8 months ago (2014-04-02 01:08:57 UTC) #3

nweiz

6 years, 8 months ago (2014-04-02 01:09:43 UTC) #4

Message was sent while issue was closed.

https://codereview.chromium.org/218993016/diff/1/pkg/http/lib/src/multipart_r...
File pkg/http/lib/src/multipart_request.dart (right):

https://codereview.chromium.org/218993016/diff/1/pkg/http/lib/src/multipart_r...
pkg/http/lib/src/multipart_request.dart:162: // `\r\n`; URL-encode `"`; and do
thing else (even for `%` or non-ASCII
On 2014/04/01 19:45:26, Bob Nystrom wrote:
> thing -> nothing.

Done.

https://codereview.chromium.org/218993016/diff/1/pkg/http/test/multipart_test...
File pkg/http/test/multipart_test.dart (right):

https://codereview.chromium.org/218993016/diff/1/pkg/http/test/multipart_test...
pkg/http/test/multipart_test.dart:122: content-disposition: form-data;
name="foo%22bar"
On 2014/04/01 19:45:26, Bob Nystrom wrote:
> Is it worth also testing that a single quote *isn't* encoded?

I don't think so. There's nothing special about a single quote anywhere in HTTP.

Expand Messages | Collapse Messages