Issue 119502 - [From Symphony][Crash]When save the file as ppt, AOO crashes
Summary: [From Symphony][Crash]When save the file as ppt, AOO crashes
Status: CLOSED FIXED
Alias: None
Product: Impress
Classification: Application
Component: save-export (show other issues)
Version: 3.4.0
Hardware: PC All
: P3 Critical (vote)
Target Milestone: 4.0.0
Assignee: Andre
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-29 07:11 UTC by Du Jing
Modified: 2012-12-26 08:10 UTC (History)
6 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
sample file (715.66 KB, application/vnd.oasis.opendocument.presentation)
2012-05-29 08:39 UTC, Du Jing
no flags Details
simple sample file which can reproduced this defect (9.82 KB, application/vnd.oasis.opendocument.presentation)
2012-06-07 02:14 UTC, SunYing
no flags Details
the shape screenshots which cause export crash (1.26 KB, image/png)
2012-06-07 02:19 UTC, SunYing
no flags Details
the shape description in content.xml (19.09 KB, image/png)
2012-06-07 02:21 UTC, SunYing
no flags Details
patch for ppt shape export crash (652 bytes, patch)
2012-06-11 08:16 UTC, SunYing
sunyingshadesun: review?
Details | Diff
patch for ppt shape export crash (713 bytes, patch)
2012-06-12 05:36 UTC, SunYing
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description Du Jing 2012-05-29 07:11:01 UTC
Build info: AOO3.4_1327774

Steps:
1 Open the sample file
2 Save the file as a ppt
3 An error message pops up.
Comment 1 Du Jing 2012-05-29 08:39:32 UTC
Created attachment 77722 [details]
sample file
Comment 2 SunYing 2012-06-05 02:39:58 UTC
I am checking this defect.
Comment 3 SunYing 2012-06-07 02:14:59 UTC
Created attachment 78079 [details]
simple sample file which can reproduced this defect
Comment 4 SunYing 2012-06-07 02:19:08 UTC
Created attachment 78080 [details]
the shape screenshots which cause export crash
Comment 5 SunYing 2012-06-07 02:21:17 UTC
Created attachment 78081 [details]
the shape description in content.xml
Comment 6 SunYing 2012-06-07 02:36:21 UTC
Root cause:
   In attach simple sample file, the shape which named customshape3 have the path "M ?f0 ?f0 L ?f0 ?f0 ?f0 ?f0 ?f0 ?f0 Z N" which contian "?f0"parameter, and "?f0" parameter have not been defined, so the vector of equation parameter order is empty, read vector directly without adjudgement when export, and crash occur.

Solution:
  add adjudgement before read vector.
Comment 7 SunYing 2012-06-11 08:16:11 UTC
Created attachment 78228 [details]
patch for ppt shape export crash
Comment 8 Andre 2012-06-11 13:55:12 UTC
Reviewing.
Comment 9 Andre 2012-06-11 15:09:39 UTC
Two remarks/questions:

1) The attached patch fixes the crash for nValue==0.
There may be other cases where nValue is outside valid bounds.
I would suggest a little modification, something like:

    OSL_ASSERT(nValue < rEquationOrder.size());
    if (nValue < rEquationOrder.size())
    {
        nValue = (sal_uInt16)rEquationOrder[ nValue ];
        nValue |= (sal_uInt32)0x80000000;
    }

2) Regarding the root cause: is the document broken, or the code that interprets the path description.  If it is the second then we need a fix there.
Comment 10 SunYing 2012-06-12 02:06:55 UTC
I think the root cause is the first.
because the sample file has a path which contian "?f0" parameter, and "?f0" parameter have not been defined, but in the ODF standard, the parameter must defined,So it is document problem. 
The following is enhanced path and parameter definition in ODF standard.

The definition for "Enhanced Path" is that the draw:enhanced-path attribute specifies a path similar to the svg:d attribute of the <svg:path> element. Instructions such as moveto, lineto, arcto and other instructions together with its parameter are describing the geometry of a shape which can be filled and or
stroked.

A parameter can also have one of the following enhancements:
1.A “?” is used to mark the beginning of a formula name. The result of the element's draw:formula attribute is used as parameter value in this case.
2. If “$” is preceding a integer value, the value is a indexing a draw:modifiers attribute. The corresponding modifier value is used as parameter value then.
Comment 11 SunYing 2012-06-12 02:39:15 UTC
I fixed the crash for rEquationOrder.size() == 0 and nValue >=0, and not for nValue == 0.
we should fixed the nValue is outside valid bounds.
I would like modify the code like the following:

  OSL_ASSERT(rEquationOrder.size()!=0);
  OSL_ASSERT(nValue < rEquationOrder.size());
  if (rEquationOrder.size()!=0 && nValue < rEquationOrder.size())
  {
    nValue = (sal_uInt16)rEquationOrder[ nValue ];
    nValue |= (sal_uInt32)0x80000000;
  }
pls review again.
Comment 12 SunYing 2012-06-12 04:34:32 UTC
I'm so sorry for my following mistake, Andre's suggestion is right, I will adopt his comments.

(In reply to comment #11)
> I fixed the crash for rEquationOrder.size() == 0 and nValue >=0, and not for
> nValue == 0.
we should fixed the nValue is outside valid bounds.
I would
> like modify the code like the following:

 
> OSL_ASSERT(rEquationOrder.size()!=0);
  OSL_ASSERT(nValue <
> rEquationOrder.size());
  if (rEquationOrder.size()!=0 && nValue <
> rEquationOrder.size())
  {
    nValue = (sal_uInt16)rEquationOrder[ nValue
> ];
    nValue |= (sal_uInt32)0x80000000;
  }
pls review again.
Comment 13 SunYing 2012-06-12 05:36:48 UTC
Created attachment 78242 [details]
patch for ppt shape export crash

update patch
Comment 14 Andre 2012-06-12 07:24:12 UTC
Commited the new patch. Thanks for the good work.
SVN revision is 1349163.
Comment 15 louqle 2012-06-18 06:33:24 UTC
verified in r1350879 on windows 7