GraphicsMagick: ReadBMPImage(): Assure that opacity channel is i...

GraphicsMagick Commits <[email protected]> Thu, 14 Nov 2024 13:37:09 -0600
Newsgroups gmane.comp.video.graphicsmagick.cvs
Message-ID <mailman.35158.1731613043.7833.graphicsmagick-commit@lists.sourceforge.net>
changeset 6a4b53aeec05 in /hg/GraphicsMagick
details: http://hg.GraphicsMagick.org/hg/GraphicsMagick?cmd=changeset;node=6a4b53aeec05
summary: ReadBMPImage(): Assure that opacity channel is initialized, even if matte is not true.

diffstat:

 ChangeLog                              |  13 ++++++
 VisualMagick/installer/inc/version.isx |   4 +-
 coders/bmp.c                           |  69 +++++++++++++++++++--------------
 coders/svg.c                           |  43 ++++++++++++++++----
 magick/render.c                        |   7 ++-
 magick/version.h                       |   4 +-
 tests/rwfile.c                         |   5 +-
 www/ChangeLog.html                     |  12 +++++
 8 files changed, 110 insertions(+), 47 deletions(-)

diffs (448 lines):

diff -r ee76fbeb7e09 -r 6a4b53aeec05 ChangeLog
--- a/ChangeLog	Fri Nov 01 11:28:14 2024 -0500
+++ b/ChangeLog	Thu Nov 14 13:36:06 2024 -0600
@@ -1,3 +1,16 @@
+2024-11-14  Bob Friesenhahn  <[email protected]>
+
+	* coders/bmp.c (ReadBMPImage): Assure that opacity channel is
+	initialized, even if matte is not true.  May help regarding
+	GraphicsMagick discussion thread "Opacity of whole Magick::image
+	is set to the value 205 after reading a 24-bit-bmp".
+
+	* coders/svg.c (ReadSVGImage): Added comments.
+
+	* magick/render.c (DrawImage): Added comments.
+
+	* tests/rwfile.c: More fixes for GCC warnings.
+
 2024-11-01  Bob Friesenhahn  <[email protected]>
 
 	* tests/bitstream.c: Tidy printfs and exit status.
diff -r ee76fbeb7e09 -r 6a4b53aeec05 VisualMagick/installer/inc/version.isx
--- a/VisualMagick/installer/inc/version.isx	Fri Nov 01 11:28:14 2024 -0500
+++ b/VisualMagick/installer/inc/version.isx	Thu Nov 14 13:36:06 2024 -0600
@@ -10,5 +10,5 @@
 
 #define public MagickPackageName "GraphicsMagick"
 #define public MagickPackageVersion "1.4"
-#define public MagickPackageVersionAddendum ".020241101"
-#define public MagickPackageReleaseDate "snapshot-20241101"
+#define public MagickPackageVersionAddendum ".020241114"
+#define public MagickPackageReleaseDate "snapshot-20241114"
diff -r ee76fbeb7e09 -r 6a4b53aeec05 coders/bmp.c
--- a/coders/bmp.c	Fri Nov 01 11:28:14 2024 -0500
+++ b/coders/bmp.c	Thu Nov 14 13:36:06 2024 -0600
@@ -497,7 +497,7 @@
 %                                                                             %
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %
-%  Method IsBMP returns True if the image format type, identified by the
+%  Method IsBMP returns MagickTrue if the image format type, identified by the
 %  magick string, is BMP.
 %
 %  The format of the IsBMP method is:
@@ -506,7 +506,7 @@
 %
 %  A description of each parameter follows:
 %
-%    o status:  Method IsBMP returns True if the image format type is BMP.
+%    o status:  Method IsBMP returns MagickTrue if the image format type is BMP.
 %
 %    o magick: This string is generally the first few bytes of an image file
 %      or blob.
@@ -518,15 +518,15 @@
 static unsigned int IsBMP(const unsigned char *magick,const size_t length)
 {
   if (length < 2)
-    return(False);
+    return(MagickFalse);
   if ((LocaleNCompare((char *) magick,"BA",2) == 0) ||
       (LocaleNCompare((char *) magick,"BM",2) == 0) ||
       (LocaleNCompare((char *) magick,"IC",2) == 0) ||
       (LocaleNCompare((char *) magick,"PI",2) == 0) ||
       (LocaleNCompare((char *) magick,"CI",2) == 0) ||
       (LocaleNCompare((char *) magick,"CP",2) == 0))
-    return(True);
-  return(False);
+    return(MagickTrue);
+  return(MagickFalse);
 }
 
 
