[Libreoffice-commits] core.git: vcl/source

[Stephan Bergmann (via logerrit)]

 vcl/source/gdi/pdfwriter_impl.cxx |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

New commits:
commit 04b7fdb5677315d191c191ba381cc48dff183765
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Sat Jun 22 00:09:02 2019 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Tue Jul 23 16:26:12 2019 +0200

    Avoid overflowing conversion from double to sal_Int32
    
    ...by clamping the value to the sal_Int32 range.  An alternative would be to
    instead print nAdjustment as a real value (which the PDF standard apparently
    supports, without giving limits to such real values), as had been implemented at
    <https://gerrit.libreoffice.org/#/c/74543/1>, but that was deemed unnecessarily
    complex, as no sane document should require nAdjustment values outside the
    sal_Int32 range.
    
    `--convert-to pdf caolan/drawinglayer_emfphelperdata_heap_use_after_free.sample`
    (from the crashtestdata files) has cases where
    
      double fAdvance = aThisPos.X() - aPrevPos.X();
    
    gets rather large (whether or not that's faithful to the input document, or a
    consequence of an earlier import error), so failed with
    
    > vcl/source/gdi/pdfwriter_impl.cxx:6078:66: runtime error: -5.83192e+09 is outside the range of representable values of type 'int'
    >  #0 in vcl::PDFWriterImpl::drawHorizontalGlyphs(std::__debug::vector<vcl::PDFWriterImpl::PDFGlyph, std::allocator<vcl::PDFWriterImpl::PDFGlyph> > const&, rtl::OStringBuffer&, Point const&, bool, double, double, double, int, int) at vcl/source/gdi/pdfwriter_impl.cxx:6078:66
    >  #1 in vcl::PDFWriterImpl::drawLayout(SalLayout&, rtl::OUString const&, bool) at vcl/source/gdi/pdfwriter_impl.cxx:6404:17
    >  #2 in vcl::PDFWriterImpl::drawTextArray(Point const&, rtl::OUString const&, long const*, int, int) at vcl/source/gdi/pdfwriter_impl.cxx:6621:9
    >  #3 in vcl::PDFWriter::DrawTextArray(Point const&, rtl::OUString const&, long const*, int, int) at vcl/source/gdi/pdfwriter.cxx:87:22
    >  #4 in vcl::PDFWriterImpl::playMetafile(GDIMetaFile const&, vcl::PDFExtOutDevData*, vcl::PDFWriter::PlayMetafileContext const&, VirtualDevice*) at vcl/source/gdi/pdfwriter_impl2.cxx:878:34
    [...]
    
    In the original compuation of sal_Int32 nAdjustment, the "+ 0.5" was presumably
    intended to round to the nearest integer, even though it would have rounded
    towards zero for negative values (as conversion to integer truncates).  So use
    std::round to always round to the nearest integer, including for negative
    values.
    
    Change-Id: Ie3ddbb66421f47417c6d9ae096f2207a29aca4a4
    Reviewed-on: https://gerrit.libreoffice.org/74543
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/vcl/source/gdi/pdfwriter_impl.cxx b/vcl/source/gdi/pdfwriter_impl.cxx
index 6515344d0644..a00fe2767f9e 100644
--- a/vcl/source/gdi/pdfwriter_impl.cxx
+++ b/vcl/source/gdi/pdfwriter_impl.cxx
@@ -6100,7 +6100,12 @@ void PDFWriterImpl::drawHorizontalGlyphs(
             const Point aPrevPos = aMat.transform( rGlyphs[nPos-1].m_aPos );
             double fAdvance = aThisPos.X() - aPrevPos.X();
             fAdvance *= 1000.0 / nPixelFontHeight;
-            const sal_Int32 nAdjustment = static_cast<sal_Int32>(rGlyphs[nPos-1].m_nNativeWidth - fAdvance + 0.5);
+            const double fAdjustment = rGlyphs[nPos-1].m_nNativeWidth - fAdvance + 0.5;
+            SAL_WARN_IF(
+                fAdjustment < SAL_MIN_INT32 || fAdjustment > SAL_MAX_INT32, "vcl.pdfwriter",
+                "adjustment " << fAdjustment << " outside 32-bit int");
+            const sal_Int32 nAdjustment = static_cast<sal_Int32>(
+                std::clamp(fAdjustment, double(SAL_MIN_INT32), double(SAL_MAX_INT32)));
             if( nAdjustment != 0 )
             {
                 // apply individual glyph positioning