Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(61)

Issue 23043002: Relax Content-Type parameter parsing (Closed)

Created:
7 years, 4 months ago by Tiger
Modified:
7 years, 2 months ago
CC:
blink-reviews, eae+blinkwatch, dglazkov+blink, jeez
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Relax Content-Type parameter parsing Unfortunately Opera 12 (this could possibly be found in files generated by other products as well, or by hand) and below does not write out its Content-Type parameters according to the standard (start parameter contains tspecials without being an unquoted-string) in some MHTML files. This patch relaxes the parsing of Content-Type parameters to allow this. R=tony@chromium.org BUG=274407 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=158314

Patch Set 1 #

Patch Set 2 : Adding tests (I've heard this might work now) #

Total comments: 2

Patch Set 3 : Review fixes (escaping) #

Total comments: 1

Patch Set 4 : Review fixes #

Patch Set 5 : Rebase #

Patch Set 6 : Test fixes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -45 lines) Patch
A LayoutTests/mhtml/relaxed-content-type-parameters.mht View 1 Binary file 0 comments Download
A + LayoutTests/mhtml/relaxed-content-type-parameters.mhtml_original View 1 2 Binary file 0 comments Download
A LayoutTests/mhtml/relaxed-content-type-parameters-expected.txt View 1 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/platform/network/ParsedContentType.cpp View 1 2 3 4 5 3 chunks +17 lines, -45 lines 0 comments Download

Messages

Total messages: 26 (0 generated)
Tiger
7 years, 4 months ago (2013-08-13 11:18:56 UTC) #1
tony
I am not an appropriate reviewer for this patch. Changing the reviewers to Jay (original ...
7 years, 4 months ago (2013-08-13 17:35:00 UTC) #2
abarth-chromium
We certainly would need a test, but the bigger question is whether we should make ...
7 years, 4 months ago (2013-08-13 19:53:30 UTC) #3
Tiger
On 2013/08/13 19:53:30, abarth wrote: > We certainly would need a test, but the bigger ...
7 years, 4 months ago (2013-08-14 10:17:08 UTC) #4
abarth-chromium
On 2013/08/14 10:17:08, Tiger wrote: > On 2013/08/13 19:53:30, abarth wrote: > > We certainly ...
7 years, 4 months ago (2013-08-14 19:35:40 UTC) #5
Tiger
Waiting for a fix/solution to the MHTML file uploading problems in https://chromiumcodereview.appspot.com/22292008/ before uploading MTHML ...
7 years, 4 months ago (2013-08-15 10:00:19 UTC) #6
Tiger
The test files need to be committed separately. I've uploaded them to <https://people.opera.com/tiger/patches/relaxed-content-type-parameters.tar.gz>. Could anyone ...
7 years, 3 months ago (2013-09-02 10:42:23 UTC) #7
Tiger
On 2013/09/02 10:42:23, Tiger wrote: > The test files need to be committed separately. I've ...
7 years, 3 months ago (2013-09-12 08:23:37 UTC) #8
Tiger
Adding more reviewers, could anyone of you @tkent, @jochen, @jchaffraix take a look a this?
7 years, 3 months ago (2013-09-12 08:26:31 UTC) #9
jochen (gone - plz use gerrit)
On 2013/09/12 08:26:31, Tiger wrote: > Adding more reviewers, could anyone of you @tkent, @jochen, ...
7 years, 3 months ago (2013-09-12 09:46:21 UTC) #10
abarth-chromium
https://chromiumcodereview.appspot.com/23043002/diff/10001/Source/core/platform/network/ParsedContentType.cpp File Source/core/platform/network/ParsedContentType.cpp (left): https://chromiumcodereview.appspot.com/23043002/diff/10001/Source/core/platform/network/ParsedContentType.cpp#oldcode67 Source/core/platform/network/ParsedContentType.cpp:67: if (!isTokenCharacter(input[tokenEnd])) I see. The key is that you're ...
7 years, 3 months ago (2013-09-12 18:16:11 UTC) #11
Tiger
On 2013/09/12 18:16:11, abarth wrote: > https://chromiumcodereview.appspot.com/23043002/diff/10001/Source/core/platform/network/ParsedContentType.cpp > File Source/core/platform/network/ParsedContentType.cpp (left): > > https://chromiumcodereview.appspot.com/23043002/diff/10001/Source/core/platform/network/ParsedContentType.cpp#oldcode67 > ...
7 years, 3 months ago (2013-09-16 10:05:03 UTC) #12
abarth-chromium
lgtm https://chromiumcodereview.appspot.com/23043002/diff/20001/Source/core/platform/network/ParsedContentType.cpp File Source/core/platform/network/ParsedContentType.cpp (right): https://chromiumcodereview.appspot.com/23043002/diff/20001/Source/core/platform/network/ParsedContentType.cpp#newcode64 Source/core/platform/network/ParsedContentType.cpp:64: while (tokenEnd < inputLength) { Rather than calling ...
7 years, 3 months ago (2013-09-16 19:41:41 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tiger@opera.com/23043002/29001
7 years, 3 months ago (2013-09-17 11:21:58 UTC) #14
commit-bot: I haz the power
Failed to apply patch for LayoutTests/TestExpectations: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
7 years, 3 months ago (2013-09-17 11:22:01 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tiger@opera.com/23043002/45001
7 years, 3 months ago (2013-09-17 13:03:00 UTC) #16
commit-bot: I haz the power
Retried try job too often on win_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_rel&number=5963
7 years, 3 months ago (2013-09-17 14:22:13 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tiger@opera.com/23043002/45001
7 years, 3 months ago (2013-09-17 15:35:29 UTC) #18
commit-bot: I haz the power
Retried try job too often on linux_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blink_rel&number=5084
7 years, 3 months ago (2013-09-17 17:00:39 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tiger@opera.com/23043002/45001
7 years, 3 months ago (2013-09-17 22:12:13 UTC) #20
commit-bot: I haz the power
Retried try job too often on linux_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blink_rel&number=5128
7 years, 3 months ago (2013-09-17 23:47:24 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tiger@opera.com/23043002/45001
7 years, 3 months ago (2013-09-18 07:30:26 UTC) #22
commit-bot: I haz the power
Retried try job too often on linux_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blink_rel&number=5170
7 years, 3 months ago (2013-09-18 09:05:11 UTC) #23
Tiger
Added an additional fix to not the last token if it was quoted but didn't ...
7 years, 3 months ago (2013-09-19 10:18:17 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tiger@opera.com/23043002/80001
7 years, 2 months ago (2013-09-25 08:29:04 UTC) #25
commit-bot: I haz the power
7 years, 2 months ago (2013-09-25 09:35:25 UTC) #26
Message was sent while issue was closed.
Change committed as 158314

Powered by Google App Engine
This is Rietveld 408576698