Bug Hunting Session
Bug 60237 - : Bug in API call StoreAsURL (or in the OO Basic API description)
Summary: : Bug in API call StoreAsURL (or in the OO Basic API description)
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
3.5.7.2 release
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: BSA
Keywords: haveBacktrace
Depends on:
Blocks: Macro-VBA
  Show dependency treegraph
 
Reported: 2013-02-03 14:39 UTC by Peter Toye
Modified: 2019-05-30 14:12 UTC (History)
3 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 Peter Toye 2013-02-03 14:39:52 UTC
Problem description: When saving a file using StoreAsURL with the MediaDescriptor property "Overwrite" not set (i.e. using the default, which the documentation describes as having the value FALSE), existing files are overwritten.

Steps to reproduce:

Edit the following macro to conform to your local directory structure and OS, put a Writer file called "Test1.odt" in the directory and run the macro.

Sub Main

dim FileString as String
dim TrueProp as  Object
TrueProp = New com.sun.star.beans.PropertyValue
dim FalseProp as Object
FalseProp = New com.sun.star.beans.PropertyValue

TrueProp.Name="Overwrite"
TrueProp.Value=TRUE
FalseProp.Name="Overwrite"
FalseProp.Value=FALSE

dim EmptyArray() as Object
dim TrueArray(0) as Object
dim FalseArray(0) as Object

TrueArray(0)=TrueProp
FalseArray(0)=FalseProp

InFileString=  "D:\Peter\PC\Open Office\Bughunt\Test1.odt"  ' This file already exists
OutFile2String="D:\Peter\PC\Open Office\Bughunt\Test2.odt" 
OutFile3String="D:\Peter\PC\Open Office\Bughunt\Test3.odt" 
OutFile4String="D:\Peter\PC\Open Office\Bughunt\Test4.odt" 

dim FileURL as String

InFileURL=ConverttoURL(InFileString)
OutFile2URL=ConverttoURL(OutFile2String)
OutFile3URL=ConverttoURL(OutFile3String)
OutFile4URL=ConverttoURL(OutFile4String)

Dim doc as Object

Doc=StarDesktop.LoadComponentfromURL(InFileURL, "_blank", 0, Array())
msgbox "Document loaded"
'xray doc

Doc.StoreToURL(OutFile2URL,TrueArray)
Doc.StoreToURL(OutFile3URL,TrueArray)
Doc.StoreToURL(OutFile4URL,TrueArray)
msgbox "Documents 2,3,4 stored OK"

Doc.StoreAsURL(OutFile2URL,TrueArray)
msgbox "Document 2 stored with Overwrite set to TRUE"

Doc.StoreAsURL(OutFile3URL,EmptyArray)
msgbox "Document 3 stored with default Overwrite"

Doc.StoreAsURL(OutFile4URL,FalseArray)
msgbox "Document 4 stored with Overwrite set to FALSE"

End sub

Current behavior:

Gives an error when storing Document Test4.

Expected behavior:

Should have given an error when storing document Test3.

This also occurs in OpenOffice and has been reported as bug no 121665 there.

              
Operating System: Windows 7
Version: 3.5.7.2 release
Comment 1 Julien Nabet 2013-02-17 00:23:47 UTC
On pc Debian x86-64 with master source updated today.

Noel: one for you?
Comment 2 QA Administrators 2015-02-19 15:45:55 UTC Comment hidden (obsolete)
Comment 3 Peter Toye 2015-02-19 18:38:01 UTC
I tried it with LO 4.3.5.2 under Windows 7 64-bit home premium. No change in behaviour.

I think that this is an extremely annoying bug, even though the workaround is to ALWAYS use a property list when calling StoreToURL. The default behaviour overwrites a file without warning - never good behaviour for a default! At the very least the documentation should be changed to reflect the current behaviour of StoreToURL with a great big warning message.

