mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Only access css::drawing::PointSequence when not empty

Browse Source 
Had to adapt EscherPropertyContainer::GetPolyPolygon due
to it using conversions from UNO API drawing::PointSequence
to old tools::polygon, containing unsafe memory accesses.
It is not useful to fix that, use new tooling instead.

Before correctly testing nCount for zero in b2dpolygontools.cxx
when you look closely always a point was added - a random
one due to accessing random memory.
This is corrected, so in test case for "tdf104115.docx"
a PolyPolygon with two polys is used, the first being
empty (had one point due to the error mentioned above).
When having no points, CreatePolygonProperties in escherex.cxx
does badly calculate and alloc the pSegmentBuf array used
to write to doc formats (in the test case - 20 bytes alloced,
22 written). This did not happen before due to having
always a point due to the error before - argh!
Corrected that and hopefully this will work now.

To be on the safe side and to not need to redefine that whole
CreatePolygonProperties I will turn back that little change
and better sort-out empty polygons inside GetPolyPolygon
alrteady. That should bring us back to the original state,
at the same time avoiding that CreatePolygonProperties has
to handle empty Polygons at all.
That stuff urgently needs cleanup - I took a look and thought
about using std::vector<sal_uInt8> so no wrong alloc or write
too much could happen, but that nTotalBezPoints needs to be
pre-calculated because it gets itself written to that
buffers...

Change-Id: Iefc885928f5bb29bceaf36c2a1555346bb21fd26
Reviewed-on: https://gerrit.libreoffice.org/56927
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@cib.de>
This commit is contained in:
Armin Le Grand
2018-07-04 10:13:16 +02:00
parent 7e971b500f
commit f73eeabf6e
2 changed files with 97 additions and 175 deletions
Download Patch File Download Diff File Expand all files Collapse all files

146
basegfx/source/polygon/b2dpolygontools.cxx
View File

