From dda7740399b08516da4eb3ce6a7a5f847090dd63 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 4 Mar 2016 12:03:13 -0800 Subject: [PATCH 01/12] Add failing spec for native image path normalization --- spec/native-image-spec.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 spec/native-image-spec.js diff --git a/spec/native-image-spec.js b/spec/native-image-spec.js new file mode 100644 index 0000000000..bdc0a06b42 --- /dev/null +++ b/spec/native-image-spec.js @@ -0,0 +1,13 @@ +const assert = require('assert'); +const nativeImage = require('electron').nativeImage; +const path = require('path'); + +describe('nativeImage module', function () { + describe('createFromPath(path)', function () { + it('normalizes paths', function () { + const nonAbsolutePath = path.join(__dirname, 'fixtures', 'api') + path.sep + '..' + path.sep + path.join('assets', 'logo.png'); + const image = nativeImage.createFromPath(nonAbsolutePath); + assert(!image.isEmpty()); + }); + }); +}); From b90c0c78959b422e50dde8c72714b1f96219ef83 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 4 Mar 2016 12:05:36 -0800 Subject: [PATCH 02/12] Use MakeAbsoluteFilePath when creating native image from path --- atom/common/api/atom_api_native_image.cc | 16 ++++++++++++---- spec/native-image-spec.js | 2 +- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/atom/common/api/atom_api_native_image.cc b/atom/common/api/atom_api_native_image.cc index a810069e71..b229c4bb39 100644 --- a/atom/common/api/atom_api_native_image.cc +++ b/atom/common/api/atom_api_native_image.cc @@ -13,6 +13,7 @@ #include "atom/common/native_mate_converters/gurl_converter.h" #include "atom/common/node_includes.h" #include "base/base64.h" +#include "base/files/file_util.h" #include "base/strings/string_util.h" #include "base/strings/pattern.h" #include "native_mate/dictionary.h" @@ -254,17 +255,24 @@ mate::Handle NativeImage::CreateFromJPEG( mate::Handle NativeImage::CreateFromPath( v8::Isolate* isolate, const base::FilePath& path) { gfx::ImageSkia image_skia; - if (path.MatchesExtension(FILE_PATH_LITERAL(".ico"))) { + + base::FilePath absolute_path = MakeAbsoluteFilePath(path); + // MakeAbsoluteFilePath returns an empty path on failures + if (absolute_path.empty()) { + absolute_path = path; + } + + if (absolute_path.MatchesExtension(FILE_PATH_LITERAL(".ico"))) { #if defined(OS_WIN) - ReadImageSkiaFromICO(&image_skia, path); + ReadImageSkiaFromICO(&image_skia, absolute_path); #endif } else { - PopulateImageSkiaRepsFromPath(&image_skia, path); + PopulateImageSkiaRepsFromPath(&image_skia, absolute_path); } gfx::Image image(image_skia); mate::Handle handle = Create(isolate, image); #if defined(OS_MACOSX) - if (IsTemplateFilename(path)) + if (IsTemplateFilename(absolute_path)) handle->SetTemplateImage(true); #endif return handle; diff --git a/spec/native-image-spec.js b/spec/native-image-spec.js index bdc0a06b42..e72682a10a 100644 --- a/spec/native-image-spec.js +++ b/spec/native-image-spec.js @@ -4,7 +4,7 @@ const path = require('path'); describe('nativeImage module', function () { describe('createFromPath(path)', function () { - it('normalizes paths', function () { + it('normalizes the path', function () { const nonAbsolutePath = path.join(__dirname, 'fixtures', 'api') + path.sep + '..' + path.sep + path.join('assets', 'logo.png'); const image = nativeImage.createFromPath(nonAbsolutePath); assert(!image.isEmpty()); From 145d5abe8007b55fe75403ebec4021199053f4f9 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 4 Mar 2016 12:09:53 -0800 Subject: [PATCH 03/12] Mention explicit using original path on failures --- atom/common/api/atom_api_native_image.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/common/api/atom_api_native_image.cc b/atom/common/api/atom_api_native_image.cc index b229c4bb39..b1f0e9c5a6 100644 --- a/atom/common/api/atom_api_native_image.cc +++ b/atom/common/api/atom_api_native_image.cc @@ -257,7 +257,7 @@ mate::Handle NativeImage::CreateFromPath( gfx::ImageSkia image_skia; base::FilePath absolute_path = MakeAbsoluteFilePath(path); - // MakeAbsoluteFilePath returns an empty path on failures + // MakeAbsoluteFilePath returns an empty path on failures so use original path if (absolute_path.empty()) { absolute_path = path; } From 8f820e09be43616ffd65ffc00cd43e6153b1a615 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 4 Mar 2016 14:54:12 -0800 Subject: [PATCH 04/12] Use template string and arrow functions --- spec/native-image-spec.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/native-image-spec.js b/spec/native-image-spec.js index e72682a10a..88588eb192 100644 --- a/spec/native-image-spec.js +++ b/spec/native-image-spec.js @@ -1,11 +1,13 @@ +'use strict'; + const assert = require('assert'); const nativeImage = require('electron').nativeImage; const path = require('path'); -describe('nativeImage module', function () { - describe('createFromPath(path)', function () { - it('normalizes the path', function () { - const nonAbsolutePath = path.join(__dirname, 'fixtures', 'api') + path.sep + '..' + path.sep + path.join('assets', 'logo.png'); +describe('nativeImage module', () => { + describe('createFromPath(path)', () => { + it('normalizes the path', () => { + const nonAbsolutePath = `${path.join(__dirname, 'fixtures', 'api')}${path.sep}..${path.sep}${path.join('assets', 'logo.png')}`; const image = nativeImage.createFromPath(nonAbsolutePath); assert(!image.isEmpty()); }); From 8215d661caee2b02f73a0b9a47024a953f7e0e8a Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 4 Mar 2016 14:58:34 -0800 Subject: [PATCH 05/12] Add api- prefix to spec --- spec/{native-image-spec.js => api-native-image-spec.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/{native-image-spec.js => api-native-image-spec.js} (100%) diff --git a/spec/native-image-spec.js b/spec/api-native-image-spec.js similarity index 100% rename from spec/native-image-spec.js rename to spec/api-native-image-spec.js From 7692edf50ec4805fb362150180a1e6b5249d842e Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 7 Mar 2016 09:28:08 -0800 Subject: [PATCH 06/12] Assert image size as well --- spec/api-native-image-spec.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/api-native-image-spec.js b/spec/api-native-image-spec.js index 88588eb192..688819cab1 100644 --- a/spec/api-native-image-spec.js +++ b/spec/api-native-image-spec.js @@ -10,6 +10,8 @@ describe('nativeImage module', () => { const nonAbsolutePath = `${path.join(__dirname, 'fixtures', 'api')}${path.sep}..${path.sep}${path.join('assets', 'logo.png')}`; const image = nativeImage.createFromPath(nonAbsolutePath); assert(!image.isEmpty()); + assert.equal(image.getSize().height, 190); + assert.equal(image.getSize().width, 538); }); }); }); From 9c88a5c1ab8fb1537418cb5d2c705251d24b56c6 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 7 Mar 2016 09:35:35 -0800 Subject: [PATCH 07/12] Check ReferencesParent before calling MakeAbsoluteFilePath --- atom/common/api/atom_api_native_image.cc | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/atom/common/api/atom_api_native_image.cc b/atom/common/api/atom_api_native_image.cc index b1f0e9c5a6..d26fdab8c6 100644 --- a/atom/common/api/atom_api_native_image.cc +++ b/atom/common/api/atom_api_native_image.cc @@ -120,6 +120,20 @@ bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image, return succeed; } +base::FilePath MakePathAbsolute(const base::FilePath& path) { + if (!path.ReferencesParent()) { + return path; + } + + base::FilePath absolute_path = MakeAbsoluteFilePath(path); + // MakeAbsoluteFilePath returns an empty path on failures so use original path + if (absolute_path.empty()) { + return path; + } else { + return absolute_path; + } +} + #if defined(OS_MACOSX) bool IsTemplateFilename(const base::FilePath& path) { return (base::MatchPattern(path.value(), "*Template.*") || @@ -255,12 +269,7 @@ mate::Handle NativeImage::CreateFromJPEG( mate::Handle NativeImage::CreateFromPath( v8::Isolate* isolate, const base::FilePath& path) { gfx::ImageSkia image_skia; - - base::FilePath absolute_path = MakeAbsoluteFilePath(path); - // MakeAbsoluteFilePath returns an empty path on failures so use original path - if (absolute_path.empty()) { - absolute_path = path; - } + base::FilePath absolute_path = MakePathAbsolute(path); if (absolute_path.MatchesExtension(FILE_PATH_LITERAL(".ico"))) { #if defined(OS_WIN) From 97930fcd84402674960b1a3503c5ea6df7012492 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 7 Mar 2016 14:19:24 -0800 Subject: [PATCH 08/12] Add specs for more image path cases --- spec/api-native-image-spec.js | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/spec/api-native-image-spec.js b/spec/api-native-image-spec.js index 688819cab1..4b21231e40 100644 --- a/spec/api-native-image-spec.js +++ b/spec/api-native-image-spec.js @@ -6,9 +6,30 @@ const path = require('path'); describe('nativeImage module', () => { describe('createFromPath(path)', () => { - it('normalizes the path', () => { - const nonAbsolutePath = `${path.join(__dirname, 'fixtures', 'api')}${path.sep}..${path.sep}${path.join('assets', 'logo.png')}`; - const image = nativeImage.createFromPath(nonAbsolutePath); + it('returns an empty image for invalid paths', () => { + assert(nativeImage.createFromPath('').isEmpty()); + assert(nativeImage.createFromPath('does-not-exist.png').isEmpty()); + }); + + it('loads images from paths relative to the current working directory', () => { + const imagePath = `.${path.sep}${path.join('spec', 'fixtures', 'assets', 'logo.png')}`; + const image = nativeImage.createFromPath(imagePath); + assert(!image.isEmpty()); + assert.equal(image.getSize().height, 190); + assert.equal(image.getSize().width, 538); + }) + + it('loads images from paths with `.` segments', () => { + const imagePath = `${path.join(__dirname, 'fixtures')}${path.sep}.${path.sep}${path.join('assets', 'logo.png')}`; + const image = nativeImage.createFromPath(imagePath); + assert(!image.isEmpty()); + assert.equal(image.getSize().height, 190); + assert.equal(image.getSize().width, 538); + }); + + it('loads images from path with `..` segments', () => { + const imagePath = `${path.join(__dirname, 'fixtures', 'api')}${path.sep}..${path.sep}${path.join('assets', 'logo.png')}`; + const image = nativeImage.createFromPath(imagePath); assert(!image.isEmpty()); assert.equal(image.getSize().height, 190); assert.equal(image.getSize().width, 538); From 0dba0b9cad445eae58bff85dcf3333622a8b4833 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 7 Mar 2016 14:21:24 -0800 Subject: [PATCH 09/12] MakePathAbsolute -> NormalizePath --- atom/common/api/atom_api_native_image.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/atom/common/api/atom_api_native_image.cc b/atom/common/api/atom_api_native_image.cc index d26fdab8c6..53f77bd41f 100644 --- a/atom/common/api/atom_api_native_image.cc +++ b/atom/common/api/atom_api_native_image.cc @@ -120,7 +120,7 @@ bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image, return succeed; } -base::FilePath MakePathAbsolute(const base::FilePath& path) { +base::FilePath NormalizePath(const base::FilePath& path) { if (!path.ReferencesParent()) { return path; } @@ -269,7 +269,7 @@ mate::Handle NativeImage::CreateFromJPEG( mate::Handle NativeImage::CreateFromPath( v8::Isolate* isolate, const base::FilePath& path) { gfx::ImageSkia image_skia; - base::FilePath absolute_path = MakePathAbsolute(path); + base::FilePath absolute_path = NormalizePath(path); if (absolute_path.MatchesExtension(FILE_PATH_LITERAL(".ico"))) { #if defined(OS_WIN) From 022c2c0d8cb9a52d3939ec64180744632377a8b4 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 7 Mar 2016 14:22:16 -0800 Subject: [PATCH 10/12] absolute_path -> normalize_path --- atom/common/api/atom_api_native_image.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/atom/common/api/atom_api_native_image.cc b/atom/common/api/atom_api_native_image.cc index 53f77bd41f..3d000b6d5d 100644 --- a/atom/common/api/atom_api_native_image.cc +++ b/atom/common/api/atom_api_native_image.cc @@ -269,19 +269,19 @@ mate::Handle NativeImage::CreateFromJPEG( mate::Handle NativeImage::CreateFromPath( v8::Isolate* isolate, const base::FilePath& path) { gfx::ImageSkia image_skia; - base::FilePath absolute_path = NormalizePath(path); + base::FilePath image_path = NormalizePath(path); - if (absolute_path.MatchesExtension(FILE_PATH_LITERAL(".ico"))) { + if (image_path.MatchesExtension(FILE_PATH_LITERAL(".ico"))) { #if defined(OS_WIN) - ReadImageSkiaFromICO(&image_skia, absolute_path); + ReadImageSkiaFromICO(&image_skia, image_path); #endif } else { - PopulateImageSkiaRepsFromPath(&image_skia, absolute_path); + PopulateImageSkiaRepsFromPath(&image_skia, image_path); } gfx::Image image(image_skia); mate::Handle handle = Create(isolate, image); #if defined(OS_MACOSX) - if (IsTemplateFilename(absolute_path)) + if (IsTemplateFilename(image_path)) handle->SetTemplateImage(true); #endif return handle; From 44376374b0010dcbe8830838fb4d8940e3d1ecf7 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 7 Mar 2016 14:22:55 -0800 Subject: [PATCH 11/12] path -> paths --- spec/api-native-image-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/api-native-image-spec.js b/spec/api-native-image-spec.js index 4b21231e40..c2950c674b 100644 --- a/spec/api-native-image-spec.js +++ b/spec/api-native-image-spec.js @@ -27,7 +27,7 @@ describe('nativeImage module', () => { assert.equal(image.getSize().width, 538); }); - it('loads images from path with `..` segments', () => { + it('loads images from paths with `..` segments', () => { const imagePath = `${path.join(__dirname, 'fixtures', 'api')}${path.sep}..${path.sep}${path.join('assets', 'logo.png')}`; const image = nativeImage.createFromPath(imagePath); assert(!image.isEmpty()); From 4c23e3950a2b2d0ef5d311877ab078d1cdaa1548 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 7 Mar 2016 16:45:10 -0800 Subject: [PATCH 12/12] Add missing semicolon --- spec/api-native-image-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/api-native-image-spec.js b/spec/api-native-image-spec.js index c2950c674b..043a914578 100644 --- a/spec/api-native-image-spec.js +++ b/spec/api-native-image-spec.js @@ -17,7 +17,7 @@ describe('nativeImage module', () => { assert(!image.isEmpty()); assert.equal(image.getSize().height, 190); assert.equal(image.getSize().width, 538); - }) + }); it('loads images from paths with `.` segments', () => { const imagePath = `${path.join(__dirname, 'fixtures')}${path.sep}.${path.sep}${path.join('assets', 'logo.png')}`;