From 6ffdc6a7b7fa2e2c9186863f4554cd933a2843b5 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Fri, 26 Jun 2015 11:07:00 +0100
Subject: [PATCH] better sanity checking for input profiles

previously it could get RGBA and CMYK images mixed up, see:

https://github.com/lovell/sharp/issues/237
---
 TODO                           |  7 +++-
 libvips/colour/icc_transform.c | 63 ++++++++++++++++++++++++++++++----
 2 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/TODO b/TODO
index 0de5dbe9..89d2af7f 100644
--- a/TODO
+++ b/TODO
@@ -1,4 +1,9 @@
-- fix up docs for HSV 
+- colour needs to split _build() into preprocess / process / postprocess
+  phases
+
+  in icc_import, for example, we want to check that the supplied profile is
+  compatible with the input image as it will be when unpacked and ready for
+  process_line 
 
 - how about something like vips_grid() which turns a tall thin one-band 
   image into a much smaller many-band image?
diff --git a/libvips/colour/icc_transform.c b/libvips/colour/icc_transform.c
index 0c59157d..e03f13fd 100644
--- a/libvips/colour/icc_transform.c
+++ b/libvips/colour/icc_transform.c
@@ -31,6 +31,8 @@
  * 29/9/14
  * 	- check input profiles for compatibility with the input image, thanks
  * 	  James
+ * 26/6/15
+ *	- better profile sanity checking for icc import
  */
 
 /*
@@ -489,6 +491,57 @@ vips_icc_profile_needs_bands( cmsHPROFILE profile )
 	return( needs_bands );
 }
 
+/* How many bands we expect to see from an image after preprocessing by our
+ * parent classes. This is a bit fragile :-( 
+ *
+ * FIXME ... split the _build() for colour into separate preprocess / process
+ * / postprocess phases so we can load profiles after preprocess but before
+ * actual processing takes place.
+ */
+static int
+vips_image_expected_bands( VipsImage *image )
+{
+	int expected_bands;
+
+	switch( image->Bands ) { 
+	case VIPS_INTERPRETATION_B_W:
+	case VIPS_INTERPRETATION_GREY16:
+		expected_bands = 1;
+		break;
+
+	case VIPS_INTERPRETATION_XYZ:
+	case VIPS_INTERPRETATION_LAB:
+	case VIPS_INTERPRETATION_LABQ:
+	case VIPS_INTERPRETATION_RGB:
+	case VIPS_INTERPRETATION_CMC:
+	case VIPS_INTERPRETATION_LCH:
+	case VIPS_INTERPRETATION_LABS:
+	case VIPS_INTERPRETATION_sRGB:
+	case VIPS_INTERPRETATION_YXY:
+	case VIPS_INTERPRETATION_RGB16:
+	case VIPS_INTERPRETATION_scRGB:
+	case VIPS_INTERPRETATION_HSV:
+		expected_bands = 3;
+		break;
+
+	case VIPS_INTERPRETATION_CMYK:
+		expected_bands = 4;
+		break;
+
+	case VIPS_INTERPRETATION_MULTIBAND:
+	case VIPS_INTERPRETATION_HISTOGRAM:
+	case VIPS_INTERPRETATION_MATRIX:
+	case VIPS_INTERPRETATION_FOURIER:
+	default:
+		expected_bands = image->Bands;
+		break;
+	}
+
+	expected_bands = VIPS_MIN( expected_bands, image->Bands );
+
+	return( expected_bands );
+}
+
 static cmsHPROFILE
 vips_icc_load_profile_image( const char *domain, VipsImage *image )
 {
@@ -506,9 +559,8 @@ vips_icc_load_profile_image( const char *domain, VipsImage *image )
 		return( NULL ); 
 	}
 
-	/* We allow extra bands for eg. alpha.
-	 */
-	if( image->Bands < vips_icc_profile_needs_bands( profile ) ) {
+	if( vips_image_expected_bands( image ) != 
+		vips_icc_profile_needs_bands( profile ) ) {
 		VIPS_FREEF( cmsCloseProfile, profile );
 		vips_warn( domain, 
 			"%s", _( "embedded profile incompatible with image" ) );
@@ -530,9 +582,8 @@ vips_icc_load_profile_file( const char *domain,
 		return( NULL );
 	}
 
-	/* We allow extra bands for eg. alpha.
-	 */
-	if( image->Bands < vips_icc_profile_needs_bands( profile ) ) {
+	if( vips_image_expected_bands( image ) != 
+		vips_icc_profile_needs_bands( profile ) ) {
 		VIPS_FREEF( cmsCloseProfile, profile );
 		vips_warn( domain, 
 			_( "profile \"%s\" incompatible with image" ),