oox/source/export/shapes.cxx | 35 +++---
sax/source/fastparser/fastparser.cxx | 14 +-
sw/qa/extras/ooxmlexport/data/tdf128820.fodt | 16 ++
sw/qa/extras/ooxmlexport/ooxmlexport14.cxx | 25 ++++
vcl/source/filter/graphicfilter2.cxx | 128 +++++++++++-----------
writerfilter/source/dmapper/DomainMapper_Impl.cxx | 13 ++
writerfilter/source/dmapper/PropertyMap.cxx | 11 +
7 files changed, 161 insertions(+), 81 deletions(-)
New commits:
commit 01ac1e24856121239e289b07b711b3f688187ccf
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Sat Nov 16 15:02:35 2019 +0300
Commit: Michael Stahl <michael.st...@cib.de>
CommitDate: Mon Nov 25 18:04:13 2019 +0100
tdf#128820: use wps namespace for simple text shapes
Without that, simple text shapes inside groups were written in
<pic:wsp> elements, with many child elements also having pic::
prefix.
Change-Id: I114cf3499e03aa5ca042211d7b134aaf5b0e7fbf
Reviewed-on: https://gerrit.libreoffice.org/82980
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Also consider saved exceptions when terminating parse
As with previous commit 18ae77a065cb8ae6940d4067f6ab7e99a3f74047, this
will start showing parse errors on invalid files which previously just
opened without warnings, silently losing the invalid stream part. Any
bug bisected to this commit is not a regression from this commit! The
real problem was already before, and was just disclosed by this (which
is the actual goal).
Also simplify unit test data for tdf#128820, which will now be enough
after the change.
A unit test (testN779627) revealed unexpected throws when parsing; this
was fixed.
Change-Id: I5a21b9001874ec6e3b8273c10043ef930bf1cc82
Reviewed-on: https://gerrit.libreoffice.org/82981
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
tdf#128820: improve unit test to not depend on opening bad file failing
... also test exported XML directly
Change-Id: I50237593dd111e7c7974452769c8d49c22012713
Reviewed-on: https://gerrit.libreoffice.org/83005
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Fix UBSan build after 9fdf8c0a5cc036ea9bd1e11dd8f2c1a6e601fae2
The said commit simplified a testdoc to testTdf128820, using a smallest
possible SVG in it. This seems to produce the smallest possible PNG of
size 8, which is passed into GraphicDescriptor::ImpDetectPNG. There its
size is read into nTemp32 past the end of the file without checks,
which keeps last value of the variable (which was the magic number
0x0d0a1a0a), which is then saved into the descriptor. Then that value
is used in ImpGraphic::ImplGetSizePixel, and later multiplying it in
lclConvertScreenPixelToHmm causes UB.
Fix by checking all the reads in GraphicDescriptor::ImpDetectPNG.
Change-Id: Ib4740fac2b87fbef57d5150151129b9852f3ecb8
Reviewed-on: https://gerrit.libreoffice.org/83119
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/83296
(cherry picked from commit d54bae3471e9b9529f1ac03c811c3370d9f5ed89)
Reviewed-on: https://gerrit.libreoffice.org/83337
Reviewed-by: Michael Stahl <michael.st...@cib.de>
diff --git a/oox/source/export/shapes.cxx b/oox/source/export/shapes.cxx
index 80de09f97a76..64513b58067c 100644
--- a/oox/source/export/shapes.cxx
+++ b/oox/source/export/shapes.cxx
@@ -377,6 +377,14 @@ awt::Size ShapeExport::MapSize( const awt::Size& rSize )
const
return awt::Size( aRetSize.Width(), aRetSize.Height() );
}
+static bool IsNonEmptySimpleText(const Reference<XInterface>& xIface)
+{
+ if (Reference<XSimpleText> xText{ xIface, UNO_QUERY })
+ return xText->getString().getLength();
+
+ return false;
+}
+
bool ShapeExport::NonEmptyText( const Reference< XInterface >& xIface )
{
Reference< XPropertySet > xPropSet( xIface, UNO_QUERY );
@@ -410,12 +418,7 @@ bool ShapeExport::NonEmptyText( const Reference<
XInterface >& xIface )
}
}
- Reference< XSimpleText > xText( xIface, UNO_QUERY );
-
- if( xText.is() )
- return xText->getString().getLength();
-
- return false;
+ return IsNonEmptySimpleText(xIface);
}
ShapeExport& ShapeExport::WritePolyPolygonShape( const Reference< XShape >&
xShape, const bool bClosed )
@@ -525,7 +528,11 @@ ShapeExport& ShapeExport::WriteGroupShape(const
uno::Reference<drawing::XShape>&
uno::Reference<lang::XServiceInfo> xServiceInfo(xChild,
uno::UNO_QUERY_THROW);
if (GetDocumentType() == DOCUMENT_DOCX)
{
- if
(xServiceInfo->supportsService("com.sun.star.drawing.GraphicObjectShape"))
+ // tdf#128820: WriteGraphicObjectShapePart calls WriteTextShape
for non-empty simple
+ // text objects, which needs writing into wps::wsp element, so
make sure to use wps
+ // namespace for those objects
+ if
(xServiceInfo->supportsService("com.sun.star.drawing.GraphicObjectShape")
+ && !IsNonEmptySimpleText(xChild))
mnXmlNamespace = XML_pic;
else
mnXmlNamespace = XML_wps;
@@ -1062,19 +1069,13 @@ void ShapeExport::WriteGraphicObjectShapePart( const
Reference< XShape >& xShape
{
SAL_INFO("oox.shape", "write graphic object shape");
- if( NonEmptyText( xShape ) )
+ if (IsNonEmptySimpleText(xShape))
{
- // avoid treating all 'IsPresentationObject' objects as having text.
- Reference< XSimpleText > xText( xShape, UNO_QUERY );
+ SAL_INFO("oox.shape", "graphicObject: wrote only text");
- if( xText.is() && !xText->getString().isEmpty() )
- {
- SAL_INFO("oox.shape", "graphicObject: wrote only text");
+ WriteTextShape(xShape);
- WriteTextShape( xShape );
-
- return;
- }
+ return;
}
SAL_INFO("oox.shape", "graphicObject without text");
diff --git a/sax/source/fastparser/fastparser.cxx
b/sax/source/fastparser/fastparser.cxx
index 1ac7b209d311..e37d7c0968e1 100644
--- a/sax/source/fastparser/fastparser.cxx
+++ b/sax/source/fastparser/fastparser.cxx
@@ -176,6 +176,8 @@ struct Entity : public ParserData
css::uno::Any maSavedException;
osl::Mutex maSavedExceptionMutex;
void saveException( const Any & e );
+ // Thread-safe check if maSavedException has value
+ bool hasException();
void throwException( const ::rtl::Reference< FastLocatorImpl >
&xDocumentLocator,
bool mbDuringParse );
@@ -623,6 +625,12 @@ void Entity::saveException( const Any & e )
}
}
+bool Entity::hasException()
+{
+ osl::MutexGuard g(maSavedExceptionMutex);
+ return maSavedException.hasValue();
+}
+
} // namespace
namespace sax_fastparser {
@@ -1041,6 +1049,8 @@ void FastSaxParserImpl::parse()
{
if( xmlParseChunk( rEntity.mpParser, reinterpret_cast<const
char*>(seqOut.getConstArray()), 0, 1 ) != XML_ERR_OK )
rEntity.throwException( mxDocumentLocator, true );
+ if (rEntity.hasException())
+ rEntity.throwException(mxDocumentLocator, true);
}
break;
}
@@ -1068,10 +1078,8 @@ void FastSaxParserImpl::parse()
{
rEntity.throwException( mxDocumentLocator, true );
}
- osl::ClearableMutexGuard g(rEntity.maSavedExceptionMutex);
- if (rEntity.maSavedException.hasValue())
+ if (rEntity.hasException())
{
- g.clear();
rEntity.throwException( mxDocumentLocator, true );
}
} while( nRead > 0 );
diff --git a/sw/qa/extras/ooxmlexport/data/tdf128820.fodt
b/sw/qa/extras/ooxmlexport/data/tdf128820.fodt
new file mode 100644
index 000000000000..576fd966e28d
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf128820.fodt
@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<office:document
xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0"
xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0"
xmlns:draw="urn:oasis:names:tc:opendocument:xmlns:drawing:1.0"
xmlns:svg="urn:oasis:names:tc:opendocument:xmlns:svg-compatible:1.0"
office:version="1.2" office:mimetype="application/vnd.oasis.opendocument.text">
+ <office:body>
+ <office:text>
+ <text:p><draw:g text:anchor-type="as-char" draw:name="A">
+ <draw:frame svg:width="1cm" svg:height="1cm" svg:x="1cm" svg:y="1cm">
+ <draw:image>
+
<office:binary-data>PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciLz4</office:binary-data>
+ <text:p>json</text:p>
+ </draw:image>
+ </draw:frame>
+ </draw:g></text:p>
+ </office:text>
+ </office:body>
+</office:document>
\ No newline at end of file
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
index 51452864fe50..e45be7536b39 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
@@ -143,6 +143,31 @@ DECLARE_OOXMLEXPORT_TEST(testTdf124367, "tdf124367.docx")
.Position);
}
+DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testTdf128820, "tdf128820.fodt")
+{
+ // Import of exported DOCX failed because of wrong namespase used for wsp
element
+ // Now test the exported XML, in case we stop failing opening invalid files
+ xmlDocPtr pXml = parseExport("word/document.xml");
+ CPPUNIT_ASSERT(pXml);
+ // The parent wpg:wgp element has three children: wpg:cNvGrpSpPr,
wpg:grpSpPr, and wpg:wsp
+ // (if we start legitimately exporting additional children, this needs to
be adjusted to check
+ // all those, to make sure we don't export wrong elements).
+ assertXPathChildren(pXml,
+
"/w:document/w:body/w:p/w:r/mc:AlternateContent/mc:Choice/w:drawing/"
+ "wp:inline/a:graphic/a:graphicData/wpg:wgp",
+ 3);
+ assertXPath(pXml,
+
"/w:document/w:body/w:p/w:r/mc:AlternateContent/mc:Choice/w:drawing/wp:inline/"
+ "a:graphic/a:graphicData/wpg:wgp/wpg:cNvGrpSpPr");
+ assertXPath(pXml,
+
"/w:document/w:body/w:p/w:r/mc:AlternateContent/mc:Choice/w:drawing/wp:inline/"
+ "a:graphic/a:graphicData/wpg:wgp/wpg:grpSpPr");
+ // This one was pic:wsp instead of wps:wsp
+ assertXPath(pXml,
+
"/w:document/w:body/w:p/w:r/mc:AlternateContent/mc:Choice/w:drawing/wp:inline/"
+ "a:graphic/a:graphicData/wpg:wgp/wps:wsp");
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/source/filter/graphicfilter2.cxx
b/vcl/source/filter/graphicfilter2.cxx
index 2b0230165e05..774d107fed5d 100644
--- a/vcl/source/filter/graphicfilter2.cxx
+++ b/vcl/source/filter/graphicfilter2.cxx
@@ -532,83 +532,93 @@ bool GraphicDescriptor::ImpDetectPNG( SvStream& rStm,
bool bExtendedInfo )
if ( bExtendedInfo )
{
- sal_uInt8 cByte = 0;
+ do {
+ sal_uInt8 cByte = 0;
- // IHDR-Chunk
- rStm.SeekRel( 8 );
+ // IHDR-Chunk
+ rStm.SeekRel( 8 );
- // width
- rStm.ReadUInt32( nTemp32 );
- aPixSize.setWidth( nTemp32 );
+ // width
+ rStm.ReadUInt32( nTemp32 );
+ if (!rStm.good())
+ break;
+ aPixSize.setWidth( nTemp32 );
- // height
- rStm.ReadUInt32( nTemp32 );
- aPixSize.setHeight( nTemp32 );
+ // height
+ rStm.ReadUInt32( nTemp32 );
+ if (!rStm.good())
+ break;
+ aPixSize.setHeight( nTemp32 );
- // Bits/Pixel
- rStm.ReadUChar( cByte );
- nBitsPerPixel = cByte;
+ // Bits/Pixel
+ rStm.ReadUChar( cByte );
+ if (!rStm.good())
+ break;
+ nBitsPerPixel = cByte;
- // Colour type - check whether it supports alpha values
- sal_uInt8 cColType = 0;
- rStm.ReadUChar( cColType );
- bIsAlpha = bIsTransparent = ( cColType == 4 || cColType == 6 );
+ // Colour type - check whether it supports alpha values
+ sal_uInt8 cColType = 0;
+ rStm.ReadUChar( cColType );
+ if (!rStm.good())
+ break;
+ bIsAlpha = bIsTransparent = ( cColType == 4 || cColType ==
6 );
- // Planes always 1;
- // compression always
- nPlanes = 1;
+ // Planes always 1;
+ // compression always
+ nPlanes = 1;
- sal_uInt32 nLen32 = 0;
- nTemp32 = 0;
+ sal_uInt32 nLen32 = 0;
+ nTemp32 = 0;
- rStm.SeekRel( 7 );
+ rStm.SeekRel( 7 );
- // read up to the start of the image
- rStm.ReadUInt32( nLen32 );
- rStm.ReadUInt32( nTemp32 );
- while( ( nTemp32 != 0x49444154 ) && rStm.good() )
- {
- if ( nTemp32 == 0x70485973 ) // physical pixel dimensions
+ // read up to the start of the image
+ rStm.ReadUInt32( nLen32 );
+ rStm.ReadUInt32( nTemp32 );
+ while( ( nTemp32 != 0x49444154 ) && rStm.good() )
{
- sal_uLong nXRes;
- sal_uLong nYRes;
+ if ( nTemp32 == 0x70485973 ) // physical pixel
dimensions
+ {
+ sal_uLong nXRes;
+ sal_uLong nYRes;
- // horizontal resolution
- nTemp32 = 0;
- rStm.ReadUInt32( nTemp32 );
- nXRes = nTemp32;
+ // horizontal resolution
+ nTemp32 = 0;
+ rStm.ReadUInt32( nTemp32 );
+ nXRes = nTemp32;
- // vertical resolution
- nTemp32 = 0;
- rStm.ReadUInt32( nTemp32 );
- nYRes = nTemp32;
+ // vertical resolution
+ nTemp32 = 0;
+ rStm.ReadUInt32( nTemp32 );
+ nYRes = nTemp32;
- // unit
- cByte = 0;
- rStm.ReadUChar( cByte );
+ // unit
+ cByte = 0;
+ rStm.ReadUChar( cByte );
- if ( cByte )
- {
- if ( nXRes )
- aLogSize.setWidth( (aPixSize.Width() * 100000)
/ nXRes );
+ if ( cByte )
+ {
+ if ( nXRes )
+ aLogSize.setWidth( (aPixSize.Width() *
100000) / nXRes );
+
+ if ( nYRes )
+ aLogSize.setHeight( (aPixSize.Height() *
100000) / nYRes );
+ }
- if ( nYRes )
- aLogSize.setHeight( (aPixSize.Height() *
100000) / nYRes );
+ nLen32 -= 9;
+ }
+ else if ( nTemp32 == 0x74524e53 ) // transparency
+ {
+ bIsTransparent = true;
+ bIsAlpha = ( cColType != 0 && cColType != 2 );
}
- nLen32 -= 9;
- }
- else if ( nTemp32 == 0x74524e53 ) // transparency
- {
- bIsTransparent = true;
- bIsAlpha = ( cColType != 0 && cColType != 2 );
+ // skip forward to next chunk
+ rStm.SeekRel( 4 + nLen32 );
+ rStm.ReadUInt32( nLen32 );
+ rStm.ReadUInt32( nTemp32 );
}
-
- // skip forward to next chunk
- rStm.SeekRel( 4 + nLen32 );
- rStm.ReadUInt32( nLen32 );
- rStm.ReadUInt32( nTemp32 );
- }
+ } while (false);
}
}
}
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 83e78ad7d7ae..cb8f2e312fca 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -2568,7 +2568,18 @@ void DomainMapper_Impl::PopShapeContext()
// Relative width calculations deferred until section's margins are
defined.
// Being cautious: only deferring undefined/minimum-width shapes in
order to avoid causing potential regressions
- if( xShape->getSize().Width <= 2 )
+ css::awt::Size aShapeSize;
+ try
+ {
+ aShapeSize = xShape->getSize();
+ }
+ catch (const css::uno::RuntimeException& e)
+ {
+ // May happen e.g. when text frame has no frame format
+ // See sw/qa/extras/ooxmlimport/data/n779627.docx
+ SAL_WARN("writerfilter.dmapper", "getSize failed. " << e.Message);
+ }
+ if( aShapeSize.Width <= 2 )
{
const uno::Reference<beans::XPropertySet> xShapePropertySet(
xShape, uno::UNO_QUERY );
SectionPropertyMap* pSectionContext = GetSectionContext();
diff --git a/writerfilter/source/dmapper/PropertyMap.cxx
b/writerfilter/source/dmapper/PropertyMap.cxx
index cd59be0f11ef..f8f703939e8a 100644
--- a/writerfilter/source/dmapper/PropertyMap.cxx
+++ b/writerfilter/source/dmapper/PropertyMap.cxx
@@ -1669,7 +1669,16 @@ void SectionPropertyMap::CloseSectionGroup(
DomainMapper_Impl& rDM_Impl )
if (
xShapePropertySet->getPropertySetInfo()->hasPropertyByName(sPropRelativeWidth) )
{
sal_uInt16 nPercent = 0;
- xShapePropertySet->getPropertyValue( sPropRelativeWidth ) >>=
nPercent;
+ try
+ {
+ xShapePropertySet->getPropertyValue(sPropRelativeWidth)
>>= nPercent;
+ }
+ catch (const css::uno::RuntimeException& e)
+ {
+ // May happen e.g. when text frame has no frame format
+ // See sw/qa/extras/ooxmlimport/data/n779627.docx
+ SAL_WARN("writerfilter", "Getting relative width failed. "
<< e.Message);
+ }
if ( nPercent )
{
const sal_Int32 nWidth = nParagraphWidth * nPercent / 100;
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
