[webkit-dev] [webkit-changes] [56750] trunk

[Brady Eidson]

Since two platforms now share this same code in their WebKit layers, it seems okay that this was moved into some shared location.  But I have a few gripes:

HTTPParsers is about parsing HTTP, not implementing policy.  A better place is probably ResourceResponse itself.

But as written the method implements a client policy.  This reeks of breaking the layering between WebCore and WebKit.  I would feel a lot better about it if the method was:
String ResourceResponseBase::contentDispositionType() const;

...and then the clients made their own decision based on that.  Safari actually does this, for example.

I'll add these comments to the bug.

~Brady


On Mar 29, 2010, at 4:58 PM, eric at webkit.org wrote:

> Revision
> 56750
> Author
> eric at webkit.org
> Date
> 2010-03-29 16:58:03 -0700 (Mon, 29 Mar 2010)
> Modified: trunk/WebCore/platform/network/HTTPParsers.cpp (56749 => 56750)
> 
> --- trunk/WebCore/platform/network/HTTPParsers.cpp	2010-03-29 23:38:40 UTC (rev 56749)
> +++ trunk/WebCore/platform/network/HTTPParsers.cpp	2010-03-29 23:58:03 UTC (rev 56750)
> @@ -2,6 +2,7 @@
> 
> +bool shouldTreatAsAttachment(const ResourceResponseBase& response)
> +{
> +    const String& contentDisposition = response.httpHeaderField("Content-Disposition");
> +    
> +    if (contentDisposition.isEmpty())
> +        return false;
> +
> +    // Some broken sites just send
> +    // Content-Disposition: ; filename="file"
> +    // screen those out here.
> +    if (contentDisposition.startsWith(";"))
> +        return false;
> +
> +    if (contentDisposition.startsWith("inline", false))
> +        return false;
> +
> +    // Some broken sites just send
> +    // Content-Disposition: filename="file"
> +    // without a disposition token... screen those out.
> +    if (contentDisposition.startsWith("filename", false))
> +        return false;
> +
> +    // Also in use is Content-Disposition: name="file"
> +    if (contentDisposition.startsWith("name", false))
> +        return false;
> +
> +    // 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"
> +    return true;  
> +}
> +
>  bool parseHTTPRefresh(const String& refresh, bool fromHttpEquivMeta, double& delay, String& url)
>  {
>      int len = refresh.length();
> _______________________________________________
> webkit-changes mailing list
> webkit-changes at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20100329/905fac56/attachment.html>