Bug 93949 - FILESAVE: Saving as XLS results in invalid/broken document without chart
Summary: FILESAVE: Saving as XLS results in invalid/broken document without chart
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.4.0.0.alpha0+ Master
Hardware: Other All
: medium normal
Assignee: Tomaz Vajngerl
URL:
Whiteboard: target:5.2.0 target:5.1.0.2 target:5...
Keywords: bibisected, bisected, filter:xls, regression
Depends on:
Blocks:
 
Reported: 2015-09-05 19:26 UTC by Luke
Modified: 2016-10-25 19:11 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Luke 2015-09-05 19:26:23 UTC
When testing Bug 78558, I discovered that Calc is no longer exporting valid .XLS files. 

Steps to reproduce:
1. Open attachment 98860 [details] in Calc 4.4 or newer
2. Save as .XLS
3. Open in Excel

Both Excel 2007 and 2013 will generate errors when opening the file. Excel 2007 gives the following error: attachment 118436 [details]
Excel 2013 says: "File Error: Data may have been lost". If you click OK, the bar chart is not displayed.

Calc 4.3 generates valid XLS files.
Comment 1 raal 2015-09-08 12:47:14 UTC
I can confirm with Verze: 5.0.1.2
ID sestavení: 81898c9f5c0d43f3473ba111d7b351050be20261, win7
Comment 2 Robinson Tryon (qubit) 2015-12-14 05:32:45 UTC Comment hidden (obsolete)
Comment 3 raal 2015-12-20 18:10:56 UTC
This seems to have begun at the below commit.
Adding Cc: to Tomaž Vajngerl ; Could you possibly take a look at this one?
Thanks
 895dbd837c814ed16c7d3b2b572f11456b2e715f is the first bad commit
commit 895dbd837c814ed16c7d3b2b572f11456b2e715f
Author: Matthew Francis <mjay.francis@gmail.com>
Date:   Sun Mar 15 01:18:22 2015 +0800

    source-hash-e97997f1e56731b97d469f775994ef3d05f17d35
    
    commit e97997f1e56731b97d469f775994ef3d05f17d35
    Author:     Tomaž Vajngerl <tomaz.vajngerl@collabora.com>
    AuthorDate: Sat Aug 16 18:31:07 2014 +0200
    Commit:     Tomaž Vajngerl <tomaz.vajngerl@collabora.com>
    CommitDate: Sat Aug 16 20:49:19 2014 +0200
    
        SvMemoryStream.remainingSize ret. size to end of data not buffer
    
        SvMemoryStream remainingSize returned the size from current
        position to internal buffer size instead to end of data. This
        was not consistent with what remainingSize description says on
        SvStream (and other SvStream implementations work) and what the
        user expects.
    
        Change-Id: I7ff391754a386c5f067a4bd4eed2ee7f2d7fd77e

:040000 040000 09351896e432d6d0d7dae36f19b2f3e44fd946de 502d13f0bed7068431735929d688fa91670ca2e2 M	opt
Comment 4 Commit Notification 2015-12-20 21:22:28 UTC
Tomaž Vajngerl committed a patch related to this issue.
It has been pushed to "master":

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

tdf#93949 ensure memory stream is properly flushed

It will be available in 5.2.0.

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 5 Commit Notification 2015-12-21 13:51:05 UTC
Tomaž Vajngerl committed a patch related to this issue.
It has been pushed to "libreoffice-5-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f3f90b6490298fdf79d5f9e19c788f0b69e0818e&h=libreoffice-5-1

tdf#93949 ensure memory stream is properly flushed

It will be available in 5.1.0.2.

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 6 Commit Notification 2015-12-21 14:09:38 UTC
Tomaž Vajngerl committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a0a35440453102d215456e3e2f16b4ee207b16de&h=libreoffice-5-0

tdf#93949 ensure memory stream is properly flushed

It will be available in 5.0.5.

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 7 Luke 2015-12-22 21:46:22 UTC
Verified fixed. Excel can now read LibreOffice generated files with charts again again. Thank Tomaž!

Tomaž, Could there be other instances that depend on GetBufSize()'s flush or is this a one off?
Comment 8 Tomaz Vajngerl 2015-12-22 22:54:57 UTC
I wonder that as well. Potential bugs could exist but this is the only one that surfaced (as far as I know). 

GetBufSize is wrong there however - this is the size of the internal allocated buffer which is usually larger, so the internal buffer doesn't need to be resized so often. Using that for remainingSize() is wrong - that should return the size from current position and the end of data in the stream. I was quite surprised that there was a regression because of this change.  I wonder if there is a bug elsewhere in SvMemoryStream but with a quick look I didn't find anything obvious.