Re: Bug in NodeRevisionContent and patch

[Christopher Lenz <cm...@gmx.de>]

Hi Martin,

Martin Holz wrote:
> Hi,
> 
> there is a serious bug in NodeRevisionContent.
> 
> The stream returned by NodesRevisionContent.streamContent() should be used 
> only once, if the was set in NodeRevisionContent.setContent(InputStream 
> inputStream) 
> 
>  However it will be used more often:
> 
> 1) in most implementations of ContentInterceptor.preStoreContent 
> 2) when storeing the content
> 3) in most implementations of ContentInterceptor.postStoreContent 
> 
> Only the first read will be successfull. 

I agree this is a serious problem, stemming from the quick&dirty design 
of the ContentInterceptor interface...

As the ContentInterceptor API has already changed since the last 
release, we should fix the API now (again :P). How about:

1. Not allowing access to the actual content stream in 
ContentInterceptor.preStoreContent, so that ContentInterceptors can only 
investigate the actual content after it has been stored, and

2. Retrieved the content and pass that into 
ContentInterceptor.postStoreContent so that the interceptor has a fresh 
stream to read from at that point, or alternatively let the interceptor 
retrieve the content itself if it needs to do so.

So the original stream from the HTTP connection would only get accessed 
when storing the content. This is really not thought through, just a 
quick shot at the problem. What do you think?

-- 
Christopher Lenz
/=/ cmlenz at gmx.de


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: Bug in NodeRevisionContent and patch

[Martin Holz <ho...@fiz-chemie.de>]

Christopher Lenz wrote:
> Hi Martin,
>
> Martin Holz wrote:
> > Hi,
> >
> > there is a serious bug in NodeRevisionContent.
> >
> > The stream returned by NodesRevisionContent.streamContent() should be
> > used only once, if the was set in
> > NodeRevisionContent.setContent(InputStream inputStream)
> >
> >  However it will be used more often:
> >
> > 1) in most implementations of ContentInterceptor.preStoreContent
> > 2) when storeing the content
> > 3) in most implementations of ContentInterceptor.postStoreContent
> >
> > Only the first read will be successfull.
>
> I agree this is a serious problem, stemming from the quick&dirty design
> of the ContentInterceptor interface...
>
> As the ContentInterceptor API has already changed since the last
> release, we should fix the API now (again :P). How about:

The interface of ContentInterceptor  is okay, the implementation
of NodeRevisionContent  is the problem :-(

> 1. Not allowing access to the actual content stream in
> ContentInterceptor.preStoreContent, so that ContentInterceptors can only
> investigate the actual content after it has been stored, and
>
> 2. Retrieved the content and pass that into
> ContentInterceptor.postStoreContent so that the interceptor has a fresh
> stream to read from at that point, or alternatively let the interceptor
> retrieve the content itself if it needs to do so.
>
> So the original stream from the HTTP connection would only get accessed
> when storing the content. This is really not thought through, just a
> quick shot at the problem. What do you think?


I need ContentInterceptor.preStoreContent to validate the input and reject  
or fix it.,if necessary . How could the store recover  the old content if 
preStoreContent fails?

I am afraid, the NodeRevisionContent must keep a local copy of the content.

Martin



  

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>