Fix HID input report sizes on Windows for devices without report IDs.

Unlike Linux and OS X, Windows always writes a report ID into the
buffer passed to ReadFile. If the device does not use report IDs the
first byte of the buffer will always be zero. Because of this, and
contrary to the MSDN documentation, the maximum report size reported
in the HIDP_CAPS structure always includes a byte for the report ID.

This patch fixes the interpretation of the values in HIDP_CAPS and
the size of the buffer passed to ReadFile. To prevent uninitialized
bytes from being passed back to an app when a shorter report is read
the true number of bytes read is provided to the completion callback
on all platforms.

BUG=404253

Review URL: https://codereview.chromium.org/474273004

Cr-Commit-Position: refs/heads/master@{#290337}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290337 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 22 insertions, 15 deletions

diff --git a/device/hid/hid_connection.cc b/device/hid/hid_connection.cc
index 04fe329..c916d6a 100644
--- a/device/hid/hid_connection.cc
+++ b/device/hid/hid_connection.cc
@@ -157,12 +157,15 @@ void HidConnection::SendFeatureReport(
 }
 
 bool HidConnection::CompleteRead(scoped_refptr<net::IOBufferWithSize> buffer,
+                                 int bytes_read,
                                  const IOCallback& callback) {
-  if (buffer->size() == 0 || IsReportIdProtected(buffer->data()[0])) {
+  DCHECK_LE(bytes_read, buffer->size());
+
+  if (bytes_read == 0 || IsReportIdProtected(buffer->data()[0])) {
     return false;
   }
 
-  callback.Run(true, buffer->size());
+  callback.Run(true, bytes_read);
   return true;
 }
 
diff --git a/device/hid/hid_connection.h b/device/hid/hid_connection.h
index bf29985..631db1b 100644
--- a/device/hid/hid_connection.h
+++ b/device/hid/hid_connection.h
@@ -66,6 +66,7 @@ class HidConnection : public base::RefCountedThreadSafe<HidConnection> {
   // and this method returns false. Otherwise it runs the callback
   // and returns true.
   bool CompleteRead(scoped_refptr<net::IOBufferWithSize> buffer,
+                    int bytes_read,
                     const IOCallback& callback);
 
  private:
diff --git a/device/hid/hid_connection_linux.cc b/device/hid/hid_connection_linux.cc
index 3c2c1da..5d6dd24 100644
--- a/device/hid/hid_connection_linux.cc
+++ b/device/hid/hid_connection_linux.cc
@@ -224,7 +224,7 @@ void HidConnectionLinux::ProcessReadQueue() {
       memcpy(read.buffer->data(), report.buffer->data(), report.buffer->size());
       pending_reports_.pop();
 
-      if (CompleteRead(report.buffer, read.callback)) {
+      if (CompleteRead(read.buffer, report.buffer->size(), read.callback)) {
         pending_reads_.pop();
       }
     }
diff --git a/device/hid/hid_connection_mac.cc b/device/hid/hid_connection_mac.cc
index d497ac0..b357216 100644
--- a/device/hid/hid_connection_mac.cc
+++ b/device/hid/hid_connection_mac.cc
@@ -163,7 +163,7 @@ void HidConnectionMac::ProcessReadQueue() {
       memcpy(read.buffer->data(), report.buffer->data(), report.buffer->size());
       pending_reports_.pop();
 
-      if (CompleteRead(report.buffer, read.callback)) {
+      if (CompleteRead(read.buffer, report.buffer->size(), read.callback)) {
         pending_reads_.pop();
       }
     }
diff --git a/device/hid/hid_connection_win.cc b/device/hid/hid_connection_win.cc
index ccdb224..dfebcba 100644
--- a/device/hid/hid_connection_win.cc
+++ b/device/hid/hid_connection_win.cc
@@ -124,12 +124,10 @@ HidConnectionWin::~HidConnectionWin() {
 
 void HidConnectionWin::PlatformRead(scoped_refptr<net::IOBufferWithSize> buffer,
                                     const HidConnection::IOCallback& callback) {
-  int expected_report_size = device_info().max_input_report_size;
-  if (device_info().has_report_id) {
-    expected_report_size++;
-  }
+  // Windows will always include the report ID (including zero if report IDs
+  // are not in use) in the buffer.
   scoped_refptr<net::IOBufferWithSize> receive_buffer =
-      new net::IOBufferWithSize(expected_report_size);
+      new net::IOBufferWithSize(device_info().max_input_report_size + 1);
 
   scoped_refptr<PendingHidTransfer> transfer(
       new PendingHidTransfer(this, buffer, receive_buffer, callback));
@@ -148,8 +146,8 @@ void HidConnectionWin::PlatformWrite(
     const HidConnection::IOCallback& callback) {
   // The Windows API always wants either a report ID (if supported) or
   // zero at the front of every output report.
-  scoped_refptr<net::IOBufferWithSize> output_buffer(buffer);
-  output_buffer = new net::IOBufferWithSize(buffer->size() + 1);
+  scoped_refptr<net::IOBufferWithSize> output_buffer(
+      new net::IOBufferWithSize(buffer->size() + 1));
   output_buffer->data()[0] = report_id;
   memcpy(output_buffer->data() + 1, buffer->data(), buffer->size());
 
@@ -252,8 +250,10 @@ void HidConnectionWin::OnTransferFinished(
       }
     }
 
-    CompleteRead(transfer->target_buffer_, transfer->callback_);
+    CompleteRead(
+        transfer->target_buffer_, bytes_transferred, transfer->callback_);
   } else {
+    VPLOG(1) << "HID transfer failed";
     transfer->callback_.Run(false, 0);
   }
 }
diff --git a/device/hid/hid_service_win.cc b/device/hid/hid_service_win.cc
index 130b3cb..23afa4f 100644
--- a/device/hid/hid_service_win.cc
+++ b/device/hid/hid_service_win.cc
@@ -251,13 +251,16 @@ void HidServiceWin::PlatformAddDevice(const std::string& device_path) {
       }
       device_info.collections.push_back(collection_info);
     }
-    if (device_info.has_report_id && device_info.max_input_report_size > 0) {
+    // Whether or not the device includes report IDs in its reports the size
+    // of the report ID is included in the value provided by Windows. This
+    // appears contrary to the MSDN documentation.
+    if (device_info.max_input_report_size > 0) {
       device_info.max_input_report_size--;
     }
-    if (device_info.has_report_id && device_info.max_output_report_size > 0) {
+    if (device_info.max_output_report_size > 0) {
       device_info.max_output_report_size--;
     }
-    if (device_info.has_report_id && device_info.max_feature_report_size > 0) {
+    if (device_info.max_feature_report_size > 0) {
       device_info.max_feature_report_size--;
     }
     HidD_FreePreparsedData(preparsed_data);