@@ -726,7 +726,7 @@
   image->rows=0;
   image->columns=0;
   status=OpenBlob(image_info,image,ReadBinaryBlobMode,exception);
-  if (status == False)
+  if (status == MagickFalse)
     ThrowBMPReaderException(FileOpenError,UnableToOpenFile,image);
   file_size=GetBlobSize(image);
   /*
@@ -1484,7 +1484,7 @@
                 {
                   if(bmp_info.compression==BI_ALPHABITFIELDS)
                   {                                   /* USE ARGB 1555 */
-                    image->matte = True;
+                    image->matte = MagickTrue;
                     bmp_info.alpha_mask=0x00008000U;
                     bmp_info.red_mask=0x00007c00U;
                     bmp_info.green_mask=0x000003e0U;
@@ -1501,7 +1501,7 @@
                 {
                   if(bmp_info.compression==BI_RGB || bmp_info.compression==BI_ALPHABITFIELDS)
                   {
-                    image->matte = True;
+                    image->matte = MagickTrue;
                     bmp_info.alpha_mask=0xff000000U;
                   }
                   bmp_info.red_mask=0x00ff0000U;
@@ -1569,7 +1569,7 @@
                                                     exception,LoadImageText,
                                                     image->filename,
                                                     image->columns,image->rows);
-                      if (status == False)
+                      if (status == MagickFalse)
                         break;
                     }
               }
@@ -1601,7 +1601,7 @@
                                                     exception,LoadImageText,
                                                     image->filename,
                                                     image->columns,image->rows);
-                      if (status == False)
+                      if (status == MagickFalse)
                         break;
                     }
               }
@@ -1645,7 +1645,7 @@
                     blue=((pixel & bmp_info.blue_mask) << shift.blue) >> 16;
                     if (quantum_bits.blue <= 8)
                       blue|=(blue >> 8);
-                    if (image->matte != False)
+                    if (image->matte != MagickFalse)
                       {
                         opacity=((pixel & bmp_info.alpha_mask) << shift.opacity) >> 16;
                         if (quantum_bits.opacity <= 8)
@@ -1655,6 +1655,7 @@
                     q->red=ScaleShortToQuantum(red);
                     q->green=ScaleShortToQuantum(green);
                     q->blue=ScaleShortToQuantum(blue);
+                    q->opacity=OpaqueOpacity;
                     q++;
                   }
                 if (!SyncImagePixels(image))
@@ -1666,7 +1667,7 @@
                                                     exception,LoadImageText,
                                                     image->filename,
                                                     image->columns,image->rows);
-                      if (status == False)
+                      if (status == MagickFalse)
                         break;
                     }
               }
@@ -1689,6 +1690,7 @@
                     q->blue=ScaleCharToQuantum(*p++);
                     q->green=ScaleCharToQuantum(*p++);
                     q->red=ScaleCharToQuantum(*p++);
+                    q->opacity=OpaqueOpacity;
                     q++;
                   }
                 if (!SyncImagePixels(image))
@@ -1700,7 +1702,7 @@
                                                     exception,LoadImageText,
                                                     image->filename,
                                                     image->columns,image->rows);
-                      if (status == False)
+                      if (status == MagickFalse)
                         break;
                     }
               }
@@ -1746,7 +1748,7 @@
                     blue=((pixel & bmp_info.blue_mask) << shift.blue) >> 16;
                     if (quantum_bits.blue <= 8)
                       blue|=(blue >> 8);
-                    if (image->matte != False)
+                    if (image->matte != MagickFalse)
                       {
                         opacity=((pixel & bmp_info.alpha_mask) << shift.opacity) >> 16;
                         /* if(opacity!=0) ZeroOpacity=0; */
@@ -1754,6 +1756,10 @@
                           opacity|=(opacity >> 8);
                         q->opacity=MaxRGB-ScaleShortToQuantum(opacity);
                       }
