incubator-odf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeremias Maerki <...@jeremias-maerki.ch>
Subject Re: OdfPackage not throwing an exception on I/O errors
Date Fri, 23 Mar 2012 11:15:40 GMT
I've just noticed that the same problem exists on the reading side.
While you can set your own ErrorHandler, any SAXException you throw
inside the ErrorHandler (because you might want to stop processing a
potentially damaged ODF file), it is just caught later and just logged 
(again). The user could have done that himself in the ErrorHandler.
Basically, the user would have to write an ErrorHandler that throws an
unchecked exception to really get the process to stop. I'm looking into
that, too.

On 23.03.2012 09:58:41 Jeremias Maerki wrote:
> Please note that I fully agree that using an ErrorHandler on the reading
> side for all sorts of things (XML parsing errors, validation errors,
> even IOExceptions) is a good way to handle things and to promote
> robustness and error tolerance.
> 
> But my post was motivated from the writing side where errors are
> swallowed. If there is an I/O error while writing the target file, I
> personally can see no real reason why the save operation should continue
> since you are almost guaranteed that you'll get an incomplete or inconsistent
> output file. If I understood Svante correctly, he suggests to use the
> ErrorHandler approach for the output side, too. It feels weird to me to
> wrap an exception during a write operation into a SAXParseException just
> so it fits through the ErrorHandler interface. I'd suggest to find a
> different approach (and disabled by default) if the save code shall be
> error tolerant in some way. Normally, I'd think that if the in-memory
> respresentation of the ODF document is consistent there should be no
> errors while writing the file as long as the OutputStream works.
> 
> Anyway, I'll concentrate on a proposal to throw IOExceptions during
> save() first. I'm sure you can always refine that later on if necessary.
> Patch coming soon...
> 
> On 23.03.2012 00:42:40 Svante Schubert wrote:
> > On 22.03.2012 20:45, Rob Weir wrote:
> > > On Thu, Mar 22, 2012 at 1:34 PM, Jeremias Maerki <dev@jeremias-maerki.ch>
wrote:
> > >> Hey guys,
> > >> is there a special reason why OdfPackage.save(OutputStream, String)
> > >> catches all IOExceptions, logs them and just continues? While I can see
> > >> the problems in the logs, my code thinks everything is fine and
> > >> continues normally.
> > >>
> > > I can't think of a good reason.
> > I can not think of a good reason, either. Already discussed it with
> > Jeremias this evening by phone and agreed to throwing Exception in
> > general, unless the user could gather more user information (robustness)
> > or more information about validness, in this case the ErrorHandler
> > <http://docs.oracle.com/javase/1.5.0/docs/api/org/xml/sax/ErrorHandler.html>
> > mechanism should be uses.
> > 
> > In addition we found out that for the line
> > data = mMediaType.getBytes("UTF-8");
> > should better raise an OdfValidationException for the validator, in case
> > this "mimetype" named file, being first in the ZIP is not being encoded
> > as ASCII, see
> > http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part3.html#MIME_type_stream
> > Well, we might here even exchange UTF-8 to ASCII, making it more
> > restrict - although first 128 char are similar, some might have used
> > other UTF-8 characters within, which would be invalid ODF.
> > 
> > For those new on the list, the pkg.OdfValidationException is inheriting
> > from the SAXParseException and enabling the similar mechanism meant for
> > SAX XML parsing for the ODF Package.
> > 
> > To summarize:
> > While parsing an XML file it is not desired that the parsing stops at
> > the first invalid XML by an exception. Instead the exception will be
> > given to an ErrorHandler, which differentiated between warning, error
> > and fatalerrors.
> > These three levels are mapped to a specification where something should
> > be used, shall be used and in the last case does not work at all.
> > 
> > I have extended the concept of the ErrorHandler not only for XML
> > validation errors, but as well for arbitrary ODF validation errors, like
> > within the package, when certain mandatory files are missing.
> > I take advantage of this concept at the ODF Validator, when a single
> > default ErrorHandler from the JDK can be reused to gather validation
> > results.
> > pkg.OdfPackageConstraint.java and dom.OdfSchemaConstraint.java lists the
> > problems that are currenlty being checked and the doc.DocumentTest.java
> > and pkg.PackageTest.java are testing the test mechanism with some nice
> > test tools as utils.ErrorHandlerStub.java
> > 
> > >> Furthermore, the public save() methods all throw Exception which is
> > >> rather bad style.
> > >>
> > > I assume you mean that they should limit themselves to IOException?
> > +1
> > >> Can we please change that? I'll offer to write a proposal in form of a
> > >> patch.
> > >>
> > > If you can contribute a patch, that would be great.  But please do a
> > > complete rebuild, to make sure the unit tests still work.
> > >
> > > So from the top directory:
> > >
> > > mvn clean install -Ppedantic
> > >
> > > Thanks!
> > >
> > > -Rob
> > >
> > >
> > >> Thanks,
> > >> Jeremias Maerki
> > >>
> > 
> 
> 
> 
> 
> Jeremias Maerki




Jeremias Maerki


Mime
View raw message