Issue 128356

1.  Minor correction - AOO does not give a SAXParse error.  AOO gives a "Read Error:  Format error discovered ... at n,nnnn (row,col)"

2.  See Issue 127745 - Read Error: Format error discovered ... at n,nnnn (row,col) which appears to be related.

Confirmed using AOO 4.1.7 on Windows 10

As this bug causes complete data loss / data corruption I think it should be more important than P5 = LOWEST.

I have just repaired a user's file where styles.xml was corrupted with two office:name annotations.  The corruption was similarly in the first style definition in the file.

The file was full of comments and I could replicate the problem by deleting a range of text which included two comments each attached to a range of text while Track changed was ON.

I believe the corruption is in styles.xml because the reviewer had Record Changes set to ON and had made a change to a style.  Such recorded changes are stored in styles.xml.

This suggests that "deleting some text which includes two comments attached to a range of text while Record changes is ON" is an effect of the bug and not the cause.

I think investigation needs to focus on how and why the duplicated annotation gets written.

Note that Issue 127745 - Read Error: Format error discovered ... at n,nnnn (row,col) concluded it was AOO writing the file which caused the corruption.

I have attached Broken_file.odt which has been anonymised but which behaves as described.

It would help the forum greatly if a resolution of this bug could be applied to v4.2.  See OpenOffice Writer issue - Read-Error - Format error at https://forum.openoffice.org/en/forum/viewtopic.php?f=15&t=101969&p=492539#p492539.  

This bug has caused the French forum to post a repair utility to repair such corrupted files.  It has been downloaded 1,497 times in 8 months.  See 
Utilitaire de réparation de fichier ODF at https://forum.openoffice.org/fr/forum/viewtopic.php?f=26&t=60992

Created attachment 86947 [details]
Broken_File.odt gives Read Error on opening

Broken_File.odt gives Read Error - Format error in styles.xml at 2,15035 due to repeated office:name annotations in the first style definition in styles.xml.

Note the file still has Record Changes set to ON and extensive changes and comments have been applied to the file.

As the file is a user's file the text has been anonymised.

Created attachment 86995 [details]
fred.odt.  File used to cause error

fred.odt is a very simple file where the error can be replicated reliably

Procedure to cause error

1.  Download fred.odt
2.  Ensure Edit > Changes > Record ..., is ON
3.  Highlight the first two sentences > press delete.
4.  Save

Now open fred.odt.  You get the "Format error" error message and the first paragraph style in content.xml has been corrupted.

This bug needs the help of a developer to find the "root cause" and fix.

Reproduced on current trunk under Linux.

A debug build gives a failed assertion (copied by hand):

> SAX parse exception caught while importing:
> [ line 2]: duplicate attributeerror 
> From File
> main/sw/source/filter/xml/sswxl.cxx at Line 220

If I run xmllint on the ``broken'' content.xml I get the following error:

> content.xml:15: parser error : Attribute office:name redefined
> 811" style:name="P1" style:family="paragraph" style:parent-style-name="Standard"

And then the error message as OP reported.

Before being able to reproduce the problem, I met some error messages about "sentence indices" being out of range. I will try to understand it they are related.

