[PATCH] Enhancements to VCL metafile handling
Chris Sherlock (via Code Review)
gerrit at gerrit.libreoffice.org
Fri May 10 04:28:58 PDT 2013
Hi,
I have submitted a patch for review:
https://gerrit.libreoffice.org/3839
To pull it, you can do:
git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/39/3839/1
Enhancements to VCL metafile handling
* Remove EMFP_DEBUG with OSL_DEBUG conditional defines and SAL_INFO
* While we are about it, to help with troubleshooting metafile
issues, add another vcl.emf area to log-areas.dox
* Improve error handling when processing an EMF header:
+ replace variable name nsal_uInt32 (!!) with sane variables
to make the code more readable
+ check to ensure that type field is 0x1, which is all it can be
for metafiles
+ check that signature field is set to ASCII-encoded value "FME"
+ loose check of version field to see if it is 0x00010000
+ warn if record count is zero - that really shouldn't be possible
+ check bytes field in header to make sure it correlates to the
actual size of the metafile
* Quite a few more comments in the code to clarify the intended
structure of a metafile, per [MS-EMF] documentation
Change-Id: Id4ed486b2dd0c6e7bdee67cb344aaaf8e8d98f84
---
M include/sal/log-areas.dox
M vcl/source/filter/wmf/enhwmf.cxx
2 files changed, 78 insertions(+), 32 deletions(-)
diff --git a/include/sal/log-areas.dox b/include/sal/log-areas.dox
index 693450b..cf1f1cc 100644
--- a/include/sal/log-areas.dox
+++ b/include/sal/log-areas.dox
@@ -260,6 +260,7 @@
@li @c vcl.atsui - ATSUI (obsolete) -using code for Mac OS X
@li @c vcl.control
@li @c vcl.coretext - CoreText-using code for Mac OS X and iOS
+ at li @c vcl.emf - EMF/EMF+ processing
@li @c vcl.fonts - font-specific code
@li @c vcl.gdi - the GDI part of VCL, devices, bitmaps, etc.
@li @c vcl.gtk - Gtk+ 2/3 plugin
diff --git a/vcl/source/filter/wmf/enhwmf.cxx b/vcl/source/filter/wmf/enhwmf.cxx
index cd71a6b..298cd1e 100644
--- a/vcl/source/filter/wmf/enhwmf.cxx
+++ b/vcl/source/filter/wmf/enhwmf.cxx
@@ -151,12 +151,6 @@
#define EMR_SETLINKEDUFIS 119
#define EMR_SETTEXTJUSTIFICATION 120
-#if OSL_DEBUG_LEVEL > 1
-#define EMFP_DEBUG(x) x
-#else
-#define EMFP_DEBUG(x)
-#endif
-
#ifdef OSL_BIGENDIAN
// currently unused
@@ -226,24 +220,27 @@
return bOk;
}
-EMFP_DEBUG(void dumpWords( SvStream& s, int i )
+#if OSL_DEBUG_LEVEL > 1
+void dumpWords( SvStream& s, int i )
{
sal_uInt32 pos = s.Tell();
sal_Int16 data;
for( ; i > 0; i -- ) {
s >> data;
- EMFP_DEBUG(printf ("\t\t\tdata: %04hx\n", data));
+ SAL_INFO("vcl.emf", "\t\t\tdata: " << std::hex << data);
}
s.Seek (pos);
-});
+};
+#endif
void EnhWMFReader::ReadEMFPlusComment(sal_uInt32 length, sal_Bool& bHaveDC)
{
if (!bEMFPlus) {
pOut->PassEMFPlusHeaderInfo();
+#if OSL_DEBUG_LEVEL > 1
// debug code - write the stream to debug file /tmp/emf-stream.emf
- EMFP_DEBUG(int pos = pWMF->Tell();
+ int pos = pWMF->Tell();
pWMF->Seek(0);
SvFileStream file( OUString( "/tmp/emf-stream.emf" ), STREAM_WRITE | STREAM_TRUNC );
@@ -251,7 +248,9 @@
file.Flush();
file.Close();
- pWMF->Seek( pos );)
+ pWMF->Seek( pos );
+#endif
+
}
bEMFPlus = true;
@@ -265,7 +264,7 @@
OSL_ASSERT(length >= 4);
// reduce by 32bit length itself, skip in SeekRel if
- // impossibly unavailble
+ // impossibly unavailable
sal_uInt32 nRemainder = length >= 4 ? length-4 : length;
const size_t nRequiredHeaderSize = 12;
@@ -277,12 +276,12 @@
*pWMF >> type >> flags >> size >> dataSize;
nRemainder -= nRequiredHeaderSize;
- EMFP_DEBUG(printf ("\t\tEMF+ record type: %d\n", type));
+ SAL_INFO ("vcl.emf", "\t\tEMF+ record type: " << std::hex << type);
// GetDC
if( type == 16388 ) {
bHaveDC = true;
- EMFP_DEBUG(printf ("\t\tEMF+ lock DC (device context)\n"));
+ SAL_INFO ("vcl.emf", "\t\tEMF+ lock DC (device context)");
}
// Get the length of the remaining data of this record based
@@ -454,26 +453,27 @@
if( !aBmpSaveList.empty()
&& ( nRecType != EMR_STRETCHBLT )
&& ( nRecType != EMR_STRETCHDIBITS )
- )
+ ) {
pOut->ResolveBitmapActions( aBmpSaveList );
+ }
bFlag = sal_False;
- EMFP_DEBUG(printf ("0x%04x-0x%04x record type: %d size: %d\n",(unsigned int) (nNextPos - nRecSize),(unsigned int) nNextPos, (int)nRecType,(int) nRecSize));
+ SAL_INFO ("vcl.emf", "0x" << std::hex << (nNextPos - nRecSize) << "-0x" << nNextPos << " record type: " << std::dec << nRecType << " size: " << nRecSize);
if( bEnableEMFPlus && nRecType == EMR_GDICOMMENT ) {
sal_uInt32 length;
*pWMF >> length;
- EMFP_DEBUG(printf ("\tGDI comment\n\t\tlength: %d\n", (int)length));
+ SAL_INFO("vcl.emf", "\tGDI comment\n\t\tlength: " << length);
if( pWMF->good() && length >= 4 ) {
sal_uInt32 id;
*pWMF >> id;
- EMFP_DEBUG(printf ("\t\tbegin %c%c%c%c id: 0x%x\n", (char)(id & 0xff), (char)((id & 0xff00) >> 8), (char)((id & 0xff0000) >> 16), (char)((id & 0xff000000) >> 24), (unsigned int)id));
+ SAL_INFO ("vcl.emf", "\t\tbegin " << (char)(id & 0xff) << (char)((id & 0xff00) >> 8) << (char)((id & 0xff0000) >> 16) << (char)((id & 0xff000000) >> 24) << " id: 0x" << std::hex << id);
// EMF+ comment (FIXME: BE?)
if( id == 0x2B464D45 && nRecSize >= 12 )
@@ -482,7 +482,7 @@
else if( id == 0x43494447 && nRecSize >= 12 ) {
// TODO: ReadGDIComment()
} else {
- EMFP_DEBUG(printf ("\t\tunknown id: 0x%x\n",(unsigned int) id));
+ SAL_INFO ("vcl.emf", "\t\tunknown id: 0x" << std::hex << id);
}
}
}
@@ -1341,16 +1341,21 @@
sal_Bool EnhWMFReader::ReadHeader()
{
- sal_uInt32 nsal_uInt32, nHeaderSize, nPalEntries;
+ sal_uInt32 nType, nSignature, nVersion;
+ sal_uInt32 nHeaderSize, nPalEntries;
sal_Int32 nLeft, nTop, nRight, nBottom;
// Spare me the METAFILEHEADER here
- // Reading the METAHEADER
- *pWMF >> nsal_uInt32 >> nHeaderSize;
- if ( nsal_uInt32 != 1 ) // Type
+ // Reading the METAHEADER - EMR_HEADER ([MS-EMF] section 2.3.4.2 EMR_HEADER Record Types)
+ *pWMF >> nType >> nHeaderSize;
+ if ( nType != 1 ) { // per [MS-EMF] 2.3.4.2 EMF Header Record Types, type MUST be 0x00000001
+ SAL_WARN("vcl.emf", "EMF header type is not set to 0x00000001 - possibly corrupted file?");
return sal_False;
+ }
- // bound size
+ // Start reading the EMR_HEADER Header object
+
+ // bound size (RectL object, see [MS-WMF] section 2.2.2.19)
Rectangle rclBounds; // rectangle in logical units
*pWMF >> nLeft >> nTop >> nRight >> nBottom;
rclBounds.Left() = nLeft;
@@ -1358,7 +1363,7 @@
rclBounds.Right() = nRight;
rclBounds.Bottom() = nBottom;
- // picture frame size
+ // picture frame size (RectL object)
Rectangle rclFrame; // rectangle in device units 1/100th mm
*pWMF >> nLeft >> nTop >> nRight >> nBottom;
rclFrame.Left() = nLeft;
@@ -1366,27 +1371,67 @@
rclFrame.Right() = nRight;
rclFrame.Bottom() = nBottom;
- *pWMF >> nsal_uInt32; // signature
+ *pWMF >> nSignature;
- if ( nsal_uInt32 != 0x464d4520 )
+ // nSignature MUST be the ASCII characters "FME", see [WS-EMF] 2.2.9 Header Object
+ // and 2.1.14 FormatSignature Enumeration
+ if ( nSignature != 0x464d4520 ) {
+ SAL_WARN("vcl.emf", "EMF\t\tSignature is not 0x464d4520 (\"FME\") - possibly corrupted file?");
return sal_False;
+ }
- *pWMF >> nsal_uInt32; // nVersion
+ *pWMF >> nVersion; // according to [WS-EMF] 2.2.9, this SHOULD be 0x0001000, however
+ // Microsoft note that not even Windows checks this...
+ if ( nVersion != 0x00010000 ) {
+ SAL_WARN("vcl.emf", "EMF\t\tThis really should be 0x00010000, though not absolutely essential...");
+ }
+
*pWMF >> nEndPos; // size of metafile
nEndPos += nStartPos;
sal_uInt32 nStrmPos = pWMF->Tell(); // checking if nEndPos is valid
pWMF->Seek( STREAM_SEEK_TO_END );
- if ( pWMF->Tell() < nEndPos )
- nEndPos = pWMF->Tell();
+ sal_uInt32 nActualFileSize = pWMF->Tell();
+
+ if ( nActualFileSize < nEndPos ) {
+ SAL_WARN("vcl.emf", "EMF\t\tEMF Header object records number of bytes as " << nEndPos
+ << ", however the file size is actually " << nActualFileSize
+ << " bytes. Possible file corruption?");
+ nEndPos = nActualFileSize;
+ }
pWMF->Seek( nStrmPos );
*pWMF >> nRecordCount;
- if ( !nRecordCount )
+ if ( !nRecordCount ) {
+ SAL_WARN("vcl.emf", "EMF\t\tEMF Header object shows record counter as 0! This shouldn't "
+ "be possible... indicator of possible file corruption?");
return sal_False;
+ }
- pWMF->SeekRel( 0xc );
+ // the number of "handles", or graphics objects used in the metafile
+
+ sal_uInt16 nHandlesCount;
+ *pWMF >> nHandlesCount;
+
+ // the next 2 bytes are reserved, but according to [MS-EMF] section 2.2.9
+ // it MUST be 0x000 and MUST be ignored... the thing is, having such a specific
+ // value is actually pretty useful in checking if there is possible corruption
+
+ sal_uInt16 nReserved;
+ *pWMF >> nReserved;
+
+ if ( nReserved != 0x0000 ) {
+ SAL_WARN("vcl.emf", "EMF\t\tEMF Header object's reserved field is NOT 0x0000... possible "
+ "corruption?");
+ }
+
+ // The next 4 bytes is specifies the number of characters in the metafile description
+ // the 4 bytes after that specific the offset from this record that contains the
+ // metafile description... zero means no description string.
+ // For now, we ignore it.
+
+ pWMF->SeekRel( 0x8 );
sal_Int32 nPixX, nPixY, nMillX, nMillY;
*pWMF >> nPalEntries >> nPixX >> nPixY >> nMillX >> nMillY;
--
To view, visit https://gerrit.libreoffice.org/3839
To unsubscribe, visit https://gerrit.libreoffice.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id4ed486b2dd0c6e7bdee67cb344aaaf8e8d98f84
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: master
Gerrit-Owner: Chris Sherlock <chris.sherlock79 at gmail.com>
More information about the LibreOffice
mailing list