+                    else
+                      {
+                        q->opacity = OpaqueOpacity;
+                      }
                     q->red=ScaleShortToQuantum(red);
                     q->green=ScaleShortToQuantum(green);
                     q->blue=ScaleShortToQuantum(blue);
@@ -1768,11 +1774,11 @@
                                                     exception,LoadImageText,
                                                     image->filename,
                                                     image->columns,image->rows);
-                      if (status == False)
+                      if (status == MagickFalse)
                         break;
                     }
               }
-            /* if(ZeroOpacity) image->matte = False; */
+            /* if(ZeroOpacity) image->matte = MagickFalse; */
             break;
           }
 
@@ -1796,6 +1802,7 @@
                     q->green = MS_VAL16_TO_QUANTUM(val_16);
                     LD_UINT16_LSB(val_16,p);
                     q->red = MS_VAL16_TO_QUANTUM(val_16);
+                    q->opacity = OpaqueOpacity;
                     q++;
                   }
                 if(!SyncImagePixels(image))
@@ -1807,7 +1814,7 @@
                                                     exception,LoadImageText,
                                                     image->filename,
                                                     image->columns,image->rows);
-                      if(status == False)
+                      if(status == MagickFalse)
                         break;
                     }
               }
@@ -1834,7 +1841,9 @@
                     q->green = MS_VAL16_TO_QUANTUM(val_16);
                     LD_UINT16_LSB(val_16,p);
                     q->red = MS_VAL16_TO_QUANTUM(val_16);
-                    p+=2;               /* TODO: add alpha*/
+                    /* FIXME: support alpha */
+                    q->opacity = OpaqueOpacity;
+                    p+=2;
                     q++;
                   }
                 if(!SyncImagePixels(image))
@@ -1846,15 +1855,15 @@
                                                     exception,LoadImageText,
                                                     image->filename,
                                                     image->columns,image->rows);
-                      if(status == False)
+                      if(status == MagickFalse)
                         break;
                     }
               }
             break;
           }
         default:
-          ThrowBMPReaderException(CorruptImageError,ImproperImageHeader,image)
-            }
+          ThrowBMPReaderException(CorruptImageError,ImproperImageHeader,image);
+        }
       MagickFreeResourceLimitedMemory(pixels);
       if (EOFBlob(image))
         {
@@ -1917,7 +1926,7 @@
           status=MagickMonitorFormatted(TellBlob(image),GetBlobSize(image),
                                         exception,LoadImagesText,
                                         image->filename);
-          if (status == False)
+          if (status == MagickFalse)
             break;
         }
     } while (IsBMP(magick,2));
@@ -2004,8 +2013,8 @@
   entry->magick=(MagickHandler) IsBMP;
   entry->description="Microsoft Windows bitmap image";
   entry->module="BMP";
-  entry->adjoin=False;
-  entry->seekable_stream=True;
+  entry->adjoin=MagickFalse;
+  entry->seekable_stream=MagickTrue;
   entry->coder_class=PrimaryCoderClass;
   (void) RegisterMagickInfo(entry);
 
@@ -2014,9 +2023,9 @@
   entry->magick=(MagickHandler) IsBMP;
   entry->description="Microsoft Windows bitmap image v2";
   entry->module="BMP";
-  entry->adjoin=False;
+  entry->adjoin=MagickFalse;
   entry->coder_class=PrimaryCoderClass;
-  entry->seekable_stream=True;
+  entry->seekable_stream=MagickTrue;
   (void) RegisterMagickInfo(entry);
 
   entry=SetMagickInfo("BMP3");
@@ -2024,8 +2033,8 @@
   entry->magick=(MagickHandler) IsBMP;
   entry->description="Microsoft Windows bitmap image v3";
   entry->module="BMP";
-  entry->adjoin=False;
-  entry->seekable_stream=True;
+  entry->adjoin=MagickFalse;
+  entry->seekable_stream=MagickTrue;
   entry->coder_class=PrimaryCoderClass;
   (void) RegisterMagickInfo(entry);
 }
@@ -2124,8 +2133,8 @@
 %
 %  A description of each parameter follows.
 %