(In reply to myself from comment #9)

> Before being able to reproduce the problem, I met some error messages about
> "sentence indices" being out of range. I will try to understand it they are
> related.

They are not. They are probably a leftover of a partially commented snippet.

File main/editeng/source/editeng/impedit4.cxx line 1468:
an EditSelection object is created from a selection that seems to be empty (the document is being loaded; the selection begins and ends at the same node). The object is never used but in commented code below.
Commenting the creation of the object takes the warning away and should have no unintended side effects.

The next warning is another failed assertion (again, copied by hand):

> Error: JoinMetadatables called from object in clipboard?
> From File main/sfx2/source/doc/Metadatable.cxx at Line 1542

The file begins with a long comment (yay! :-) about XML id's.
I hope this is the right place to look into.

(In reply to Arrigo Marchiori from comment #11)
> The next warning is another failed assertion (again, copied by hand):
> 
> > Error: JoinMetadatables called from object in clipboard?
> > From File main/sfx2/source/doc/Metadatable.cxx at Line 1542
> 
> The file begins with a long comment (yay! :-) about XML id's.
> I hope this is the right place to look into.

This warning appears because the "Redlines" i.e. the recorded changes, when selected, are first copied into a "clipboard document" and then removed. Removing them consists of deleting and joining text nodes, and this calls the method SwContent::JoinMetadatables that issues the above warning.

This happens in the moment we select them, before we press the DEL key as the OP suggests.

I am still trying to understand if this is related with the bug, though.
On one hand, it should, because the bug is about some kind of ID's.
On the other hand, it should not, because the warning is about the "clipboard document", and not the document itself.

Thank you for looking at this bug.  

I also reported an extremely similar bug which may be related so may assist your search.

See Issue 127745 - Read Error: Format error discovered ... at n,nnnn (row,col)

https://bz.apache.org/ooo/show_bug.cgi?id=127745

The P1 Style definition is similarly corrupted and 

office:name="__Annotation__153_24419901911111111" office:name="__Annotation__158_2441990191111" office:name="__Annotation__248_244199019111111" office:name="__Annotation__401_244199019111" office:name="__Annotation__414_24419901911" 

has been inserted into the P1 Style definition.

(In reply to John from comment #13)
> Thank you for looking at this bug.  
> 
> I also reported an extremely similar bug which may be related so may assist
> your search.
> 
> See Issue 127745 - Read Error: Format error discovered ... at n,nnnn
> (row,col)
> 
> https://bz.apache.org/ooo/show_bug.cgi?id=127745

This is very interesting! Thank you!

I can reproduce _this_ bug by following the suggestions from bug 127745, that is: change anything in fred.odt (like adding a space) and then save.

I _wish_ that bug 127745 was a duplicated of this one (or vice-versa) but I cannot confirm yet ;-)

What is very useful, is that I do not have to focus on the "JoinMetadatables" warning I reported in comment 11, because it's not essential any more to reproduce this bug.

It is possible to find where the problem arises, by adding an assertion (actually, a safety check) to the methods of SvXMLAttributeList that add new attributes: existing attributes should not be added, but rather changed.

I will open a thread on the dev@ mailing list for asking how to cope with this kind of problems, in order to (hopefully) mitigate the effect of this bug.

After settling this out, we will need to see _why_ the attribute is set twice.

Stack trace from the assertion:

#13 0x00007fffedafbd5c in SvXMLAttributeList::AddAttribute(rtl::OUString const&, rtl::OUString const&)     
    (this=this@entry=0x7fff96cf21f0, sName=..., sValue=...)
    at main/xmloff/source/core/attrlist.cxx:165

#14 0x00007fffedb07d49 in SvXMLExport::AddAttribute(unsigned short, xmloff::token::XMLTokenEnum, rtl::OUString const&)
    (this=<optimized out>, nPrefixKey=nPrefixKey@entry=0, eName=eName@entry=xmloff::token::XML_NAME, rValue=...)
    at main/xmloff/source/core/xmlexp.cxx:1083

#15 0x00007fffedc72086 in XMLTextParagraphExport::exportTextRangeEnumeration(com::sun::star::uno::Reference<com::sun::star::container::XEnumeration> const&, unsigned char, unsigned char, unsigned char)  
    (this=this@entry=0x7fff96fa7140, rTextEnum=..., bAutoStyles=bAutoStyles@entry=1 '\001', bIsProgress=bIsProgress@entry=0 '\000', bPrvChrIsSpc=bPrvChrIsSpc@entry=1 '\001')
    at main/xmloff/source/text/txtparae.cxx:2260

#16 0x00007fffedc7395d in XMLTextParagraphExport::exportParagraph(com::sun::star::uno::Reference<com::sun::star::text::XTextContent> const&, unsigned char, unsigned char, unsigned char, MultiPropertySetHelper&)
   (this=this@entry=0x7fff96fa7140, rTextContent=..., bAutoStyles=bAutoStyles@entry=1 '\001', bIsProgress=bIsProgress@entry=0 '\000', bExportParagraph=bExportParagraph@entry=1 '\001', rPropSetHelper=...)
    at main/xmloff/source/text/txtparae.cxx:2198

#17 0x00007fffedc718d2 in XMLTextParagraphExport::exportTextContentEnumeration(com::sun::star::uno::Reference<com::sun::star::container::XEnumeration> const&, unsigned char, com::sun::star::uno::Reference<com::sun::star::text::XTextSection>
 const&, unsigned char, unsigned char, com::sun::star::uno::Reference<com::sun::star::beans::XPropertySet> const*, unsig
ned char)                                                                                                               
    (this=this@entry=0x7fff96fa7140, rContEnum=..., bAutoStyles=bAutoStyles@entry=1 '\001', rBaseSection=..., bIsProgress=bIsProgress@entry=0 '\000', bExportParagraph=bExportParagraph@entry=1 '\001', pRangePropSet=<optimized out>, bExportLevels=<optimized out>) at main/xmloff/source/text/txtparae.cxx:1861

#18 0x00007fffedc73f42 in XMLTextParagraphExport::exportText(com::sun::star::uno::Reference<com::sun::star::text::XText> const&, unsigned char, unsigned char, unsigned char)                                                                   
    (this=0x7fff96fa7140, rText=..., bAutoStyles=bAutoStyles@entry=1 '\001', bIsProgress=bIsProgress@entry=0 '\000', bExportParagraph=bExportParagraph@entry=1 '\001')                                                                          
    at main/xmloff/source/text/txtparae.cxx:1731

#19 0x00007fffedb48527 in XMLTextParagraphExport::collectTextAutoStyles(com::sun::star::uno::Reference<com::sun::star::text::XText> const&, unsigned char, unsigned char)
    (this=<optimized out>, rText=..., bIsProgress=bIsProgress@entry=0 '\000', bExportParagraph=bExportParagraph@entry=1 '\001') 
    at main/solver/450/unxlngx6/inc/xmloff/txtparae.hxx:598

#20 0x00007fffedc26ad7 in XMLRedlineExport::ExportChangeAutoStyle(com::sun::star::uno::Reference<com::sun::star::beans::XPropertySet> const&) (this=this@entry=0x7fff95e8d300, rPropSet=...)                                                    
    at main/xmloff/source/text/XMLRedlineExport.cxx:296

#21 0x00007fffedc26cf4 in XMLRedlineExport::ExportChangesListAutoStyles() (this=0x7fff95e8d300)                         
    at main/xmloff/source/text/XMLRedlineExport.cxx:329

#22 0x00007fffedc26d82 in XMLRedlineExport::ExportChangesList(unsigned char)
    (this=<optimized out>, bAutoStyles=bAutoStyles@entry=1 '\001')
    at main/xmloff/source/text/XMLRedlineExport.cxx:139

#23 0x00007fffedc6b87f in XMLTextParagraphExport::exportTrackedChanges(unsigned char)
    (this=<optimized out>, bAutoStyles=bAutoStyles@entry=1 '\001')
    at main/xmloff/source/text/txtparae.cxx:3510

#24 0x00007fff9b8a6ad5 in SwXMLExport::_ExportAutoStyles() (this=0x7fff96c76420)                                        
    at main/sw/source/filter/xml/xmlfmte.cxx:218

#25 0x00007fffedb0d48e in SvXMLExport::ImplExportAutoStyles(unsigned char) (this=this@entry=0x7fff96c76420)
    at main/xmloff/source/core/xmlexp.cxx:1252

#26 0x00007fffedb0e991 in SvXMLExport::exportDoc(xmloff::token::XMLTokenEnum)      
    (this=this@entry=0x7fff96c76420, eClass=eClass@entry=xmloff::token::XML_TEXT)                                     
    at main/xmloff/source/core/xmlexp.cxx:1541

#27 0x00007fff9b8a1abe in SwXMLExport::exportDoc(xmloff::token::XMLTokenEnum)
    (this=0x7fff96c76420, eClass=xmloff::token::XML_TEXT)
    at main/sw/source/filter/xml/xmlexp.cxx:391

#28 0x00007fffedb08abe in SvXMLExport::filter(com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) (this=0x7fff96c76420, aDescriptor=<optimized out>)
    at main/xmloff/source/core/xmlexp.cxx:938

#29 0x00007fff9b89c2f3 in SwXMLWriter::WriteThroughComponent(com::sun::star::uno::Reference<com::sun::star::io::XOutputStream> const&, com::sun::star::uno::Reference<com::sun::star::lang::XComponent> const&, com::sun::star::uno::Reference<com::sun::star::lang::XMultiServiceFactory> const&, char const*, com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&) 
    (this=this@entry=0x7fff959a8b90, xOutputStream=..., xComponent=..., rFactory=..., pServiceName=pServiceName@entry=0x7fff9ba7c169 "com.sun.star.comp.Writer.XMLOasisContentExporter", rArguments=..., rMediaDesc=...)                        
    at main/sw/source/filter/xml/wrtxml.cxx:650

Fun fact: in comment #15 I thought I had a stack trace just before the moment the wrong XML tag was emitted... but it was not!
In fact, if I understood correctly, that is the "autostyle collection" phase, that consists of parsing the text as for exporting it... but not emitting any tags (bAutoStyles is set to true, that translates to "do nothing" for XML element emitters).

Because bAutoStyles de-activated the generation of the tag, the attribute set for the element to be emitted remains full (instead of being emptied, as it would happen if the tag was emitted), and causing the name clash later.

The XML element is emitted later, according to the following stack trace:

#1  0x00007fffedb08d5d in SvXMLExport::StartElement(rtl::OUString const&, unsigned char)
    (this=0x7fffcc0fd420, rName=..., bIgnWSOutside=bIgnWSOutside@entry=1 '\001')
    at main/xmloff/source/core/xmlexp.cxx:2351

#2  0x00007fffedb08f1e in SvXMLElementExport::StartElement(unsigned short, rtl::OUString const&, unsigned char)
    (this=this@entry=0x7fffffff81e0, nPrefixKey=nPrefixKey@entry=1, rLName=..., bIgnoreWhitespaceOutside=bIgnoreWhitespaceOutside@entry=1 '\001') 
    at main/xmloff/source/core/xmlexp.cxx:2654

#3  0x00007fffedb0902e in SvXMLElementExport::SvXMLElementExport(SvXMLExport&, unsigned short, rtl::OUString const&, uns
igned char, unsigned char)
    (this=0x7fffffff81e0, rExp=<optimized out>, nPrefixKey=<optimized out>, rLName=..., bIWSOutside=<optimized out>, bIWSInside=<optimized out>)
    at main/xmloff/source/core/xmlexp.cxx:2683

