Skip to content

Commit 7a84fc5

Browse files
committedFeb 28, 2021
Fix always-false comparison warning in Canvas.cc in Node 15+
Node.js v15 (V8 8.6) has kMaxLength of u64{1}<<32, which is statically determinable to be larger than the largest uint32_t: ``` warning: comparison is always false due to limited range of data type [-Wtype-limits] if (static_cast<uint32_t>(canvas->nBytes()) > node::Buffer::kMaxLength) { ```
1 parent 646b605 commit 7a84fc5

File tree

3 files changed

+11
-2
lines changed

3 files changed

+11
-2
lines changed
 

‎CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,15 @@ project adheres to [Semantic Versioning](http://semver.org/).
1010
### Changed
1111
* Upgrade dtslint
1212
* Add Node.js v15 to CI.
13+
* The C++ class method `nBytes()` now returns a size_t. (Because this is a C++
14+
method only, this is not considered a breaking change.)
1315
### Added
1416
* Added support for `inverse()` and `invertSelf()` to `DOMMatrix` (#1648)
1517
### Fixed
1618
* Fix Pango logging "expect ugly output" on Windows (#1643)
1719
* Fix benchmark for createPNGStream (#1672)
1820
* Fix dangling reference in BackendOperationNotAvailable exception (#1740)
21+
* Fix always-false comparison warning in Canvas.cc.
1922

2023
2.7.0
2124
==================

‎src/Canvas.cc

+4-1
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ NAN_METHOD(Canvas::ToBuffer) {
408408
if (info[0]->StrictEquals(Nan::New<String>("raw").ToLocalChecked())) {
409409
cairo_surface_t *surface = canvas->surface();
410410
cairo_surface_flush(surface);
411-
if (static_cast<uint32_t>(canvas->nBytes()) > node::Buffer::kMaxLength) {
411+
if (canvas->nBytes() > node::Buffer::kMaxLength) {
412412
Nan::ThrowError("Data exceeds maximum buffer length.");
413413
return;
414414
}
@@ -597,6 +597,9 @@ streamPDF(void *c, const uint8_t *data, unsigned len) {
597597
Nan::HandleScope scope;
598598
Nan::AsyncResource async("canvas:StreamPDF");
599599
PdfStreamInfo* streaminfo = static_cast<PdfStreamInfo*>(c);
600+
// TODO this is technically wrong, we're returning a pointer to the data in a
601+
// vector in a class with automatic storage duration. If the canvas goes out
602+
// of scope while we're in the handler, a use-after-free could happen.
600603
Local<Object> buf = Nan::NewBuffer(const_cast<char *>(reinterpret_cast<const char *>(data)), len, stream_pdf_free, 0).ToLocalChecked();
601604
Local<Value> argv[3] = {
602605
Nan::Null()

‎src/Canvas.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <pango/pangocairo.h>
1010
#include <v8.h>
1111
#include <vector>
12+
#include <cstddef>
1213

1314
/*
1415
* Maxmimum states per context.
@@ -63,7 +64,9 @@ class Canvas: public Nan::ObjectWrap {
6364

6465
DLL_PUBLIC inline uint8_t *data(){ return cairo_image_surface_get_data(surface()); }
6566
DLL_PUBLIC inline int stride(){ return cairo_image_surface_get_stride(surface()); }
66-
DLL_PUBLIC inline int nBytes(){ return getHeight() * stride(); }
67+
DLL_PUBLIC inline std::size_t nBytes(){
68+
return static_cast<std::size_t>(getHeight()) * stride();
69+
}
6770

6871
DLL_PUBLIC inline int getWidth() { return backend()->getWidth(); }
6972
DLL_PUBLIC inline int getHeight() { return backend()->getHeight(); }

0 commit comments

Comments
 (0)
Please sign in to comment.