Title: [106514] trunk/Source/WebCore
- Revision
- 106514
- Author
- [email protected]
- Date
- 2012-02-01 16:14:55 -0800 (Wed, 01 Feb 2012)
Log Message
contentDispositionType misparses the Content-Disposition header in some obscure corner cases
https://bugs.webkit.org/show_bug.cgi?id=77577
Reviewed by Eric Seidel.
The contentDispositionType extracts the disposition-type from the
Content-Disposition header. According to RFC 6266 (and previous RFCs),
the disposition-type must be an RFC 2616 token. Rather than enforce
this general rule, we had special-cased some examples (including
name=foo and filename=bar). This patch generalizes our check to
properly validate that the disposition-type is an RFC 2616 token.
In conjunction with some other work in the Chromium network stack, this
causes Chromium to pass the following tests:
http://greenbytes.de/tech/tc2231/#inlonlyquoted
http://greenbytes.de/tech/tc2231/#attonlyquoted
Without this patch, these test cases neither trigger a navigation nor a
download in Chromium. This patch does not appear to cause any visible
change in Safari. (Safari passes these tests both before and after
this patch.)
* platform/network/HTTPParsers.cpp:
(WebCore::isRFC2616Token):
(WebCore::contentDispositionType):
- This patch also adds a comment to
filenameFromHTTPContentDisposition, which explains some of the
was this function incorrectly implements the requirements in
RFC 6266. Resolving these issues is a subject for a future
patch.
* platform/network/HTTPParsers.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (106513 => 106514)
--- trunk/Source/WebCore/ChangeLog 2012-02-02 00:09:27 UTC (rev 106513)
+++ trunk/Source/WebCore/ChangeLog 2012-02-02 00:14:55 UTC (rev 106514)
@@ -1,3 +1,38 @@
+2012-02-01 Adam Barth <[email protected]>
+
+ contentDispositionType misparses the Content-Disposition header in some obscure corner cases
+ https://bugs.webkit.org/show_bug.cgi?id=77577
+
+ Reviewed by Eric Seidel.
+
+ The contentDispositionType extracts the disposition-type from the
+ Content-Disposition header. According to RFC 6266 (and previous RFCs),
+ the disposition-type must be an RFC 2616 token. Rather than enforce
+ this general rule, we had special-cased some examples (including
+ name=foo and filename=bar). This patch generalizes our check to
+ properly validate that the disposition-type is an RFC 2616 token.
+
+ In conjunction with some other work in the Chromium network stack, this
+ causes Chromium to pass the following tests:
+
+ http://greenbytes.de/tech/tc2231/#inlonlyquoted
+ http://greenbytes.de/tech/tc2231/#attonlyquoted
+
+ Without this patch, these test cases neither trigger a navigation nor a
+ download in Chromium. This patch does not appear to cause any visible
+ change in Safari. (Safari passes these tests both before and after
+ this patch.)
+
+ * platform/network/HTTPParsers.cpp:
+ (WebCore::isRFC2616Token):
+ (WebCore::contentDispositionType):
+ - This patch also adds a comment to
+ filenameFromHTTPContentDisposition, which explains some of the
+ was this function incorrectly implements the requirements in
+ RFC 6266. Resolving these issues is a subject for a future
+ patch.
+ * platform/network/HTTPParsers.h:
+
2012-02-01 Dan Bernstein <[email protected]>
WebCore part of <rdar://problem/10442663> Paginated display does not respect page-break-{before,after}
Modified: trunk/Source/WebCore/platform/network/HTTPParsers.cpp (106513 => 106514)
--- trunk/Source/WebCore/platform/network/HTTPParsers.cpp 2012-02-02 00:09:27 UTC (rev 106513)
+++ trunk/Source/WebCore/platform/network/HTTPParsers.cpp 2012-02-02 00:14:55 UTC (rev 106514)
@@ -73,30 +73,47 @@
return true;
}
+// See RFC 2616, Section 2.2.
+bool isRFC2616Token(const String& characters)
+{
+ if (characters.isEmpty())
+ return false;
+ for (unsigned i = 0; i < characters.length(); ++i) {
+ UChar c = characters[i];
+ if (c >= 0x80 || c <= 0x1F || c == 0x7F
+ || c == '(' || c == ')' || c == '<' || c == '>' || c == '@'
+ || c == ',' || c == ';' || c == ':' || c == '\\' || c == '"'
+ || c == '/' || c == '[' || c == ']' || c == '?' || c == '='
+ || c == '{' || c == '}' || c == ' ' || c == '\t')
+ return false;
+ }
+ return true;
+}
+
ContentDispositionType contentDispositionType(const String& contentDisposition)
-{
+{
if (contentDisposition.isEmpty())
return ContentDispositionNone;
- // Some broken sites just send
- // Content-Disposition: ; filename="file"
- // screen those out here.
- if (contentDisposition.startsWith(";"))
- return ContentDispositionNone;
+ Vector<String> parameters;
+ contentDisposition.split(';', parameters);
- if (contentDisposition.startsWith("inline", false))
+ String dispositionType = parameters[0];
+ dispositionType.stripWhiteSpace();
+
+ if (equalIgnoringCase(dispositionType, "inline"))
return ContentDispositionInline;
- // Some broken sites just send
- // Content-Disposition: filename="file"
+ // Some broken sites just send bogus headers like
+ //
+ // Content-Disposition: ; filename="file"
+ // Content-Disposition: filename="file"
+ // Content-Disposition: name="file"
+ //
// without a disposition token... screen those out.
- if (contentDisposition.startsWith("filename", false))
+ if (!isRFC2616Token(dispositionType))
return ContentDispositionNone;
- // Also in use is Content-Disposition: name="file"
- if (contentDisposition.startsWith("name", false))
- return ContentDispositionNone;
-
// We have a content-disposition of "attachment" or unknown.
// RFC 2183, section 2.8 says that an unknown disposition
// value should be treated as "attachment"
@@ -167,6 +184,11 @@
return parseDateFromNullTerminatedCharacters(value.utf8().data());
}
+// FIXME: This function doesn't comply with RFC 6266.
+// For example, this function doesn't handle the interaction between " and ;
+// that arises from quoted-string, nor does this function properly unquote
+// attribute values. Further this function appears to process parameter names
+// in a case-sensitive manner. (There are likely other bugs as well.)
String filenameFromHTTPContentDisposition(const String& value)
{
Vector<String> keyValuePairs;
Modified: trunk/Source/WebCore/platform/network/HTTPParsers.h (106513 => 106514)
--- trunk/Source/WebCore/platform/network/HTTPParsers.h 2012-02-02 00:09:27 UTC (rev 106513)
+++ trunk/Source/WebCore/platform/network/HTTPParsers.h 2012-02-02 00:14:55 UTC (rev 106514)
@@ -50,6 +50,7 @@
} ContentDispositionType;
ContentDispositionType contentDispositionType(const String&);
+bool isRFC2616Token(const String&);
bool parseHTTPRefresh(const String& refresh, bool fromHttpEquivMeta, double& delay, String& url);
double parseDate(const String&);
String filenameFromHTTPContentDisposition(const String&);
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