#4  0x00007fffedbe252f in SvXMLAutoStylePoolP_Impl::exportXML(int, com::sun::star::uno::Reference<com::sun::star::xml::sax::XDocumentHandler> const&, SvXMLUnitConverter const&, SvXMLNamespaceMap const&, SvXMLAutoStylePoolP const*) const
    (this=<optimized out>, nFamily=nFamily@entry=32767, pAntiImpl=pAntiImpl@entry=0x7fff97b6f510)
    at main/xmloff/source/style/impastp4.cxx:477

#5  0x00007fffedbed974 in SvXMLAutoStylePoolP::exportXML(int, com::sun::star::uno::Reference<com::sun::star::xml::sax::XDocumentHandler> const&, SvXMLUnitConverter const&, SvXMLNamespaceMap const&) const
    (this=this@entry=0x7fff97b6f510, nFamily=32767, nFamily@entry=100)
    at main/xmloff/source/style/xmlaustp.cxx:434

#6  0x00007fffedc6b912 in XMLTextParagraphExport::exportTextAutoStyles() (this=0x7fff9c3bd050)
    at main/xmloff/source/text/txtparae.cxx:3537

#7  0x00007fffa2d13c14 in SwXMLExport::_ExportAutoStyles() (this=0x7fffcc0fd420)
    at main/sw/source/filter/xml/xmlfmte.cxx:237

