Bug 76209 - localc crash on reading conditional formats
Summary: localc crash on reading conditional formats
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.1.3.2 release
Hardware: All All
: medium normal
Assignee: Markus Mohrhard
URL:
Whiteboard: target:4.3.0
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-15 15:48 UTC by M Welinder
Modified: 2014-06-23 00:09 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
File with a range of conditional formatting (12.00 KB, application/vnd.ms-excel)
2014-03-15 15:48 UTC, M Welinder
Details
Same file with row 36 removed (12.00 KB, application/vnd.ms-excel)
2014-03-15 16:49 UTC, M Welinder
Details
console logs + bt with symbols (12.32 KB, text/plain)
2014-03-15 19:27 UTC, Julien Nabet
Details
bt from warning (2.07 KB, text/plain)
2014-03-15 19:42 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description M Welinder 2014-03-15 15:48:39 UTC
Created attachment 95859 [details]
File with a range of conditional formatting

With upcoming file:

welinder@toshiba ~/gnome/gnumeric/test $ localc ~/ccc.xls 
terminate called after throwing an instance of 'boost::bad_index'
  what():  'at()' out of bounds


The file was generated from gnumeric git:

welinder@toshiba ~/gnome/gnumeric/test $ ../src/ssconvert ../samples/cond-format-tests.gnumeric ~/ccc.xls


Note: I am not sure this xls file is actually valid.  I *think* it is, but
LO shouldn't crash regardless.
Comment 1 M Welinder 2014-03-15 16:49:35 UTC
Created attachment 95861 [details]
Same file with row 36 removed

This is the same file, except that row 36 has been removed.  That is the
row that tests applying horizontal right-alignment via a conditional format.
Comment 2 Julien Nabet 2014-03-15 19:27:55 UTC
Created attachment 95871 [details]
console logs + bt with symbols

On pc Debian x86-64 with master sources updated yesterday, I could reproduce the problem.
Comment 3 Julien Nabet 2014-03-15 19:42:41 UTC
Created attachment 95872 [details]
bt from warning

I put a break on the warning and got this bt.
Comment 4 Julien Nabet 2014-03-15 19:53:28 UTC
Kohei/Markus/Eike: any idea from the 2 bts?
Comment 5 Markus Mohrhard 2014-03-20 00:52:19 UTC
According to our code that file is invalid but I just added a check to avoid crashing with a 0 based index. According to the code we assume a 1 based index at this point.
Comment 6 M Welinder 2014-03-20 14:28:48 UTC
I note that Excel is apparently happy with that file.

What field in what record do you believe is invalid?
Comment 7 Commit Notification 2014-03-20 14:51:41 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=96926d2f2f90ad8b0aadbc0f5e915abbce6a2518

don't crash with possibly invalid index, fdo#76209



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 8 M Welinder 2014-03-20 15:18:00 UTC
Ok, nevermind my question.  There is nothing wrong with the file.

The real problem is in function ReadCF, see
http://docs.libreoffice.org/sc/html/xicontent_8cxx_source.html

1. The code fails to take the optional number format record into account.
   The record, when present, lives before the font part.  This particular
   cf does not have it, but others in the file do.
   Search for DXFNum in MS-XLS.


2. This code fails to take the optional alignment record into account.
   The record, when present, lives between font and border.


(2) is what sends it off to the deep end here.  It is trying to parse the
alignment record as a formula.  That is obviously not going to work very
well.

The committed patch is fine in and of itself.  It's just not enough.
Comment 9 Commit Notification 2014-04-06 14:31:00 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=74116032d3ebab422ba8508f42198d101822024f

import alignment dxf record, related fdo#76209



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 10 Commit Notification 2014-04-06 14:31:12 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ac6773e1e3e1382990c0f1942001b8b07cdc4682

import dxf number format record, related fdo#76209



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2014-04-06 14:31:25 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=570200983a0eded4a4d3bfcefc6a4cfa3d388f1d

import dxf protection record from xls, related fdo#76209



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.