-%    o status: Method WriteBMPImage return True if the image is written.
-%      False is returned is there is a memory shortage or if the image file
+%    o status: Method WriteBMPImage return MagickTrue if the image is written.
+%      MagickFalse is returned is there is a memory shortage or if the image file
 %      fails to write.
 %
 %    o image_info: Specifies a pointer to a ImageInfo structure.
diff -r ee76fbeb7e09 -r 6a4b53aeec05 coders/svg.c
--- a/coders/svg.c	Fri Nov 01 11:28:14 2024 -0500
+++ b/coders/svg.c	Thu Nov 14 13:36:06 2024 -0600
@@ -1,5 +1,5 @@
 /*
-% Copyright (C) 2003-2023 GraphicsMagick Group
+% Copyright (C) 2003-2024 GraphicsMagick Group
 % Copyright (C) 2002 ImageMagick Studio
 %
 % This program is covered by multiple licenses, which are described in
@@ -4073,19 +4073,19 @@
   (void) LogMagickEvent(CoderEvent,GetMagickModule(),"begin SAX");
   /*
     xmlSubstituteEntitiesDefault(1) enables external ENTITY support
-    (e.g. SVGResolveEntity() which allows XML to be downloaded from an
-    external source.  This may be a security hazard if the input is
-    not trustworthy or if connecting to the correct source is not
-    assured. If the XML is parsed on the backside of a firewall then
-    it may be able to access unintended resources.
-
-    See "https://www.w3.org/TR/SVG11/svgdtd.html#DTD.1.16" and
-    "https://hdivsecurity.com/owasp-xml-external-entities-xxe".
+    (e.g. SVGResolveEntity() which allows XML to be loaded from an
+    external source (url or local file).  This may be a security
+    hazard if the input is not trustworthy or if connecting to the
+    correct source is not assured. If the XML is parsed on the
+    backside of a firewall then it may be able to access unintended
+    resources.
+
+    See "https://www.w3.org/TR/SVG11/svgdtd.html#DTD.1.16".
 
     FIXME: Do we need a way for the user to enable this?  Does
     retrieval of external entities work at all?
   */
-  (void) xmlSubstituteEntitiesDefault(0);
+  (void) xmlSubstituteEntitiesDefault(0); /* deprecated */
 
   (void) memset(&SAXModules,0,sizeof(SAXModules));
 #if defined(ENABLE_XML_INTERNAL_SUBSET) && ENABLE_XML_INTERNAL_SUBSET
