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.

Sign up for GitHub

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

Jump to bottom

fix: add missing buffer size check in nativeImage (backport: 5-0-x) #17568

Merged
merged 1 commit into from Mar 27, 2019
Merged

fix: add missing buffer size check in nativeImage (backport: 5-0-x) #17568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
130 changes: 77 additions & 53 deletions atom/common/api/atom_api_native_image.cc
View file
Open in desktop
Expand Up @@ -81,49 +81,71 @@ float GetScaleFactorFromOptions(mate::Arguments* args) {
return scale_factor;
}

bool AddImageSkiaRep(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
int width,
int height,
double scale_factor) {
auto decoded = std::make_unique<SkBitmap>();
bool AddImageSkiaRepFromPNG(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
double scale_factor) {
SkBitmap bitmap;
if (!gfx::PNGCodec::Decode(data, size, &bitmap))
return false;

image->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale_factor));
return true;
}

bool AddImageSkiaRepFromJPEG(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
double scale_factor) {
auto bitmap = gfx::JPEGCodec::Decode(data, size);
if (!bitmap)
return false;

// `JPEGCodec::Decode()` doesn't tell `SkBitmap` instance it creates
// that all of its pixels are opaque, that's why the bitmap gets
// an alpha type `kPremul_SkAlphaType` instead of `kOpaque_SkAlphaType`.
// Let's fix it here.
// TODO(alexeykuzmin): This workaround should be removed
// when the `JPEGCodec::Decode()` code is fixed.
// See https://github.com/electron/electron/issues/11294.
bitmap->setAlphaType(SkAlphaType::kOpaque_SkAlphaType);

image->AddRepresentation(gfx::ImageSkiaRep(*bitmap, scale_factor));
return true;
}

bool AddImageSkiaRepFromBuffer(gfx::ImageSkia* image,
const unsigned char* data,
size_t size,
int width,
int height,
double scale_factor) {
// Try PNG first.
if (!gfx::PNGCodec::Decode(data, size, decoded.get())) {
// Try JPEG.
decoded = gfx::JPEGCodec::Decode(data, size);
if (decoded) {
// `JPEGCodec::Decode()` doesn't tell `SkBitmap` instance it creates
// that all of its pixels are opaque, that's why the bitmap gets
// an alpha type `kPremul_SkAlphaType` instead of `kOpaque_SkAlphaType`.
// Let's fix it here.
// TODO(alexeykuzmin): This workaround should be removed
// when the `JPEGCodec::Decode()` code is fixed.
// See https://github.com/electron/electron/issues/11294.
decoded->setAlphaType(SkAlphaType::kOpaque_SkAlphaType);
}
}
if (AddImageSkiaRepFromPNG(image, data, size, scale_factor))
return true;

if (!decoded) {
// Try Bitmap
if (width > 0 && height > 0) {
decoded.reset(new SkBitmap);
decoded->allocN32Pixels(width, height, false);
decoded->setPixels(
const_cast<void*>(reinterpret_cast<const void*>(data)));
} else {
return false;
}
}
// Try JPEG second.
if (AddImageSkiaRepFromJPEG(image, data, size, scale_factor))
return true;

if (width == 0 || height == 0)
return false;

auto info = SkImageInfo::MakeN32(width, height, kPremul_SkAlphaType);
if (size < info.computeMinByteSize())
return false;

image->AddRepresentation(gfx::ImageSkiaRep(*decoded, scale_factor));
SkBitmap bitmap;
bitmap.allocN32Pixels(width, height, false);
bitmap.setPixels(const_cast<void*>(reinterpret_cast<const void*>(data)));

image->AddRepresentation(gfx::ImageSkiaRep(bitmap, scale_factor));
return true;
}

