Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent print crash on bad deviceName #21946

Merged
merged 2 commits into from Jan 31, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/api/web-contents.md
Expand Up @@ -1260,7 +1260,7 @@ Returns [`PrinterInfo[]`](structures/printer-info.md)
* `silent` Boolean (optional) - Don't ask user for print settings. Default is `false`.
* `printBackground` Boolean (optional) - Prints the background color and image of
the web page. Default is `false`.
* `deviceName` String (optional) - Set the printer device name to use. Default is `''`.
* `deviceName` String (optional) - Set the printer device name to use. Must be the system-defined name and not the 'friendly' name, e.g 'Brother_QL_820NWB' and not 'Brother QL-820NWB'.
* `color` Boolean (optional) - Set whether the printed web page will be in color or grayscale. Default is `true`.
* `margins` Object (optional)
* `marginType` String (optional) - Can be `default`, `none`, `printableArea`, or `custom`. If `custom` is chosen, you will also need to specify `top`, `bottom`, `left`, and `right`.
Expand Down
23 changes: 21 additions & 2 deletions docs/api/webview-tag.md
Expand Up @@ -545,9 +545,28 @@ Stops any `findInPage` request for the `webview` with the provided `action`.

* `options` Object (optional)
* `silent` Boolean (optional) - Don't ask user for print settings. Default is `false`.
* `printBackground` Boolean (optional) - Also prints the background color and image of
* `printBackground` Boolean (optional) - Prints the background color and image of
the web page. Default is `false`.
* `deviceName` String (optional) - Set the printer device name to use. Default is `''`.
* `deviceName` String (optional) - Set the printer device name to use. Must be the system-defined name and not the 'friendly' name, e.g 'Brother_QL_820NWB' and not 'Brother QL-820NWB'.
* `color` Boolean (optional) - Set whether the printed web page will be in color or grayscale. Default is `true`.
codebytere marked this conversation as resolved.
Show resolved Hide resolved
* `margins` Object (optional)
* `marginType` String (optional) - Can be `default`, `none`, `printableArea`, or `custom`. If `custom` is chosen, you will also need to specify `top`, `bottom`, `left`, and `right`.
* `top` Number (optional) - The top margin of the printed web page, in pixels.
* `bottom` Number (optional) - The bottom margin of the printed web page, in pixels.
* `left` Number (optional) - The left margin of the printed web page, in pixels.
* `right` Number (optional) - The right margin of the printed web page, in pixels.
* `landscape` Boolean (optional) - Whether the web page should be printed in landscape mode. Default is `false`.
* `scaleFactor` Number (optional) - The scale factor of the web page.
* `pagesPerSheet` Number (optional) - The number of pages to print per page sheet.
* `collate` Boolean (optional) - Whether the web page should be collated.
* `copies` Number (optional) - The number of copies of the web page to print.
* `pageRanges` Record<string, number> (optional) - The page range to print. Should have two keys: `from` and `to`.
* `duplexMode` String (optional) - Set the duplex mode of the printed web page. Can be `simplex`, `shortEdge`, or `longEdge`.
* `dpi` Object (optional)
* `horizontal` Number (optional) - The horizontal dpi.
* `vertical` Number (optional) - The vertical dpi.
* `header` String (optional) - String to be printed as page header.
* `footer` String (optional) - String to be printed as page footer.

Returns `Promise<void>`

Expand Down
26 changes: 26 additions & 0 deletions shell/browser/api/atom_api_web_contents.cc
Expand Up @@ -116,6 +116,10 @@
#if BUILDFLAG(ENABLE_PRINTING)
#include "chrome/browser/printing/print_view_manager_basic.h"
#include "components/printing/common/print_messages.h"

#if defined(OS_WIN)
#include "printing/backend/win_helper.h"
#endif
#endif

#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
Expand Down Expand Up @@ -346,6 +350,23 @@ base::Optional<base::TimeDelta> GetCursorBlinkInterval() {
return base::nullopt;
}

#if BUILDFLAG(ENABLE_PRINTING)
// This will return false if no printer with the provided device_name can be
// found on the network. We need to check this because Chromium does not do
// sanity checking of device_name validity and so will crash on invalid names.
bool DeviceNameValid(const std::string& device_name) {
codebytere marked this conversation as resolved.
Show resolved Hide resolved
#if defined(OS_MACOSX)
base::ScopedCFTypeRef<CFStringRef> new_printer_id(
base::SysUTF8ToCFStringRef(device_name));
codebytere marked this conversation as resolved.
Show resolved Hide resolved
return PMPrinterCreateFromPrinterID(new_printer_id.get());
codebytere marked this conversation as resolved.
Show resolved Hide resolved
#elif defined(OS_WIN)
printing::ScopedPrinterHandle printer;
return printer.OpenPrinterWithName(base::UTF8ToUTF16(device_name).c_str());
#endif
return true;
}
#endif

} // namespace

WebContents::WebContents(v8::Isolate* isolate,
Expand Down Expand Up @@ -1783,6 +1804,11 @@ void WebContents::Print(gin_helper::Arguments* args) {
// Printer device name as opened by the OS.
base::string16 device_name;
options.Get("deviceName", &device_name);
if (!device_name.empty() &&
!DeviceNameValid(base::UTF16ToUTF8(device_name))) {
args->ThrowError("webContents.print(): Invalid deviceName provided.");
return;
}
settings.SetStringKey(printing::kSettingDeviceName, device_name);

int scale_factor = 100;
Expand Down
16 changes: 15 additions & 1 deletion spec-main/api-web-contents-spec.ts
Expand Up @@ -99,20 +99,34 @@ describe('webContents module', () => {
})

ifdescribe(features.isPrintingEnabled())('webContents.print()', () => {
let w: BrowserWindow

beforeEach(() => {
w = new BrowserWindow({ show: false })
})

afterEach(closeAllWindows)

it('throws when invalid settings are passed', () => {
const w = new BrowserWindow({ show: false })
expect(() => {
// @ts-ignore this line is intentionally incorrect
w.webContents.print(true)
}).to.throw('webContents.print(): Invalid print settings specified.')
})

it('throws when an invalid callback is passed', () => {
expect(() => {
// @ts-ignore this line is intentionally incorrect
w.webContents.print({}, true)
}).to.throw('webContents.print(): Invalid optional callback provided.')
})

ifit(process.platform !== 'linux')('throws when an invalid deviceName is passed', () => {
expect(() => {
w.webContents.print({ deviceName: 'i-am-a-nonexistent-printer' }, () => {})
}).to.throw('webContents.print(): Invalid deviceName provided.')
})

it('does not crash', () => {
const w = new BrowserWindow({ show: false })
codebytere marked this conversation as resolved.
Show resolved Hide resolved
expect(() => {
Expand Down