Add support for selectable versions in QR encoder.

Bug: 1088012
Change-Id: I5f09de4fb703864e13af8b67f6f4f130735f82fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2238491
Commit-Queue: Travis Skare <skare@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786083}

chrome/browser/ui/views/webauthn/authenticator_qr_sheet_view.cc

@@ -29,7 +29,7 @@ class QRView : public views::View {
   static constexpr int kDinoTilePixels = 3;
   // kMid is the pixel offset from the top (or left) to the middle of the
   // displayed QR code.
-  static constexpr int kMid = (kTilePixels * (2 + QRCode::kSize + 2)) / 2;
+  static constexpr int kMid = (kTilePixels * (2 + QRCode::V5::kSize + 2)) / 2;
   // kDinoX is the x-coordinate of the dino image.
   static constexpr int kDinoX =
       kMid - (dino_image::kDinoWidth * kDinoTilePixels) / 2;
@@ -37,21 +37,30 @@ class QRView : public views::View {
   static constexpr int kDinoY =
       kMid - (dino_image::kDinoHeight * kDinoTilePixels) / 2;
 
-  explicit QRView(base::span<const uint8_t> qr_data)
-      : qr_tiles_(qr_.Generate(qr_data)) {}
+  explicit QRView(base::span<const uint8_t> qr_data) {
+    base::Optional<QRCode::GeneratedCode> code = qr_.Generate(qr_data);
+    DCHECK(code);
+    // The QR Encoder supports dynamic sizing but we expect our data to fit in
+    // a version five code.
+    DCHECK(code->qr_size == QRCode::V5::kSize);
+    qr_tiles_ = code->data;
+  }
+
   ~QRView() override = default;
 
   void RefreshQRCode(base::span<const uint8_t> new_qr_data) {
     state_ = (state_ + 1) % 6;
-    qr_tiles_ = qr_.Generate(new_qr_data);
+    base::Optional<QRCode::GeneratedCode> code = qr_.Generate(new_qr_data);
+    DCHECK(code);
+    qr_tiles_ = code->data;
     SchedulePaint();
   }
 
   // View:
   gfx::Size CalculatePreferredSize() const override {
     // A two-tile border is required around the QR code.
-    return gfx::Size((2 + QRCode::kSize + 2) * kTilePixels,
-                     (2 + QRCode::kSize + 2) * kTilePixels);
+    return gfx::Size((2 + QRCode::V5::kSize + 2) * kTilePixels,
+                     (2 + QRCode::V5::kSize + 2) * kTilePixels);
   }
 
   void OnPaint(gfx::Canvas* canvas) override {
@@ -83,28 +92,28 @@ class QRView : public views::View {
 
     // Draw the two-tile border around the edge.
     // Top.
-    canvas->FillRect(
-        gfx::Rect(0, 0, (2 + QRCode::kSize + 2) * kTilePixels, 2 * kTilePixels),
-        off);
+    canvas->FillRect(gfx::Rect(0, 0, (2 + QRCode::V5::kSize + 2) * kTilePixels,
+                               2 * kTilePixels),
+                     off);
     // Bottom.
     canvas->FillRect(
-        gfx::Rect(0, (2 + QRCode::kSize) * kTilePixels,
-                  (2 + QRCode::kSize + 2) * kTilePixels, 2 * kTilePixels),
+        gfx::Rect(0, (2 + QRCode::V5::kSize) * kTilePixels,
+                  (2 + QRCode::V5::kSize + 2) * kTilePixels, 2 * kTilePixels),
         off);
     // Left
     canvas->FillRect(gfx::Rect(0, 2 * kTilePixels, 2 * kTilePixels,
-                               QRCode::kSize * kTilePixels),
+                               QRCode::V5::kSize * kTilePixels),
                      off);
     // Right
     canvas->FillRect(
-        gfx::Rect((2 + QRCode::kSize) * kTilePixels, 2 * kTilePixels,
-                  2 * kTilePixels, QRCode::kSize * kTilePixels),
+        gfx::Rect((2 + QRCode::V5::kSize) * kTilePixels, 2 * kTilePixels,
+                  2 * kTilePixels, QRCode::V5::kSize * kTilePixels),
         off);
 
     // Paint the QR code.
     int index = 0;
-    for (int y = 0; y < QRCode::kSize; y++) {
-      for (int x = 0; x < QRCode::kSize; x++) {
+    for (int y = 0; y < QRCode::V5::kSize; y++) {
+      for (int x = 0; x < QRCode::V5::kSize; x++) {
         SkColor tile_color = qr_tiles_[index++] & 1 ? on : off;
         canvas->FillRect(gfx::Rect((x + 2) * kTilePixels, (y + 2) * kTilePixels,
                                    kTilePixels, kTilePixels),
@@ -152,7 +161,7 @@ class QRView : public views::View {
   }
 
   QRCode qr_;
-  base::span<const uint8_t, QRCode::kTotalSize> qr_tiles_;
+  base::span<const uint8_t> qr_tiles_;
   int state_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(QRView);
@@ -169,7 +178,7 @@ constexpr size_t Base64EncodedSize(size_t input_length) {
 // |qr_generator_key| and the current time such that the caBLE discovery code
 // can recognise the URL as valid.
 base::span<uint8_t> QRDataForCurrentTime(
-    uint8_t out_buf[QRCode::kInputBytes],
+    uint8_t out_buf[QRCode::V5::kInputBytes],
     base::span<const uint8_t, 32> qr_generator_key) {
   const int64_t current_tick = device::CableDiscoveryData::CurrentTimeTick();
   const device::CableQRData qr_data =
@@ -187,7 +196,7 @@ base::span<uint8_t> QRDataForCurrentTime(
   static constexpr char kPrefix[] = "fido://c1/";
   static constexpr size_t kPrefixLength = sizeof(kPrefix) - 1;
 
-  static_assert(QRCode::kInputBytes >= kPrefixLength + kEncodedDataLength,
+  static_assert(QRCode::V5::kInputBytes >= kPrefixLength + kEncodedDataLength,
                 "unexpected QR input length");
   memcpy(out_buf, kPrefix, kPrefixLength);
   memcpy(&out_buf[kPrefixLength], base64_qr_data.data(), kEncodedDataLength);
@@ -228,7 +237,7 @@ AuthenticatorQRSheetView::~AuthenticatorQRSheetView() = default;
 
 std::unique_ptr<views::View>
 AuthenticatorQRSheetView::BuildStepSpecificContent() {
-  uint8_t qr_data_buf[QRCode::kInputBytes];
+  uint8_t qr_data_buf[QRCode::V5::kInputBytes];
   auto qr_view = std::make_unique<AuthenticatorQRViewCentered>(
       QRDataForCurrentTime(qr_data_buf, qr_generator_key_));
   qr_view_ = qr_view.get();
@@ -239,6 +248,6 @@ AuthenticatorQRSheetView::BuildStepSpecificContent() {
 }
 
 void AuthenticatorQRSheetView::Update() {
-  uint8_t qr_data_buf[QRCode::kInputBytes];
+  uint8_t qr_data_buf[QRCode::V5::kInputBytes];
   qr_view_->RefreshQRCode(QRDataForCurrentTime(qr_data_buf, qr_generator_key_));
 }

chrome/common/qr_code_generator/qr_code_generator.h

@@ -9,38 +9,134 @@
 #include <stdint.h>
 
 #include "base/containers/span.h"
+#include "base/optional.h"
 
-// QRCodeGenerator generates version five, class M QR codes that carry 84
-// bytes of raw data. References in the following comments refer to ISO 18004
-// (3rd edition).
+// QRCodeGenerator generates class M QR codes of various versions.
+// References in the following comments refer to ISO 18004 (3rd edition).
+// Supports versions up to 26 by adding constants.
 class QRCodeGenerator {
  public:
-  // kSize is the number of "tiles" in each dimension for a v5 QR code. See
-  // table 1. (The colored squares in in QR codes are called tiles in the
-  // spec.)
-  static constexpr int kSize = 37;
-  // kTotalSize is the total number of tiles for a v5 QR code, in both
-  // directions.
-  static constexpr int kTotalSize = kSize * kSize;
-  // These values are taken from table 9 (page 38) for a version five, class M
-  // QR code. (That table is very badly formatted for version five, the rows in
-  // the last column don't line up correctly. The correct value for 5M is
-  // (67,43,12).)
-  static constexpr size_t kTotalBytes = 134;
-  static constexpr size_t kNumSegments = 2;
-  static constexpr size_t kSegmentDataBytes = 43;
-
-  static constexpr size_t kSegmentBytes = kTotalBytes / kNumSegments;
-  static constexpr size_t kSegmentECBytes = kSegmentBytes - kSegmentDataBytes;
-  static constexpr size_t kDataBytes = kSegmentDataBytes * kNumSegments;
-  // Two bytes of overhead are needed for QR framing.
-  static constexpr size_t kInputBytes = kDataBytes - 2;
-
-  // Generate generates a QR code containing the given data and returns a
-  // pointer to an array of kTotalSize bytes where the least-significant bit of
-  // each byte is set if that tile should be "black". The length of |in| must be
-  // less than, or equal to, |kInputBytes|.
-  base::span<uint8_t, kTotalSize> Generate(base::span<const uint8_t> in);
+  // A structure containing QR version-specific constants and data.
+  // All versions currently use error correction at level M.
+  struct QRVersionInfo {
+    constexpr QRVersionInfo(const int version,
+                            const int size,
+                            const size_t total_bytes,
+                            const size_t group_bytes,
+                            const size_t num_segments,
+                            const size_t segment_data_bytes,
+                            const size_t group_bytes_1,
+                            const size_t num_segments_1,
+                            const size_t segment_data_bytes_1)
+        : version(version),
+          size(size),
+          total_bytes(total_bytes),
+          group_bytes(group_bytes),
+          num_segments(num_segments),
+          segment_data_bytes(segment_data_bytes),
+          group_bytes_1(group_bytes_1),
+          num_segments_1(num_segments_1),
+          segment_data_bytes_1(segment_data_bytes_1) {}
+
+    // The version of the QR code.
+    const int version;
+
+    // The number of "tiles" in each dimension for a QR code of |version|. See
+    // table 1. (The colored squares in in QR codes are called tiles in the
+    // spec.)
+    const int size;
+
+    // Values taken from Table 9, page 38, for a QR code of version |version|.
+    const size_t total_bytes;  // Sum of group_bytes for each group.
+    const size_t group_bytes;
+    const size_t num_segments;
+    const size_t segment_data_bytes;
+    const size_t group_bytes_1;
+    const size_t num_segments_1;
+    const size_t segment_data_bytes_1;
+
+    // Total number of tiles for the QR code, size*size.
+    constexpr int total_size() const { return size * size; }
+
+    constexpr size_t segment_bytes() const {
+      return group_bytes / num_segments;
+    }
+
+    constexpr size_t segment_ec_bytes() const {
+      return segment_bytes() - segment_data_bytes;
+    }
+
+    constexpr size_t data_bytes() const {
+      return segment_data_bytes * num_segments;
+    }
+
+    constexpr size_t segment_bytes_1() const {
+      if (num_segments_1 == 0)
+        return 0;
+      return group_bytes_1 / num_segments_1;
+    }
+
+    constexpr size_t segment_ec_bytes_1() const {
+      return segment_bytes_1() - segment_data_bytes_1;
+    }
+
+    constexpr size_t data_bytes_1() const {
+      return segment_data_bytes_1 * num_segments_1;
+    }
+
+    // Two bytes of overhead are needed for QR framing.
+    // If extending beyond version 26, framing would need to be updated.
+    size_t input_bytes() const {
+      int framing_bytes = version <= 9 ? 2 : 3;
+      return data_bytes() + data_bytes_1() - framing_bytes;
+    }
+
+    DISALLOW_COPY_AND_ASSIGN(QRVersionInfo);
+  };
+
+  // Contains output data for Generate().
+  // The default state contains no data.
+  struct GeneratedCode {
+   public:
+    GeneratedCode();
+    GeneratedCode(GeneratedCode&&);
+    ~GeneratedCode();
+
+    // Pixel data; pointer to an array of bytes, where the least-significant
+    // bit of each byte is set if that tile should be "black".
+    // Clients should ensure four modules of padding when rendering the code.
+    // On error, will not be populated, and will evaluate to false.
+    base::span<uint8_t> data;
+
+    // Width and height (which are equal) of the generated data, in tiles.
+    int qr_size = 0;
+
+    DISALLOW_COPY_AND_ASSIGN(GeneratedCode);
+  };
+
+  // Static parameters for V5 QR codes.
+  // These exist while migrating clients to dynamic sizes.
+  struct V5 {
+    static constexpr int kSize = 37;
+    static constexpr int kTotalSize = kSize * kSize;
+    static constexpr size_t kNumSegments = 2;
+    static constexpr size_t kSegmentDataBytes = 43;
+    static constexpr size_t kDataBytes = kSegmentDataBytes * kNumSegments;
+    static constexpr size_t kInputBytes = kDataBytes - 2;
+  };
+
+  QRCodeGenerator();
+  ~QRCodeGenerator();
+
+  // Returns parameters for different QR code versions, or nullptr if the
+  // version is not supported (you may provide support in the cc file).
+  static const QRVersionInfo* GetVersionInfo(int version);
+
+  // Generates a QR code containing the given data.
+  // The generator will attempt to choose a version that fits the data. The
+  // returned span's length is input-dependent and not known at compile-time in
+  // this case.
+  base::Optional<GeneratedCode> Generate(base::span<const uint8_t> in);
 
  private:
   // MaskFunction3 implements one of the data-masking functions. See figure 21.
@@ -88,14 +184,27 @@ class QRCodeGenerator {
 
   // AddErrorCorrection writes the Reed-Solomon expanded version of |in| to
   // |out|.
-  static void AddErrorCorrection(uint8_t out[kSegmentBytes],
-                                 const uint8_t in[kSegmentDataBytes]);
+  // |out| should have length segment_bytes for the code's version.
+  // |in| should have length segment_data_bytes for the code's version.
+  // |segment_bytes| and |segment_ec_bytes| must be provided for the current
+  // version/level/group.
+  void AddErrorCorrection(uint8_t out[],
+                          const uint8_t in[],
+                          size_t segment_bytes,
+                          size_t segment_ec_bytes);
+
+  // Parameters for the currently-selected version of the QR code.
+  // Generate() will pick a version that can contain enough data.
+  // Unowned; nullptr until initialized in Generate().
+  const QRVersionInfo* version_info_ = nullptr;
 
   // d_ represents a QR code with one byte per pixel. Each byte is one pixel.
   // The LSB is set if the pixel is "black". The second bit is set if the pixel
   // is part of the structure of the QR code, i.e. finder or alignment symbols,
   // timing patterns, or format data.
-  uint8_t d_[kSize * kSize];
+  // Initialized and possibly reinitialized in Generate().
+  std::unique_ptr<uint8_t[]> d_;
+
   // clip_dump_ is the target of paints that would otherwise fall outside of the
   // QR code.
   uint8_t clip_dump_;

chrome/common/qr_code_generator/qr_code_generator_unittest.cc

@@ -13,14 +13,43 @@ TEST(QRCodeGenerator, Generate) {
   // run under ASan, this will also check that every byte of the output has been
   // written to.
   QRCodeGenerator qr;
-  uint8_t input[QRCodeGenerator::kInputBytes];
+  uint8_t input[QRCodeGenerator::V5::kInputBytes];
   memset(input, 'a', sizeof(input));
-  auto qr_data = qr.Generate(input);
+  base::Optional<QRCodeGenerator::GeneratedCode> qr_code = qr.Generate(input);
+  ASSERT_NE(qr_code, base::nullopt);
+  auto& qr_data = qr_code->data;
 
   int index = 0;
-  for (int y = 0; y < QRCodeGenerator::kSize; y++) {
-    for (int x = 0; x < QRCodeGenerator::kSize; x++) {
+  ASSERT_EQ(qr_data.size(),
+            static_cast<size_t>(QRCodeGenerator::V5::kTotalSize));
+  for (int y = 0; y < QRCodeGenerator::V5::kSize; y++) {
+    for (int x = 0; x < QRCodeGenerator::V5::kSize; x++) {
       ASSERT_EQ(0, qr_data[index++] & 0b11111100);
     }
   }
 }
+
+TEST(QRCodeGenerator, GenerateVersionSelection) {
+  // Ensure that dynamic version selection works,
+  // even when reusing the same QR Generator object.
+  // Longer inputs produce longer codes.
+  QRCodeGenerator qr;
+
+  uint8_t input_small[10];
+  memset(input_small, 'a', sizeof(input_small));
+  base::Optional<QRCodeGenerator::GeneratedCode> qr_code_small(
+      qr.Generate(input_small));
+  ASSERT_NE(qr_code_small, base::nullopt);
+  int size_small = qr_code_small->data.size();
+
+  uint8_t input_large[100];
+  memset(input_large, 'a', sizeof(input_large));
+  base::Optional<QRCodeGenerator::GeneratedCode> qr_code_large(
+      qr.Generate(input_large));
+  ASSERT_NE(qr_code_large, base::nullopt);
+  int size_large = qr_code_large->data.size();
+
+  ASSERT_GT(size_small, 0);
+  ASSERT_GT(size_large, 0);
+  ASSERT_GT(size_large, size_small);
+}

chrome/services/qrcode_generator/qrcode_generator_service_impl.cc

@@ -226,34 +226,52 @@ void QRCodeGeneratorServiceImpl::GenerateQRCode(
   mojom::GenerateQRCodeResponsePtr response =
       mojom::GenerateQRCodeResponse::New();
 
-  // TODO(skare): Use a higher QR code version to allow for more data.
-  // TODO(skare): cap string length with message in the UI.
-  if (request->data.length() > QRCodeGenerator::kInputBytes) {
+  if (!request->data.data()) {
+    response->error_code = mojom::QRCodeGeneratorError::UNKNOWN_ERROR;
+    std::move(callback).Run(std::move(response));
+    return;
+  }
+
+  // These lengths account for the dino we superimpose, which
+  // cuts into error correction bits.
+  constexpr size_t kLengthSmall = 84;
+  constexpr size_t kLengthMedium = 122;
+  // TODO(skare): Test very large codes with readers:
+  // constexpr size_t kLengthLarge = 331;
+  constexpr size_t kLengthMax = kLengthMedium;
+  if (request->data.length() > kLengthMax) {
     response->error_code = mojom::QRCodeGeneratorError::INPUT_TOO_LONG;
     std::move(callback).Run(std::move(response));
     return;
   }
 
-  // TODO(skare): Use a higher QR code vdersion. Until then, use the size
-  // from the common encoder.
-  const gfx::Size qr_output_data_size = {QRCodeGenerator::kSize,
-                                         QRCodeGenerator::kSize};
   // TODO(skare): cap string length with message in the UI.
-  uint8_t input[QRCodeGenerator::kInputBytes + 1] = {0};
+  uint8_t input[kLengthMax + 1] = {0};
   base::strlcpy(reinterpret_cast<char*>(input), request->data.c_str(),
-                QRCodeGenerator::kInputBytes);
+                kLengthMax);
+  size_t data_size =
+      request->data.length() <= kLengthSmall ? kLengthSmall : kLengthMedium;
+
   // Pad with spaces rather than null for better reader compatibility.
-  // This will go away when supporting multiple versions/data length.
-  for (size_t i = request->data.length(); i < QRCodeGenerator::kInputBytes; i++)
+  for (size_t i = request->data.length(); i < data_size; i++)
     input[i] = 0x20;
-  input[QRCodeGenerator::kInputBytes] = 0;
+  input[data_size - 1] = 0;
 
   QRCodeGenerator qr;
-  auto qr_data_span = qr.Generate(base::span<const uint8_t>(
-      reinterpret_cast<const uint8_t*>(request->data.data()),
-      request->data.size()));
+  base::Optional<QRCodeGenerator::GeneratedCode> qr_data =
+      qr.Generate(base::span<const uint8_t>(
+          reinterpret_cast<const uint8_t*>(request->data.data()),
+          request->data.size()));
+  if (!qr_data) {
+    // The above check should have caught the too-long-URL case.
+    // Remaining errors can be treated as UNKNOWN.
+    response->error_code = mojom::QRCodeGeneratorError::UNKNOWN_ERROR;
+    std::move(callback).Run(std::move(response));
+    return;
+  }
+  auto& qr_data_span = qr_data->data;
 
-  // The least significant bit of each byte in |qr_data_span| is set if the tile
+  // The least significant bit of each byte in |qr_data.span| is set if the tile
   // should be black.
   for (size_t i = 0; i < qr_data_span.size(); i++) {
     qr_data_span[i] &= 1;
@@ -261,8 +279,9 @@ void QRCodeGeneratorServiceImpl::GenerateQRCode(
 
   response->data.insert(response->data.begin(), qr_data_span.begin(),
                         qr_data_span.end());
-  response->data_size = qr_output_data_size;
+  response->data_size = {qr_data_span.size(), qr_data_span.size()};
   response->error_code = mojom::QRCodeGeneratorError::NONE;
+  gfx::Size qr_output_data_size = {qr_data->qr_size, qr_data->qr_size};
   RenderBitmap(qr_data_span.data(), qr_output_data_size, request, &response);
 
   std::move(callback).Run(std::move(response));