bool AddImageSkiaRep(gfx::ImageSkia* image,
const base::FilePath& path,
double scale_factor) {
bool AddImageSkiaRepFromPath(gfx::ImageSkia* image,
const base::FilePath& path,
double scale_factor) {
std::string file_contents;
{
base::ThreadRestrictions::ScopedAllowIO allow_io;
Expand All @@ -134,7 +156,8 @@ bool AddImageSkiaRep(gfx::ImageSkia* image,
const unsigned char* data =
reinterpret_cast<const unsigned char*>(file_contents.data());
size_t size = file_contents.size();
return AddImageSkiaRep(image, data, size, 0, 0, scale_factor);

return AddImageSkiaRepFromBuffer(image, data, size, 0, 0, scale_factor);
}

bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image,
Expand All @@ -143,12 +166,12 @@ bool PopulateImageSkiaRepsFromPath(gfx::ImageSkia* image,
std::string filename(path.BaseName().RemoveExtension().AsUTF8Unsafe());
if (base::MatchPattern(filename, "*@*x"))
// Don't search for other representations if the DPI has been specified.
return AddImageSkiaRep(image, path, GetScaleFactorFromPath(path));
return AddImageSkiaRepFromPath(image, path, GetScaleFactorFromPath(path));
else
succeed |= AddImageSkiaRep(image, path, 1.0f);
succeed |= AddImageSkiaRepFromPath(image, path, 1.0f);

for (const ScaleFactorPair& pair : kScaleFactorPairs)
succeed |= AddImageSkiaRep(
succeed |= AddImageSkiaRepFromPath(
image, path.InsertBeforeExtensionASCII(pair.name), pair.scale);
return succeed;
}
Expand Down Expand Up @@ -423,19 +446,20 @@ void NativeImage::AddRepresentation(const mate::Dictionary& options) {
v8::Local<v8::Value> buffer;
GURL url;
if (options.Get("buffer", &buffer) && node::Buffer::HasInstance(buffer)) {
AddImageSkiaRep(
&image_skia,
reinterpret_cast<unsigned char*>(node::Buffer::Data(buffer)),
node::Buffer::Length(buffer), width, height, scale_factor);
skia_rep_added = true;
auto* data = reinterpret_cast<unsigned char*>(node::Buffer::Data(buffer));
auto size = node::Buffer::Length(buffer);
skia_rep_added = AddImageSkiaRepFromBuffer(&image_skia, data, size, width,
height, scale_factor);
} else if (options.Get("dataURL", &url)) {
std::string mime_type, charset, data;
if (net::DataURL::Parse(url, &mime_type, &charset, &data)) {
if (mime_type == "image/png" || mime_type == "image/jpeg") {
AddImageSkiaRep(&image_skia,
reinterpret_cast<const unsigned char*>(data.c_str()),
data.size(), width, height, scale_factor);
skia_rep_added = true;
auto* data_ptr = reinterpret_cast<const unsigned char*>(data.c_str());
if (mime_type == "image/png") {
skia_rep_added = AddImageSkiaRepFromPNG(&image_skia, data_ptr,
data.size(), scale_factor);
} else if (mime_type == "image/jpeg") {
skia_rep_added = AddImageSkiaRepFromJPEG(&image_skia, data_ptr,
data.size(), scale_factor);
}
}
}
Expand Down Expand Up @@ -526,9 +550,9 @@ mate::Handle<NativeImage> NativeImage::CreateFromBuffer(
}

gfx::ImageSkia image_skia;
AddImageSkiaRep(&image_skia,
reinterpret_cast<unsigned char*>(node::Buffer::Data(buffer)),
node::Buffer::Length(buffer), width, height, scale_factor);
AddImageSkiaRepFromBuffer(
&image_skia, reinterpret_cast<unsigned char*>(node::Buffer::Data(buffer)),
node::Buffer::Length(buffer), width, height, scale_factor);
return Create(args->isolate(), gfx::Image(image_skia));
}

Expand Down
17 changes: 17 additions & 0 deletions spec/api-native-image-spec.js
View file
Open in desktop
Expand Up @@ -132,6 +132,11 @@ describe('nativeImage module', () => {
expect(nativeImage.createFromBuffer(Buffer.from([])).isEmpty())
})

it('returns an empty image when the buffer is too small', () => {
const image = nativeImage.createFromBuffer(Buffer.from([1, 2, 3, 4]), { width: 100, height: 100 })
expect(image.isEmpty()).to.be.true()
})

it('returns an image created from the given buffer', () => {
const imageA = nativeImage.createFromPath(path.join(__dirname, 'fixtures', 'assets', 'logo.png'))

Expand Down Expand Up @@ -452,6 +457,18 @@ describe('nativeImage module', () => {
})

describe('addRepresentation()', () => {
it('does not add representation when the buffer is too small', () => {
const image = nativeImage.createEmpty()

image.addRepresentation({
buffer: Buffer.from([1, 2, 3, 4]),
width: 100,
height: 100
})

expect(image.isEmpty()).to.be.true()
})

it('supports adding a buffer representation for a scale factor', () => {
const image = nativeImage.createEmpty()

Expand Down