Re: Fix for a crash test?

On 25/02/14 13:05, Miklos Vajna wrote:
> Hi Julien,
>
> On Sat, Feb 22, 2014 at 07:05:39AM -0800, julien2412 <serval2412@yahoo.fr> wrote:
> > On pc Debian x86-64 with master sources updated yesterday, I reproduced the
> > crash fdo51430-1.doc.
> > The bt shows a failing assert because of the line 2423 of this block:
> >    2415         else
> >    2416         {
> >    2417             //What I do here, is that I examine the attributes, and
> > if
> >    2418             //I find out, that it's CJK/CTL, then I set the
> > paragraph space
> >    2419             //to the value set in
> > HTML_CJK_PARSPACE/HTML_CTL_PARSPACE.
> >    2420 
> >    2421             sal_Bool bIsCJK = false;
> >    2422             sal_Bool bIsCTL = false;
> >    2423             SwpHints& rHints = pTxtNode->GetSwpHints(); //// HERE
> >    2424             sal_uInt16 nWhich;
> >    2425             SwTxtAttr *pHt;
> >
> > (see
> > http://opengrok.libreoffice.org/xref/core/sw/source/filter/html/swhtml.cxx#2415)
> >
> > If I check pTxtNode->HasHints()  on SwHTMLParser::AddParSpace in
> > sw/source/filter/html/swhtml.cxx, no crash. But as often, I wonder if it's
> > the good fix or if I just hide a bug
>
> In general, GetSwpHints() should only be used after a HasHints() call.
> In an import filter, once again in general you are supposed to handle
> all kind of strange user input. Based on that, I would just add the
> HasHints() check there, unless you want to spend time on checking if the
> bugdoc crashed in older versions as well (and if it did not, why that
> didn't happen).

yes, what Miklos said...

apparently this was added in commit
1b0f7ee1e04d7b0fe81f7458ae2116615e0f6889 and there's another
questionable bit in there: when it finds some RES_CTL_* hint it does not
set "bCTL = true" but "bCJK = false", so bCTL is never true?

also i wonder if the HTML import code actually sets only one of the
western/cjk/ctl font height etc. attributes or all 3 at the same time...