Avoid crash in DocumentProperties

printing/backend/win_helper.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.
 
 #include "printing/backend/win_helper.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <memory>
@@ skipping 453 matching lines @@
   const char* xps_color = color ? kXpsTicketColor : kXpsTicketMonochrome;
   std::string xps_ticket = base::StringPrintf(kXpsTicketTemplate, xps_color);
   std::unique_ptr<DEVMODE, base::FreeDeleter> ticket =
       printing::XpsTicketToDevMode(printer_name, xps_ticket);
   if (!ticket)
     return default_ticket;
 
   return ticket;
 }
 
-bool IsPrinterRPCSOnly(HANDLE printer) {
-  PrinterInfo5 info_5;
-  if (!info_5.Init(printer))
-    return false;
-  const wchar_t* name = info_5.get()->pPrinterName;
-  const wchar_t* port = info_5.get()->pPortName;
+bool IsPrinterRPCSOnly(const wchar_t* name, const wchar_t* port) {
   int num_languages =
       DeviceCapabilities(name, port, DC_PERSONALITY, NULL, NULL);
   if (num_languages != 1)
     return false;
   std::vector<wchar_t> buf(33, 0);
   DeviceCapabilities(name, port, DC_PERSONALITY, buf.data(), NULL);
   static constexpr wchar_t kRPCSLanguage[] = L"RPCS";
   return wcscmp(buf.data(), kRPCSLanguage) == 0;
 }
 
+bool PrinterHasValidPaperSize(const wchar_t* name, const wchar_t* port) {
+  return DeviceCapabilities(name, port, DC_PAPERSIZE, NULL, NULL) > 0;

[Lei Zhang, 2017/06/02 21:29:54]

nit: Can we write nullptr in new code?

[rbpotter, 2017/06/03 01:23:24]

Done.

+}
+
 std::unique_ptr<DEVMODE, base::FreeDeleter> CreateDevMode(HANDLE printer,
                                                           DEVMODE* in) {
   LONG buffer_size = DocumentProperties(
       NULL, printer, const_cast<wchar_t*>(L""), NULL, NULL, 0);
   if (buffer_size < static_cast<int>(sizeof(DEVMODE)))
     return std::unique_ptr<DEVMODE, base::FreeDeleter>();
 
   // Some drivers request buffers with size smaller than dmSize + dmDriverExtra.
   // crbug.com/421402
   buffer_size *= 2;
 
   std::unique_ptr<DEVMODE, base::FreeDeleter> out(
       reinterpret_cast<DEVMODE*>(calloc(buffer_size, 1)));
   DWORD flags = (in ? (DM_IN_BUFFER) : 0) | DM_OUT_BUFFER;
 
+  PrinterInfo5 info_5;
+  if (!info_5.Init(printer))
+    return std::unique_ptr<DEVMODE, base::FreeDeleter>();

[Lei Zhang, 2017/06/02 21:29:54]

I think you can just return nullptr. If so, maybe convert all the returns below
too?

[rbpotter, 2017/06/03 01:23:24]

Done.

+  const wchar_t* name = info_5.get()->pPrinterName;
+  const wchar_t* port = info_5.get()->pPortName;
+
   // Check for RPCS drivers on Windows 8+ as DocumentProperties will crash if
-  // called on one of these printers. See crbug.com/679160
-  if (base::win::GetVersion() >= base::win::VERSION_WIN8 &&
-      IsPrinterRPCSOnly(printer)) {
+  // called on one of these printers. Also check that valid paper sizes exist;
+  // some old drivers return no paper sizes and crash in DocumentProperties if
+  // used with Win10. See crbug.com/679160

[Lei Zhang, 2017/06/02 21:29:54]

Append "Also ..." to the existing comment and reference the new bug, or just do
these as two separate checks.

[rbpotter, 2017/06/03 01:23:24]

Done.

+  if ((base::win::GetVersion() >= base::win::VERSION_WIN8 &&
+       IsPrinterRPCSOnly(name, port)) ||
+      !PrinterHasValidPaperSize(name, port)) {
     return std::unique_ptr<DEVMODE, base::FreeDeleter>();
   }
 
   if (DocumentProperties(
           NULL, printer, const_cast<wchar_t*>(L""), out.get(), in, flags) !=
       IDOK) {
     return std::unique_ptr<DEVMODE, base::FreeDeleter>();
   }
   int size = out->dmSize;
   int extra_size = out->dmDriverExtra;
@@ skipping 34 matching lines @@
     *canceled = (result == IDCANCEL);
   if (result != IDOK)
     return std::unique_ptr<DEVMODE, base::FreeDeleter>();
   int size = out->dmSize;
   int extra_size = out->dmDriverExtra;
   CHECK_GE(buffer_size, size + extra_size);
   return out;
 }
 
 }  // namespace printing