ImageView: introduce bounds checks in constructor

* throw on ImageView(nullptr, ...)
* new default constructor
* new bounds checked constructor: ImageView(data, size, width, ...)
* add zxing_ImageView_new_checked to c-API
* use new_checked version in Rust and .NET wrappers

core/src/ImageView.h

@@ -7,6 +7,8 @@
 
 #include <algorithm>
 #include <cstdint>
+#include <cstdio>
+#include <stdexcept>
 
 namespace ZXing {
 
@@ -42,10 +44,14 @@ class ImageView
 {
 protected:
 	const uint8_t* _data = nullptr;
-	ImageFormat _format;
+	ImageFormat _format = ImageFormat::None;
 	int _width = 0, _height = 0, _pixStride = 0, _rowStride = 0;
 
 public:
+	/** ImageView default constructor creates a 'null' image view
+	 */
+	ImageView() = default;
+
 	/**
 	 * ImageView constructor
 	 *
@@ -63,7 +69,29 @@ class ImageView
 		  _height(height),
 		  _pixStride(pixStride ? pixStride : PixStride(format)),
 		  _rowStride(rowStride ? rowStride : width * _pixStride)
-	{}
+	{
+		// TODO: [[deprecated]] this check is to prevent exising code from suddenly throwing, remove in 3.0
+		if (_data == nullptr && _width == 0 && _height == 0 && rowStride == 0 && pixStride == 0) {
+			fprintf(stderr, "zxing-cpp deprecation warning: ImageView(nullptr, ...) will throw in the future, use ImageView()\n");
+			return;
+		}
+
+		if (_data == nullptr)
+			throw std::invalid_argument("Can not construct an ImageView from a NULL pointer");
+
+		if (_width <= 0 || _height <= 0)
+			throw std::invalid_argument("Neither width nor height of ImageView can be less or equal to 0");
+	}
+
+	/**
+	 * ImageView constructor with bounds checking
+	 */
+	ImageView(const uint8_t* data, int size, int width, int height, ImageFormat format, int rowStride = 0, int pixStride = 0)
+		: ImageView(data, width, height, format, rowStride, pixStride)
+	{
+		if (_rowStride < 0 || _pixStride < 0 || size < _height * _rowStride)
+			throw std::invalid_argument("ImageView parameters are inconsistent (out of bounds)");
+	}
 
 	int width() const { return _width; }
 	int height() const { return _height; }

core/src/ReadBarcode.cpp

@@ -26,7 +26,7 @@ class LumImage : public ImageView
 	{}
 
 public:
-	LumImage() : ImageView(nullptr, 0, 0, ImageFormat::Lum) {}
+	LumImage() = default;
 	LumImage(int w, int h) : LumImage(std::make_unique<uint8_t[]>(w * h), w, h) {}
 
 	uint8_t* data() { return _memory.get(); }
@@ -135,7 +135,7 @@ Result ReadBarcode(const ImageView& _iv, const ReaderOptions& opts)
 
 Results ReadBarcodes(const ImageView& _iv, const ReaderOptions& opts)
 {
-	if (sizeof(PatternType) < 4 && opts.hasFormat(BarcodeFormat::LinearCodes) && (_iv.width() > 0xffff || _iv.height() > 0xffff))
+	if (sizeof(PatternType) < 4 && (_iv.width() > 0xffff || _iv.height() > 0xffff))
 		throw std::invalid_argument("Maximum image width/height is 65535");
 
 	if (!_iv.data(0, 0) || _iv.width() * _iv.height() == 0)

core/src/zxing-c.cpp

@@ -77,7 +77,24 @@ zxing_ImageView* zxing_ImageView_new(const uint8_t* data, int width, int height,
 									 int pixStride)
 {
 	ImageFormat cppformat = static_cast<ImageFormat>(format);
-	return new ImageView(data, width, height, cppformat, rowStride, pixStride);
+	try {
+		return new ImageView(data, width, height, cppformat, rowStride, pixStride);
+	} catch (std::exception& e) {
+		lastErrorMsg = e.what();
+	}
+	return NULL;
+}
+
+zxing_ImageView* zxing_ImageView_new_checked(const uint8_t* data, int size, int width, int height, zxing_ImageFormat format,
+											 int rowStride, int pixStride)
+{
+	ImageFormat cppformat = static_cast<ImageFormat>(format);
+	try {
+		return new ImageView(data, size, width, height, cppformat, rowStride, pixStride);
+	} catch (std::exception& e) {
+		lastErrorMsg = e.what();
+	}
+	return NULL;
 }
 
 void zxing_ImageView_delete(zxing_ImageView* iv)

core/src/zxing-c.h

@@ -49,6 +49,8 @@ typedef enum {
 
 zxing_ImageView* zxing_ImageView_new(const uint8_t* data, int width, int height, zxing_ImageFormat format, int rowStride,
 									 int pixStride);
+zxing_ImageView* zxing_ImageView_new_checked(const uint8_t* data, int size, int width, int height, zxing_ImageFormat format,
+											 int rowStride, int pixStride);
 void zxing_ImageView_delete(zxing_ImageView* iv);
 
 void zxing_ImageView_crop(zxing_ImageView* iv, int left, int top, int width, int height);

test/blackbox/ImageLoader.cpp

@@ -23,7 +23,7 @@ class STBImage : public ImageView
 	std::unique_ptr<stbi_uc[], void (*)(void*)> _memory;
 
 public:
-	STBImage() : ImageView(nullptr, 0, 0, ImageFormat::None), _memory(nullptr, stbi_image_free) {}
+	STBImage() : ImageView(), _memory(nullptr, stbi_image_free) {}
 
 	void load(const fs::path& imgPath)
 	{

wrappers/dotnet/ZXingCpp/ZXingCpp.cs

@@ -47,7 +47,7 @@ internal class Dll
 	[DllImport(DllName)] public static extern IntPtr zxing_PositionToString(Position position);
 	[DllImport(DllName)] public static extern BarcodeFormats zxing_BarcodeFormatsFromString(string str);
 
-	[DllImport(DllName)] public static extern IntPtr zxing_ImageView_new(byte[] data, int width, int height, ImageFormat format, int rowStride, int pixStride);
+	[DllImport(DllName)] public static extern IntPtr zxing_ImageView_new_checked(byte[] data, int size, int width, int height, ImageFormat format, int rowStride, int pixStride);
 	[DllImport(DllName)] public static extern void zxing_ImageView_delete(IntPtr iv);
 
 	[DllImport(DllName)] public static extern IntPtr zxing_ReadBarcodes(IntPtr iv, IntPtr opts);
@@ -186,9 +186,9 @@ public class ImageView
 
 	public ImageView(byte[] data, int width, int height, ImageFormat format, int rowStride = 0, int pixStride = 0)
 	{
-		_d = zxing_ImageView_new(data, width, height, format, rowStride, pixStride);
+		_d = zxing_ImageView_new_checked(data, data.Length, width, height, format, rowStride, pixStride);
 		if (_d == IntPtr.Zero)
-			throw new Exception("Failed to create ImageView.");
+			throw new Exception(MarshalAsString(zxing_LastErrorMsg()));
 	}
 
 	~ImageView() => zxing_ImageView_delete(_d);

wrappers/rust/src/bindings.rs

@@ -100,6 +100,15 @@ extern "C" {
 		rowStride: ::core::ffi::c_int,
 		pixStride: ::core::ffi::c_int,
 	) -> *mut zxing_ImageView;
+	pub fn zxing_ImageView_new_checked(
+		data: *const u8,
+		size: ::core::ffi::c_int,
+		width: ::core::ffi::c_int,
+		height: ::core::ffi::c_int,
+		format: zxing_ImageFormat,
+		rowStride: ::core::ffi::c_int,
+		pixStride: ::core::ffi::c_int,
+	) -> *mut zxing_ImageView;
 	pub fn zxing_ImageView_delete(iv: *mut zxing_ImageView);
 	pub fn zxing_ImageView_crop(
 		iv: *mut zxing_ImageView,

wrappers/rust/src/lib.rs

@@ -48,6 +48,13 @@ fn c2r_vec(buf: *mut u8, len: c_int) -> Vec<u8> {
 	res
 }
 
+fn last_error() -> Error {
+	match unsafe { zxing_LastErrorMsg().as_mut() } {
+		None => panic!("Internal error: zxing_LastErrorMsg() returned NULL"),
+		Some(error) => Error::new(ErrorKind::InvalidInput, c2r_str(error)),
+	}
+}
+
 macro_rules! last_error_or {
 	($expr:expr) => {
 		match unsafe { zxing_LastErrorMsg().as_mut() } {
@@ -137,8 +144,9 @@ impl<'a> ImageView<'a> {
 	/// # Safety
 	///
 	/// The memory gets accessed inside the c++ library at random places between
-	/// `ptr` and `ptr + height * row_stride + width * pix_stride. Note that both
-	/// the stride values could be negative, e.g. if the image view is rotated.
+	/// `ptr` and `ptr + height * row_stride` or `ptr + width * pix_stride`.
+	/// Note that both the stride values could be negative, e.g. if the image
+	/// view is rotated.
 	pub unsafe fn from_ptr<T: TryInto<c_int>, U: TryInto<c_int>>(
 		ptr: *const u8,
 		width: T,
@@ -147,30 +155,37 @@ impl<'a> ImageView<'a> {
 		row_stride: U,
 		pix_stride: U,
 	) -> Result<Self, Error> {
-		let res = ImageView(Rc::new(ImageViewOwner(
-			zxing_ImageView_new(
-				ptr,
-				Self::try_into_int(width)?,
-				Self::try_into_int(height)?,
-				format as zxing_ImageFormat,
-				Self::try_into_int(row_stride)?,
-				Self::try_into_int(pix_stride)?,
-			),
-			PhantomData,
-		)));
-		Ok(res)
+		let iv = zxing_ImageView_new(
+			ptr,
+			Self::try_into_int(width)?,
+			Self::try_into_int(height)?,
+			format as zxing_ImageFormat,
+			Self::try_into_int(row_stride)?,
+			Self::try_into_int(pix_stride)?,
+		);
+		if iv.is_null() {
+			Err(last_error())
+		} else {
+			Ok(ImageView(Rc::new(ImageViewOwner(iv, PhantomData))))
+		}
 	}
 
 	pub fn from_slice<T: TryInto<c_int> + Clone>(data: &'a [u8], width: T, height: T, format: ImageFormat) -> Result<Self, Error> {
-		let pix_size = match format {
-			ImageFormat::Lum => 1,
-			ImageFormat::RGB => 3,
-			ImageFormat::RGBX => 4,
-		};
-		if Self::try_into_int(data.len())? < Self::try_into_int(width.clone())? * Self::try_into_int(height.clone())? * pix_size {
-			Err(Error::new(ErrorKind::InvalidInput, "data.len() < width * height * pix_size"))
-		} else {
-			unsafe { Self::from_ptr(data.as_ptr(), width, height, format, 0, 0) }
+		unsafe {
+			let iv = zxing_ImageView_new_checked(
+				data.as_ptr(),
+				data.len() as c_int,
+				Self::try_into_int(width)?,
+				Self::try_into_int(height)?,
+				format as zxing_ImageFormat,
+				0,
+				0,
+			);
+			if iv.is_null() {
+				Err(last_error())
+			} else {
+				Ok(ImageView(Rc::new(ImageViewOwner(iv, PhantomData))))
+			}
 		}
 	}
 
@@ -362,8 +377,7 @@ pub fn read_barcodes<'a>(image: impl TryInto<ImageView<'a>>, opts: impl AsRef<Re
 			zxing_Barcodes_delete(results);
 			Ok(vec)
 		} else {
-			//TODO: maybe replace with simple Err(zxing_lastErrorMsg...)
-			last_error_or!(Vec::<Barcode>::default())
+			Err(last_error())
 		}
 	}
 }