#8  0x00007fffedb0d48e in SvXMLExport::ImplExportAutoStyles(unsigned char) (this=this@entry=0x7fffcc0fd420)
    at main/xmloff/source/core/xmlexp.cxx:1252

The common frames are no. 8 here and no. 25 in the previous comment.
In other words, we are still in SwXMLExport::_ExportAutoStyles() and we advanced from line 218 from line 237.

At this point, it seems that the previous phase ("autostyle collection") has prepared an XML tag without emitting it, but setting its attributes. This phase consists of analyzing those attributes and emitting another tag (<style:style>) using... other? or maybe the same? attributes.

This system is flawed because the previous phase tried to duplicate an attribute, and this phase blindly outputs the duplication.

In order to fix this, we need to understand very well what each one of the plethora of involved classes does. I am afraid it's not going to be a short trip.

<style:style> elements _are not supposed to have any_ office:name attribute.

The solution to the generation of a corrupted document is: avoid outputting the office:name attribute at all.

(In reply to Arrigo Marchiori from comment #17)
> <style:style> elements _are not supposed to have any_ office:name attribute.
> 

Whilst not outputting the office:name attribute in the first style definition will probably fix Issue 128356 does this numbering peculiarity from Issue 127745 suggest something else is happening which needs to be fixed?

Note it always seems only to be FIRST style definition which is corrupted, be it a paragraph or table style definition, or content.xml or styles.xml.

(See comment #13)

> See Issue 127745 - Read Error: Format error discovered ... at n,nnnn
> (row,col)
> 
> https://bz.apache.org/ooo/show_bug.cgi?id=127745
> 
> The P1 Style definition is similarly corrupted and 
> 
> office:name="__Annotation__153_24419901911111111"
> office:name="__Annotation__158_2441990191111"
> office:name="__Annotation__248_244199019111111"
> office:name="__Annotation__401_244199019111"
> office:name="__Annotation__414_24419901911" 
> 
> has been inserted into the P1 Style definition.

Note the strange numbers where a " 1 " seems to be appended again and again to the Annotation number.

Pull request (draft by now): https://github.com/apache/openoffice/pull/122

First of all, John, thank you again for helping to "connect the dots"!

(In reply to John from comment #18)
> (In reply to Arrigo Marchiori from comment #17)
> > <style:style> elements _are not supposed to have any_ office:name attribute.
> > 
> 
> Whilst not outputting the office:name attribute in the first style
> definition will probably fix Issue 128356 does this numbering peculiarity
> from Issue 127745 suggest something else is happening which needs to be
> fixed?

I am not sure.

After applying the tentative fix, the corruption from issue 127745 also disappears (when saving, of course).

> Note it always seems only to be FIRST style definition which is corrupted,
> be it a paragraph or table style definition, or content.xml or styles.xml.
> 
> (See comment #13)
> 
> > See Issue 127745 - Read Error: Format error discovered ... at n,nnnn
> > (row,col)
> > 
> > https://bz.apache.org/ooo/show_bug.cgi?id=127745
> > 
> > The P1 Style definition is similarly corrupted and 
> > 
> > office:name="__Annotation__153_24419901911111111"
> > office:name="__Annotation__158_2441990191111"
> > office:name="__Annotation__248_244199019111111"
> > office:name="__Annotation__401_244199019111"
> > office:name="__Annotation__414_24419901911" 
> > 
> > has been inserted into the P1 Style definition.
> 
> Note the strange numbers where a " 1 " seems to be appended again and again
> to the Annotation number.

We could address the fact that repeated "office:name" attributes were added to a <style:style> element.

<office:name> contents being changed at every save is a different problem, for what I have seen so far.
I will reply to you on that report, as I believe it is slightly off-topic here.

*** Issue 127745 has been marked as a duplicate of this issue. ***

*** Issue 126330 has been marked as a duplicate of this issue. ***

*** Issue 126479 has been marked as a duplicate of this issue. ***

Arrigo

I have just had a thought which I expect you had too, but just in case ...

We see differing numbers of office:name annotations get added to and corrupt the P1 Style definition:  sometimes two, sometimes three, etc.  

I did a quick check with Sammy Russel 1draft - CORRECTED.odt from bug 127745.  

If I simultaneously delete all the comments attached to text ranges then 5 office:name annotations get added to the P1 Style definition.  

If I simultaneously delete only 3 comments attached to text ranges then only 3 office:name annotations get added to the P1 Style definition. 

So, it appears that "all the office:name annotations which are deleted" get added into the P1 Style definition supporting your Comment 17:

> The solution to the generation of a corrupted document is: avoid outputting 
> the office:name attribute at all.

Hello John,

(In reply to John from comment #24)
[...]
> I did a quick check with Sammy Russel 1draft - CORRECTED.odt from bug
> 127745.  
> 
> If I simultaneously delete all the comments attached to text ranges then 5
> office:name annotations get added to the P1 Style definition.  
> 
> If I simultaneously delete only 3 comments attached to text ranges then only
> 3 office:name annotations get added to the P1 Style definition. 
> 
> So, it appears that "all the office:name annotations which are deleted" get
> added into the P1 Style definition supporting your Comment 17:
> 
> > The solution to the generation of a corrupted document is: avoid outputting 
> > the office:name attribute at all.

Thank you for further looking into this problem.
I can confirm that, on my test build with the patch applied, the problem remains solved if I follow your example, that is deleting one, two or more comments.

*** Issue 128564 has been marked as a duplicate of this issue. ***