[webkit-reviews] review denied: [Bug 25064] Implement a content sniffer : [Attachment 30433] Step 3: Implement sniffing algorithm

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri May 22 21:56:47 PDT 2009


Maciej Stachowiak <mjs at apple.com> has denied Adam Barth <abarth at webkit.org>'s
request for review:
Bug 25064: Implement a content sniffer
https://bugs.webkit.org/show_bug.cgi?id=25064

Attachment 30433: Step 3: Implement sniffing algorithm
https://bugs.webkit.org/attachment.cgi?id=30433&action=review

------- Additional Comments from Maciej Stachowiak <mjs at apple.com>
This can't land for at least the following reasons:

1) In light of the r- of part 2.
2) Because there are no test cases included.

Here are some other comments on the details:

A) These macros seem a little clunky. Perhaps it would be better to have a
.in-type file to generate a source file that has the static sniffing tables.

> #define MAGIC_NUMBER(mimeType, magic) \
>   { (mimeType), (magic), sizeof(magic)-1, false },


B) For the sniffing rules that don't come from the HTML5 spec, should they be
proposed for HTML5?

C) On sniffing feeds - I think the security risk of misinterpreting a non-feed
XML document as a feed is low, so I would suggest to err on the side of being
more generous. I think the code here will not sniff a feed in all the cases
that Safari will, which is probably unwise.

D) Nothing in this patch set seems to turn off the built-in sniffing of NSURL,
CFNetwork, or other network layers that may do their own sniffing.

In addition, I'll try to do some research on CFNetwork's current sniffing rules
to see if there are any important ones that should be added here.

Nice work so far, please revise per above.


More information about the webkit-reviews mailing list