Mime sniffer: reduce table from 256 bytes to 4

net/base/mime_sniffer.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Detecting mime types is a tricky business because we need to balance
 // compatibility concerns with security issues.  Here is a survey of how other
 // browsers behave and then a description of how we intend to behave.
 //
 // HTML payload, no Content-Type header:
 // * IE 7: Render as HTML
@@ skipping 634 matching lines @@
   return pos < end;
 }
 
 // Byte order marks
 static const MagicNumber kByteOrderMark[] = {
   MAGIC_NUMBER("text/plain", "\xFE\xFF")  // UTF-16BE
   MAGIC_NUMBER("text/plain", "\xFF\xFE")  // UTF-16LE
   MAGIC_NUMBER("text/plain", "\xEF\xBB\xBF")  // UTF-8
 };
 
-// Whether a given byte looks like it might be part of binary content.
-// Source: HTML5 spec
-static char kByteLooksBinary[] = {
-  1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 1, 0, 0, 1, 1,  // 0x00 - 0x0F
-  1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1,  // 0x10 - 0x1F
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0x20 - 0x2F
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0x30 - 0x3F
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0x40 - 0x4F
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0x50 - 0x5F
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0x60 - 0x6F
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0x70 - 0x7F
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0x80 - 0x8F
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0x90 - 0x9F
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0xA0 - 0xAF
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0xB0 - 0xBF
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0xC0 - 0xCF
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0xD0 - 0xDF
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0xE0 - 0xEF
-  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // 0xF0 - 0xFF
-};
+// The bytes which are considered to be "binary" are all < 0x20. Encode them one
+// bit per byte, with 1 for a "binary" bit, and 0 for a "text" bit. The
+// least-significant bit represents byte 0x00, the most-significant bit
+// represents byte 0x1F.
+// 0b11110111111111111100100111111111
+static const uint32 kBinaryBits = 0xf7ffc9ff;

[asanka, 2015/04/29 23:17:44]

Let's try to make this a bit more readable.

kByteLooksBinary and kBinaryBits are both trying to represent a map for
binary-data-bytes (https://mimesniff.spec.whatwg.org/#binary-data-byte). I
suggest tossing in a reference to the spec.

Someone glossing over the code is unlikely to count the bits to know what this
bitmap contains. In effect, this makes the binary sniffing logic quite opaque.

One option is to make it explicit what characters we are excluding, since there
are only 5 values. E.g.:

  static const uint32 kBinaryBits =
      ~(1 << '\t' | 1 << '\n' | 1 << '\r' | 1 << '\f' | 1 << '\x1b');

... which I think should evaluate to the same value as you have here.

Or some other creative way of listing out the contents so that a reader can
easily understand what's going on here.

Also, let's move this next to the definition of ContainsControlCodes() below.
That way it'll be clear how the bitmap is used.

[Adam Rice, 2015/05/07 16:52:50]

Done.

 
 // Returns true and sets result to "application/octet-stream" if the content
 // appears to be binary data. Otherwise, returns false and sets "text/plain".
 // Clears have_enough_content if more data could possibly change the result.
 static bool SniffBinary(const char* content,
                         size_t size,
                         bool* have_enough_content,
                         std::string* result) {
   // There is no concensus about exactly how to sniff for binary content.
   // * IE 7: Don't sniff for binary looking bytes, but trust the file extension.
@@ skipping 13 matching lines @@
   std::string unused;
   if (CheckForMagicNumbers(content, size,
                            kByteOrderMark, arraysize(kByteOrderMark),
                            counter, &unused)) {
     // If there is BOM, we think the buffer is not binary.
     result->assign("text/plain");
     return false;
   }
 
   // Next we look to see if any of the bytes "look binary."
-  for (size_t i = 0; i < size; ++i) {
-    // If we a see a binary-looking byte, we think the content is binary.
-    if (kByteLooksBinary[static_cast<unsigned char>(content[i])]) {
-      result->assign("application/octet-stream");
-      return true;
-    }
+  if (ContainsControlCodes(content, size)) {
+    result->assign("application/octet-stream");
+    return true;
   }
 
   // No evidence either way. Default to non-binary and, if truncated, clear
   // have_enough_content because there could be a binary looking byte in the
   // truncated data.
   *have_enough_content &= is_truncated;
   result->assign("text/plain");
   return false;
 }
 
@@ skipping 239 matching lines @@
                                 std::string* result) {
   // First check the extra table.
   if (CheckForMagicNumbers(content, size, kExtraMagicNumbers,
                            arraysize(kExtraMagicNumbers), NULL, result))
     return true;
   // Finally check the original table.
   return CheckForMagicNumbers(content, size, kMagicNumbers,
                               arraysize(kMagicNumbers), NULL, result);
 }
 
+bool ContainsControlCodes(const char* content, size_t size) {

[asanka, 2015/04/29 23:17:44]

Strictly speaking, this is a much fuzzier predicate than its name suggests.
Perhaps call it something like 'LooksLikeBinary'.

[Adam Rice, 2015/05/07 16:52:50]

Done.

+  for (size_t i = 0; i < size; ++i) {
+    uint8 byte = static_cast<uint8>(content[i]);
+    if (byte < 0x20 && (kBinaryBits & (1 << byte)))
+      return true;
+  }
+  return false;
+}
+
 }  // namespace net