Apache don't seem to agree with me and think it's trivial. Having overwritten a file (fortunately backed up) by assuming that the documentation is correct I can't agree!
Comment 4 Julien Nabet 2015-02-19 19:28:28 UTC
On pc Debian x86-64 with master sources updated today, I got this error when launching macro:
BASIC runtime error.
An exception occurred 
Type: com.sun.star.lang.IllegalArgumentException
Message: Unsupported URL <file:///home/julien/compile-libreoffice/bugs/60237_macrostore/Test1.odt>: "type detection failed".
Comment 5 Julien Nabet 2015-02-19 19:29:52 UTC
Ok, I had a bug in my macro with file name, the macro runned successfully completely.
Comment 6 Julien Nabet 2015-02-19 19:57:01 UTC
Oups, forget what I indicated, I indeed reproduced the pb, here's the error message for the fourth file:
BASIC runtime error.
An exception occurred 
Type: com.sun.star.task.ErrorCodeIOException
Message: SfxBaseModel::impl_store <file:///home/julien/compile-libreoffice/bugs/60237_macrostore/Test4.odt> failed: 0x20d.
Comment 7 Julien Nabet 2015-02-19 20:12:58 UTC
In gdb session with "catch throw ErrorCodeIOException", I could retrieved a bt, here's main part:
 #0  0x00002aaaabcb6dbd in __cxa_throw () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#1  0x00002aaaaed7c994 in SfxBaseModel::impl_store (this=0x7306430, sURL="file:///home/julien/compile-libreoffice/bugs/60237_macrostore/Test4.odt", 
    seqArguments=uno::Sequence of length 1 = {...}, bSaveTo=false) at /home/julien/compile-libreoffice/libreoffice/sfx2/source/doc/sfxbasemodel.cxx:3137
#2  0x00002aaaaed7457e in SfxBaseModel::storeAsURL (this=0x7306430, rURL="file:///home/julien/compile-libreoffice/bugs/60237_macrostore/Test4.odt", 
    rArgs=uno::Sequence of length 1 = {...}) at /home/julien/compile-libreoffice/libreoffice/sfx2/source/doc/sfxbasemodel.cxx:1662
#3  0x00002aaac3d59fef in gcc3::callVirtualMethod(void*, unsigned int, void*, _typelib_TypeDescriptionReference*, bool, unsigned long*, unsigned int, unsigned long*, double*)
    () from /home/julien/compile-libreoffice/libreoffice/instdir/program/libgcc3_uno.so
#4  0x00002aaac3d58fc5 in cpp_call (pThis=0x692d600, aVtableSlot=..., pReturnTypeRef=0x648060, nParams=2, pParams=0x684ba80, pUnoReturn=0x7fffffff3c10, 
    pUnoArgs=0x7fffffff3be0, ppUnoExc=0x7fffffff3c58) at /home/julien/compile-libreoffice/libreoffice/bridges/source/cpp_uno/gcc3_linux_x86-64/uno2cpp.cxx:231
#5  0x00002aaac3d599df in bridges::cpp_uno::shared::unoInterfaceProxyDispatch (pUnoI=0x692d600, pMemberDescr=0x684c9e0, pReturn=0x7fffffff3c10, pArgs=0x7fffffff3be0, 
    ppException=0x7fffffff3c58) at /home/julien/compile-libreoffice/libreoffice/bridges/source/cpp_uno/gcc3_linux_x86-64/uno2cpp.cxx:429
#6  0x00002aaad097bd43 in stoc_corefl::IdlInterfaceMethodImpl::invoke (this=0x684cce0, rObj=
    uno::Any {<com::sun::star::uno::XInterface> = {_vptr.XInterface = 0x7306328}, <No data fields>}, rArgs=uno::Sequence of length 2 = {...})
    at /home/julien/compile-libreoffice/libreoffice/stoc/source/corereflection/criface.cxx:706