@@ -3285,91 +3285,95 @@ namespace basegfx
// prepare new polygon
B2DPolygon aRetval;
const css::awt::Point* pPointSequence = rPointSequenceSource.getConstArray();
const css::drawing::PolygonFlags* pFlagSequence = rFlagSequenceSource.getConstArray();
// get first point and flag
B2DPoint aNewCoordinatePair(pPointSequence->X, pPointSequence->Y); pPointSequence++;
css::drawing::PolygonFlags ePolygonFlag(*pFlagSequence); pFlagSequence++;
B2DPoint aControlA;
B2DPoint aControlB;
// first point is not allowed to be a control point
OSL_ENSURE(ePolygonFlag != css::drawing::PolygonFlags_CONTROL,
"UnoPolygonBezierCoordsToB2DPolygon: Start point is a control point, illegal input polygon (!)");
// add first point as start point
aRetval.append(aNewCoordinatePair);
for(sal_uInt32 b(1); b < nCount;)
if(0 != nCount)
{
// prepare loop
bool bControlA(false);
bool bControlB(false);
const css::awt::Point* pPointSequence = rPointSequenceSource.getConstArray();
const css::drawing::PolygonFlags* pFlagSequence = rFlagSequenceSource.getConstArray();
// get next point and flag
aNewCoordinatePair = B2DPoint(pPointSequence->X, pPointSequence->Y);
ePolygonFlag = *pFlagSequence;
pPointSequence++; pFlagSequence++; b++;
// get first point and flag
B2DPoint aNewCoordinatePair(pPointSequence->X, pPointSequence->Y); pPointSequence++;
css::drawing::PolygonFlags ePolygonFlag(*pFlagSequence); pFlagSequence++;
B2DPoint aControlA;
B2DPoint aControlB;
if(b < nCount && ePolygonFlag == css::drawing::PolygonFlags_CONTROL)
// first point is not allowed to be a control point
OSL_ENSURE(ePolygonFlag != css::drawing::PolygonFlags_CONTROL,
"UnoPolygonBezierCoordsToB2DPolygon: Start point is a control point, illegal input polygon (!)");
// add first point as start point
aRetval.append(aNewCoordinatePair);
for(sal_uInt32 b(1); b < nCount;)
{
aControlA = aNewCoordinatePair;
bControlA = true;
// prepare loop
bool bControlA(false);
bool bControlB(false);
// get next point and flag
aNewCoordinatePair = B2DPoint(pPointSequence->X, pPointSequence->Y);
ePolygonFlag = *pFlagSequence;
pPointSequence++; pFlagSequence++; b++;
if(b < nCount && ePolygonFlag == css::drawing::PolygonFlags_CONTROL)
{
aControlA = aNewCoordinatePair;
bControlA = true;
// get next point and flag
aNewCoordinatePair = B2DPoint(pPointSequence->X, pPointSequence->Y);
ePolygonFlag = *pFlagSequence;
pPointSequence++; pFlagSequence++; b++;
}
if(b < nCount && ePolygonFlag == css::drawing::PolygonFlags_CONTROL)
{
aControlB = aNewCoordinatePair;
bControlB = true;
// get next point and flag
aNewCoordinatePair = B2DPoint(pPointSequence->X, pPointSequence->Y);
ePolygonFlag = *pFlagSequence;
pPointSequence++; pFlagSequence++; b++;
}
// two or no control points are consumed, another one would be an error.
// It's also an error if only one control point was read
SAL_WARN_IF(ePolygonFlag == css::drawing::PolygonFlags_CONTROL || bControlA != bControlB,
"basegfx", "UnoPolygonBezierCoordsToB2DPolygon: Illegal source polygon (!)");
// the previous writes used the B2DPolyPoygon -> utils::PolyPolygon converter
// which did not create minimal PolyPolygons, but created all control points
// as null vectors (identical points). Because of the former P(CA)(CB)-norm of
// B2DPolygon and it's unused sign of being the zero-vector and CA and CB being
// relative to P, an empty edge was exported as P == CA == CB. Luckily, the new
// export format can be read without errors by the old OOo-versions, so we need only
// to correct here at read and do not need to export a wrong but compatible version
// for the future.
if(bControlA
&& aControlA.equal(aControlB)
&& aControlA.equal(aRetval.getB2DPoint(aRetval.count() - 1)))
{
bControlA = false;
}
if(bControlA)
{
// add bezier edge
aRetval.appendBezierSegment(aControlA, aControlB, aNewCoordinatePair);
}
else
{
// add edge
aRetval.append(aNewCoordinatePair);
}
}
if(b < nCount && ePolygonFlag == css::drawing::PolygonFlags_CONTROL)
{
aControlB = aNewCoordinatePair;
bControlB = true;
// get next point and flag
aNewCoordinatePair = B2DPoint(pPointSequence->X, pPointSequence->Y);
ePolygonFlag = *pFlagSequence;
pPointSequence++; pFlagSequence++; b++;
}
// two or no control points are consumed, another one would be an error.
// It's also an error if only one control point was read
SAL_WARN_IF(ePolygonFlag == css::drawing::PolygonFlags_CONTROL || bControlA != bControlB,
"basegfx", "UnoPolygonBezierCoordsToB2DPolygon: Illegal source polygon (!)");
// the previous writes used the B2DPolyPoygon -> utils::PolyPolygon converter
// which did not create minimal PolyPolygons, but created all control points
// as null vectors (identical points). Because of the former P(CA)(CB)-norm of
// B2DPolygon and it's unused sign of being the zero-vector and CA and CB being
// relative to P, an empty edge was exported as P == CA == CB. Luckily, the new
// export format can be read without errors by the old OOo-versions, so we need only
// to correct here at read and do not need to export a wrong but compatible version
// for the future.
if(bControlA
&& aControlA.equal(aControlB)
&& aControlA.equal(aRetval.getB2DPoint(aRetval.count() - 1)))
{
bControlA = false;
}
if(bControlA)
{
// add bezier edge
aRetval.appendBezierSegment(aControlA, aControlB, aNewCoordinatePair);
}
else
{
// add edge
aRetval.append(aNewCoordinatePair);
}
// #i72807# API import uses old line start/end-equal definition for closed,
// so we need to correct this to closed state here
checkClosed(aRetval);
}
// #i72807# API import uses old line start/end-equal definition for closed,
// so we need to correct this to closed state here
checkClosed(aRetval);
return aRetval;
}

126
filter/source/msfilter/escherex.cxx
View File