@@ -4130,6 +4130,29 @@
     }
   if (svg_info.parser != (xmlParserCtxtPtr) NULL)
     {
+      /*
+        Enable substituting entity values in the output.
+
+        xmlCtxtUseOptions(svg_info.parser,XML_PARSE_NOENT) enables external ENTITY support
+        (e.g. SVGResolveEntity() which allows XML to be loaded from an
+        external source (url or local file).  This may be a security
+        hazard if the input is not trustworthy or if connecting to the
+        correct source is not assured. If the XML is parsed on the
+        backside of a firewall then it may be able to access unintended
+        resources.
+
+         See "https://www.w3.org/TR/SVG11/svgdtd.html#DTD.1.16".
+
+         https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-parser.html#xmlParserOption
+         https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-parser.html#XML_PARSE_NOENT
+
+         (void) xmlCtxtUseOptions(svg_info.parser,XML_PARSE_NOENT);
+      */
+      /*
+        Disable network access via XML_PARSE_NONET
+        (void) xmlCtxtUseOptions(svg_info.parser,XML_PARSE_NONET);
+      */
+
       while ((n=ReadBlob(image,MaxTextExtent-1,message)) != 0)
         {
           message[n]='\0';
diff -r ee76fbeb7e09 -r 6a4b53aeec05 magick/render.c
--- a/magick/render.c	Fri Nov 01 11:28:14 2024 -0500
+++ b/magick/render.c	Thu Nov 14 13:36:06 2024 -0600
@@ -2439,6 +2439,10 @@
   Added to support new elements "use" and "class".
 
   FIXME: Need to add anti-recursion measures.
+
+  See if DrawImageGetCurrentRecurseLevel(),
+  DrawImageSetCurrentRecurseLevel(), DrawImageRecurseIn(),
+  DrawImageRecurseOut() can be used.
 */
 static
 char * InsertAttributeIntoInputStream (
@@ -2461,7 +2465,7 @@
   size_t NeededLength;
 
   if (*pStatus == MagickFail)
-    return MagickFail;
+    return (char *) NULL;
 
   /* get attribute name, then get attribute value */
   if (MagickGetToken(q,&q,*ptoken,*ptoken_max_length) < 1)
@@ -2938,6 +2942,7 @@
       {
         if (LocaleCompare("class",keyword) == 0)
           {/*class*/
+            /* FIXME: Deal with possible recursion */
             q = InsertAttributeIntoInputStream(keyword,q,&primitive,&primitive_extent,
                                                &token,&token_max_length,image,
                                                &status,MagickFalse/*UndefAttrIsError*/);
diff -r ee76fbeb7e09 -r 6a4b53aeec05 magick/version.h
--- a/magick/version.h	Fri Nov 01 11:28:14 2024 -0500
+++ b/magick/version.h	Thu Nov 14 13:36:06 2024 -0600
@@ -38,8 +38,8 @@
 #define MagickLibVersion  0x282502
 #define MagickLibVersionText  "1.4"
 #define MagickLibVersionNumber 28,25,2
-#define MagickChangeDate   "20241101"
-#define MagickReleaseDate  "snapshot-20241101"
+#define MagickChangeDate   "20241114"
+#define MagickReleaseDate  "snapshot-20241114"
 
 /*
   The MagickLibInterfaceNewest and MagickLibInterfaceOldest defines
diff -r ee76fbeb7e09 -r 6a4b53aeec05 tests/rwfile.c
--- a/tests/rwfile.c	Fri Nov 01 11:28:14 2024 -0500
+++ b/tests/rwfile.c	Thu Nov 14 13:36:06 2024 -0600
@@ -328,11 +328,12 @@
       /*
         Prepend magic specifier if extension will be ignored.
       */
-      (void) snprintf(filespec, sizeof(filespec), "%s:%s.%%s", format, basefilespec);
+      (void) snprintf(filespec, sizeof(filespec), "%.*s:%.*s", 32, format,
+                      (int) sizeof(filespec)-34 , basefilespec);
     }
   else
     {
-      (void) snprintf(filespec, sizeof(filespec), "%s.%%s", basefilespec);
+      (void) snprintf(filespec, sizeof(filespec), "%s", basefilespec);
     }
 
   (void) snprintf( filename, sizeof(filename), filespec, 1, format );
diff -r ee76fbeb7e09 -r 6a4b53aeec05 www/ChangeLog.html
--- a/www/ChangeLog.html	Fri Nov 01 11:28:14 2024 -0500
+++ b/www/ChangeLog.html	Thu Nov 14 13:36:06 2024 -0600
@@ -38,6 +38,18 @@
 
 <main id="graphicsmagick-changelog">
 <h1 class="title">GraphicsMagick ChangeLog</h1>
+<p>2024-11-14  Bob Friesenhahn  &lt;<a class="reference external" href="mailto:bfriesen&#37;&#52;&#48;simple&#46;dallas&#46;tx&#46;us">bfriesen<span>&#64;</span>simple<span>&#46;</span>dallas<span>&#46;</span>tx<span>&#46;</span>us</a>&gt;</p>
+<blockquote>
+<ul class="simple">
+<li><p>coders/bmp.c (ReadBMPImage): Assure that opacity channel is
+initialized, even if matte is not true.  May help regarding
+GraphicsMagick discussion thread &quot;Opacity of whole Magick::image
+is set to the value 205 after reading a 24-bit-bmp&quot;.</p></li>
+<li><p>coders/svg.c (ReadSVGImage): Added comments.</p></li>
+<li><p>magick/render.c (DrawImage): Added comments.</p></li>
+<li><p>tests/rwfile.c: More fixes for GCC warnings.</p></li>
+</ul>
+</blockquote>
 <p>2024-11-01  Bob Friesenhahn  &lt;<a class="reference external" href="mailto:bfriesen&#37;&#52;&#48;simple&#46;dallas&#46;tx&#46;us">bfriesen<span>&#64;</span>simple<span>&#46;</span>dallas<span>&#46;</span>tx<span>&#46;</span>us</a>&gt;</p>
 <blockquote>
 <ul class="simple">