#7  0x00002aaaae4030e3 in SbUnoObject::Notify (this=0x755bbe0, rBC=..., rHint=...) at /home/julien/compile-libreoffice/libreoffice/basic/source/classes/sbunoobj.cxx:2286
#8  0x00002aaaaf685418 in SfxBroadcaster::Broadcast (this=0x6a23930, rHint=...) at /home/julien/compile-libreoffice/libreoffice/svl/source/notify/SfxBroadcaster.cxx:51
#9  0x00002aaaae50ea0e in SbxVariable::Broadcast (this=0x747d8f0, nHintId=65536) at /home/julien/compile-libreoffice/libreoffice/basic/source/sbx/sbxvar.cxx:182
#10 0x00002aaaae508773 in SbxValue::SbxValue (this=0x76ce3e0, __vtt_parm=0x2aaaae81a210 <VTT for SbxMethod+16>, r=..., __in_chrg=<optimized out>)
    at /home/julien/compile-libreoffice/libreoffice/basic/source/sbx/sbxvalue.cxx:100
#11 0x00002aaaae50df92 in SbxVariable::SbxVariable (this=0x76ce3e0, __vtt_parm=0x2aaaae81a208 <VTT for SbxMethod+8>, r=..., __in_chrg=<optimized out>)
    at /home/julien/compile-libreoffice/libreoffice/basic/source/sbx/sbxvar.cxx:77
#12 0x00002aaaae500f12 in SbxMethod::SbxMethod (this=0x76ce3e0, r=..., __in_chrg=<optimized out>, __vtt_parm=<optimized out>)
    at /home/julien/compile-libreoffice/libreoffice/basic/source/sbx/sbxobj.cxx:900
Comment 8 Julien Nabet 2015-02-19 20:14:26 UTC
(gdb) p seqArguments
$1 = uno::Sequence of length 1 = {{Name = "Overwrite", Handle = 0, Value = uno::Any 0 '\000', State = com::sun::star::beans::PropertyState_DIRECT_VALUE}}
Comment 9 Julien Nabet 2015-02-19 21:27:33 UTC
Trying to unwind with gdb, it seems to come from SfxMedium::Transfer_Impl(), see
http://opengrok.libreoffice.org/xref/core/sfx2/source/doc/docfile.cxx#1732

I'll try to find some time to keep on
Comment 10 Julien Nabet 2015-02-20 22:26:19 UTC
It seems it's perhaps in fileaccess::TaskManager::endTask but I don't understand all this part.
I can't help here :-(
Comment 11 Julien Nabet 2015-02-21 00:53:00 UTC
code pointers
When it's ok:
Breakpoint 5, SfxBaseModel::storeAsURL (this=0x76e8c50, rURL="file:///home/julien/compile-libreoffice/bugs/60237_macrostore/Test2.odt", 
    rArgs=uno::Sequence of length 1 = {...}) at /home/julien/compile-libreoffice/libreoffice/sfx2/source/doc/sfxbasemodel.cxx:1662
1662	        impl_store( rURL, rArgs, false );
(gdb) p rArgs
$11 = uno::Sequence of length 1 = {{Name = "Overwrite", Handle = 0, Value = uno::Any 1 '\001', State = com::sun::star::beans::PropertyState_DIRECT_VALUE}}

or even:
Breakpoint 5, SfxBaseModel::storeAsURL (this=0x76e8c50, rURL="file:///home/julien/compile-libreoffice/bugs/60237_macrostore/Test3.odt", rArgs=empty uno::Sequence)
    at /home/julien/compile-libreoffice/libreoffice/sfx2/source/doc/sfxbasemodel.cxx:1662
1662	        impl_store( rURL, rArgs, false );


When it's ko:
Breakpoint 5, SfxBaseModel::storeAsURL (this=0x76e8c50, rURL="file:///home/julien/compile-libreoffice/bugs/60237_macrostore/Test4.odt", 
    rArgs=uno::Sequence of length 1 = {...}) at /home/julien/compile-libreoffice/libreoffice/sfx2/source/doc/sfxbasemodel.cxx:1662
1662	        impl_store( rURL, rArgs, false );
(gdb) p rArgs
$13 = uno::Sequence of length 1 = {{Name = "Overwrite", Handle = 0, Value = uno::Any 0 '\000', State = com::sun::star::beans::PropertyState_DIRECT_VALUE}}
Comment 12 Aron Budea 2015-02-23 16:29:42 UTC
I'm by no means an expert, but after debugging this issue, the following piece of code looks really suspicious:

In method SfxMedium::TransactedTransferForFS_Impl, here
http://opengrok.libreoffice.org/xref/core/sfx2/source/doc/docfile.cxx#1586 :

SFX_ITEMSET_ARG( GetItemSet(), pOverWrite, SfxBoolItem, SID_OVERWRITE, false );
SFX_ITEMSET_ARG( GetItemSet(), pRename, SfxBoolItem, SID_RENAME, false );
bool bRename = pRename && pRename->GetValue();
bool bOverWrite = pOverWrite ? pOverWrite->GetValue() : !bRename;

The possibilities:
-SID_OVERWRITE is set: that value is used
-SID_OVERWRITE is not set:
        -SID_RENAME is set: the opposite of SID_RENAME is used
        -SID_RENAME is not set (bRename is false): bOverWrite becomes true <= this is what happens in example 3

I hope that helps. :)
Comment 13 Peter Toye 2015-02-23 19:05:16 UTC
I am in no way competent to suggest a programming cure for this, but I think we have to be vary careful in doing so. There may be users who have written macros which make use of the current default behaviour, and which will stop working as designed if the LO code is changed. 

As I have written elsewhere, default behaviour which is destructive should be deprecated, but I have a nasty feeling that we are rather stuck with it. Apart from modifying the documentation to highlight this behaviour, is there anything else we can do? I have done this in the tutorial https://wiki.openoffice.org/wiki/Saving_a_document but as I am not an AOO or LO developer I don't think I can (or should) modify the basic API documentation without the agreement of the devlopment teams. 