@@ -93,6 +93,8 @@
#include <vcl/virdev.hxx>
#include <rtl/crc.h>
#include <rtl/strbuf.hxx>
#include <basegfx/polygon/b2dpolypolygontools.hxx>
#include <basegfx/polygon/b2dpolygontools.hxx>
#include <memory>
using namespace css;
@@ -1735,120 +1737,36 @@ tools::PolyPolygon EscherPropertyContainer::GetPolyPolygon( const uno::Reference
return aRetPolyPoly;
}
// adapting to basegfx::B2DPolyPolygon now, has no sense to do corrections in the
// old tools::PolyPolygon creation code. Convert to that at return time
tools::PolyPolygon EscherPropertyContainer::GetPolyPolygon( const uno::Any& rAny )
{
bool bNoError = true;
basegfx::B2DPolyPolygon aRetval;
tools::Polygon aPolygon;
tools::PolyPolygon aPolyPolygon;
if ( rAny.getValueType() == cppu::UnoType<drawing::PolyPolygonBezierCoords>::get())
if(auto pBCC = o3tl::tryAccess<drawing::PolyPolygonBezierCoords>(rAny))
{
auto pSourcePolyPolygon = o3tl::doAccess<drawing::PolyPolygonBezierCoords>(rAny);
sal_uInt16 nOuterSequenceCount = static_cast<sal_uInt16>(pSourcePolyPolygon->Coordinates.getLength());
aRetval = basegfx::utils::UnoPolyPolygonBezierCoordsToB2DPolyPolygon(*pBCC);
}
else if(auto pCC = o3tl::tryAccess<drawing::PointSequenceSequence>(rAny))
{
aRetval = basegfx::utils::UnoPointSequenceSequenceToB2DPolyPolygon(*pCC);
}
else if(auto pC = o3tl::tryAccess<drawing::PointSequence>(rAny))
{
aRetval.append(basegfx::utils::UnoPointSequenceToB2DPolygon(*pC));
}
// get pointer of inner sequences
drawing::PointSequence const * pOuterSequence = pSourcePolyPolygon->Coordinates.getConstArray();
drawing::FlagSequence const * pOuterFlags = pSourcePolyPolygon->Flags.getConstArray();
basegfx::B2DPolyPolygon aRetval2;
bNoError = pOuterSequence && pOuterFlags;
if ( bNoError )
for(sal_uInt32 a(0); a < aRetval.count(); a++)
{
if(0 != aRetval.getB2DPolygon(a).count())
{
sal_uInt16 a, b, nInnerSequenceCount;
awt::Point const * pArray;
// this will be a polygon set
for ( a = 0; a < nOuterSequenceCount; a++ )
{
drawing::PointSequence const * pInnerSequence = pOuterSequence++;
drawing::FlagSequence const * pInnerFlags = pOuterFlags++;
bNoError = pInnerSequence && pInnerFlags;
if ( bNoError )
{
// get pointer to arrays
pArray = pInnerSequence->getConstArray();
drawing::PolygonFlags const * pFlags = pInnerFlags->getConstArray();
if ( pArray && pFlags )
{
nInnerSequenceCount = static_cast<sal_uInt16>(pInnerSequence->getLength());
aPolygon = tools::Polygon( nInnerSequenceCount );
for( b = 0; b < nInnerSequenceCount; b++)
{
drawing::PolygonFlags ePolyFlags = *pFlags++;
awt::Point aPoint( *(pArray++) );
aPolygon[ b ] = Point( aPoint.X, aPoint.Y );
aPolygon.SetFlags( b, static_cast<PolyFlags>(ePolyFlags) );
if ( ePolyFlags == drawing::PolygonFlags_CONTROL )
continue;
}
aPolyPolygon.Insert( aPolygon );
}
}
}
aRetval2.append(aRetval.getB2DPolygon(a));
}
}
else if ( auto pSourcePolyPolygon = o3tl::tryAccess<drawing::PointSequenceSequence>(rAny) )
{
sal_uInt16 nOuterSequenceCount = static_cast<sal_uInt16>(pSourcePolyPolygon->getLength());
// get pointer to inner sequences
drawing::PointSequence const * pOuterSequence = pSourcePolyPolygon->getConstArray();
bNoError = pOuterSequence != nullptr;
if ( bNoError )
{
sal_uInt16 a, b, nInnerSequenceCount;
// this will be a polygon set
for( a = 0; a < nOuterSequenceCount; a++ )
{
drawing::PointSequence const * pInnerSequence = pOuterSequence++;
bNoError = pInnerSequence != nullptr;
if ( bNoError )
{
// get pointer to arrays
awt::Point const * pArray =
pInnerSequence->getConstArray();
if ( pArray != nullptr )
{
nInnerSequenceCount = static_cast<sal_uInt16>(pInnerSequence->getLength());
aPolygon = tools::Polygon( nInnerSequenceCount );
for( b = 0; b < nInnerSequenceCount; b++)
{
aPolygon[ b ] = Point( pArray->X, pArray->Y );
pArray++;
}
aPolyPolygon.Insert( aPolygon );
}
}
}
}
}
else if ( auto pInnerSequence = o3tl::tryAccess<drawing::PointSequence>(rAny) )
{
bNoError = pInnerSequence != nullptr;
if ( bNoError )
{
sal_uInt16 a, nInnerSequenceCount;
// get pointer to arrays
awt::Point const * pArray = pInnerSequence->getConstArray();
if ( pArray != nullptr )
{
nInnerSequenceCount = static_cast<sal_uInt16>(pInnerSequence->getLength());
aPolygon = tools::Polygon( nInnerSequenceCount );
for( a = 0; a < nInnerSequenceCount; a++)
{
aPolygon[ a ] = Point( pArray->X, pArray->Y );
pArray++;
}
aPolyPolygon.Insert( aPolygon );
}
}
}
return aPolyPolygon;
return tools::PolyPolygon(aRetval2);
}
bool EscherPropertyContainer::CreatePolygonProperties(