There is also the issue of a split in functionality between LO and AOO (or do the dev teams talk to each other?).
Comment 14 LeMoyne Castle 2015-02-26 04:58:56 UTC
(In reply to Aron Budea from comment #12)
> I'm by no means an expert, but after debugging this issue, the following
> piece of code looks really suspicious:
> 
...
> bool bRename = pRename && pRename->GetValue();
> bool bOverWrite = pOverWrite ? pOverWrite->GetValue() : !bRename;

Aron, the SID_xxx values are compile time constants that are available.  If the SID_xxx, RID_xxx tables get messed up LibreOffice will almost certainly crash at start. 

I think this is working as desired: the Overwrite value will be used if given, but if it's not LibO will only overwrite a file of the same name.  

I can see how overwriting a file of the same name can cause data loss and if one is in that situation then adding a time stamp to the file name will avoid the loss.  

The default is not exactly false, it is false (=no overwrite) except when the save would overwrite a file of the same name.  This is a core save routine in Writer and no one would be happy with a word processor that insisted on asking about overwrites everytime a file was saved again.  So, in my opinion, the issue is with the documentation.
Comment 15 Aron Budea 2015-03-01 18:33:04 UTC
(In reply to lo from comment #13)
> As I have written elsewhere, default behaviour which is destructive should
> be deprecated, but I have a nasty feeling that we are rather stuck with it.
> Apart from modifying the documentation to highlight this behaviour, is there
> anything else we can do? I have done this in the tutorial
> https://wiki.openoffice.org/wiki/Saving_a_document but as I am not an AOO or
> LO developer I don't think I can (or should) modify the basic API
> documentation without the agreement of the devlopment teams. 

I don't agree that it should ultimately be considered as an issue in the API documentation, because the API usage is based on that documentation, and users expect the functions to work as described there. If someone wanted to use this function, they must've expected that Overwrite would be false by default, based on the documentation.
If they noticed the contradiction between the documentation and actual behavior, and relied on the way it worked, that's a deliberate action (and a serious mistake).
On the other hand there might also be code out there that relies on the documented default value of the Overwrite flag, and the bug simply went unnoticed.
Comment 16 Aron Budea 2015-03-01 18:42:17 UTC
(In reply to LeMoyne Castle from comment #14)
> Aron, the SID_xxx values are compile time constants that are available.  If
> the SID_xxx, RID_xxx tables get messed up LibreOffice will almost certainly
> crash at start. 

My bad if I wasn't clear, I was not implying to meddle with the SID_xxx constants, I was only referring to the property values stored with those IDs.

> I think this is working as desired: the Overwrite value will be used if
> given, but if it's not LibO will only overwrite a file of the same name.  

No it isn't, it overwrites any file, not only if it has the same URL.
Plus if the desired behavior is to save over the same file, the store() function is designed for that purpose.

I'm not sure which core save routine in Writer you're reffering to, but if you try to overwrite the same file with Save As... (which storeAsURL() resembles the closest), it's not going to without asking first.
Comment 17 Peter Toye 2015-11-26 12:02:28 UTC
(In reply to Aron Budea from comment #15)

So where does this leave us? At the moment the code and documentation differ - at least one should change.

> I don't agree that it should ultimately be considered as an issue in the API
> documentation, because the API usage is based on that documentation, and
> users expect the functions to work as described there. If someone wanted to
> use this function, they must've expected that Overwrite would be false by
> default, based on the documentation.
I agree with this, provided that changing the code to reflect the documentation isn't going to stop existing programs from working.

> If they noticed the contradiction between the documentation and actual
> behavior, and relied on the way it worked, that's a deliberate action (and a
> serious mistake).
One has to rely on the way it works, so it's not a mistake! I could have simply decided to remember each time I saved that it should be to a different file (unless I really wanted to overwrite). Of course I didn't and set the Overwrite flag to prevent overwriting.

> On the other hand there might also be code out there that relies on the
> documented default value of the Overwrite flag, and the bug simply went
> unnoticed.
...and overwrote a file (as happened to me)! 

My personal view is that the code should be changed to reflect the documentation, which in my opinion is the desired behaviour. But I have no idea if the LO development team feel that current behaviour should take priority over documentation. Maybe someone from that team could comment. I'm only a user so I don't know the correct form to get action, but it's been 8 months since any comments on this issue.
Comment 18 Peter Toye 2016-11-18 14:36:57 UTC
It's now nearly a year since any comment was made on this subject. Is no-one looking at this?
Comment 19 QA Administrators 2017-11-19 19:15:16 UTC Comment hidden (obsolete)
Comment 20 Peter Toye 2017-11-20 18:40:39 UTC
Bug is still there in version 5.7.3 (latest stable).
Comment 21 Peter Toye 2017-11-20 19:12:29 UTC
(In reply to Peter Toye from comment #20)
> Bug is still there in version 5.7.3 (latest stable).

Version: 5.3.7.2 (x64)
Build ID: 6b8ed514a9f8b44d37a1b96673cbbdd077e24059
CPU Threads: 4; OS Version: Windows 6.1; UI Render: default; Layout Engine: new; 
Locale: en-GB (en_GB); Calc: group


Sorry forgot to add this.
Comment 22 Peter Toye 2019-05-30 11:03:13 UTC
(In reply to Aron Budea from comment #16)
> (In reply to LeMoyne Castle from comment #14)
> > Aron, the SID_xxx values are compile time constants that are available.  If
> > the SID_xxx, RID_xxx tables get messed up LibreOffice will almost certainly
> > crash at start. 
> 
> My bad if I wasn't clear, I was not implying to meddle with the SID_xxx
> constants, I was only referring to the property values stored with those IDs.
> 
> > I think this is working as desired: the Overwrite value will be used if
> > given, but if it's not LibO will only overwrite a file of the same name.  
> 
> No it isn't, it overwrites any file, not only if it has the same URL.
> Plus if the desired behavior is to save over the same file, the store()
> function is designed for that purpose.
> 
> I'm not sure which core save routine in Writer you're reffering to, but if
> you try to overwrite the same file with Save As... (which storeAsURL()
> resembles the closest), it's not going to without asking first.

Is anything happening on this? I reported it 6 years ago and the last comment was from Aron in 2015! There's not much point in a bug reporting system if the dev team dno't look at it.