You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by iris ding <ir...@gmail.com> on 2014/07/01 03:07:19 UTC

two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Hi Guys,

I think There are two issues in
org.apache.cxf.jaxrs.provider.DataSourceProvider:
Issue 1: ClassCastException if you post a FileDataSource in your resource
class: 
Issue 2: Return the original InputStream directly in
InputStreamDataSource.getInputStream().


I have created a JIRA in 
https://issues.apache.org/jira/browse/CXF-5835 and put more details there.

Would you take a look at the problem and see whether we can make any
modification for it?








--
View this message in context: http://cxf.547215.n5.nabble.com/two-issues-in-org-apache-cxf-jaxrs-provider-DataSourceProvider-tp5745783.html
Sent from the cxf-dev mailing list archive at Nabble.com.

Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by Daniel Kulp <dk...@apache.org>.
I’m still failing to see any issue.  As far as the user is concerned it’s an InputStream.  Whether that is a ServeltInputStream, FileInputStream, ByteArrayInputStream, etc… should be irrelevant.  As long as it behaves according to the InputStream contract, it shouldn’t matter.

That said, that InputStream would ONLY be valid within the scope of the request.  If the user needs to save the information beyond the scope of the request, they should do whatever is necessary to “save” the data from that stream to someplace.

Again, this is not a problem.

Dan

On Jun 30, 2014, at 10:30 PM, iris ding <ir...@gmail.com> wrote:

> Johan,
> 
> Can I know the reason for your point?
> 
> Actually, the key reason for the issue 2 is not for available() usage. Let
> me give an example for this issue:
> 
> If you deploy your application to Tomcat, the incomming InputStream might be
> a tomcat class which implements ServletInputStream.
> 
> If you deploy your application to WebSphere, the incoming InputStream might
> be a websphere specific class which implements ServletInputStream.
> 
> In other case, the incomming InputStream might be some other class which
> implements InputStream directly. 
> 
> In such case if we do not do conversion and return the original incomming
> InputStream, Users will loose control of it. He might need to use specific
> API to operate the different stream.
> 
> Iris Ding 
> 
> 
> 
> --
> View this message in context: http://cxf.547215.n5.nabble.com/two-issues-in-org-apache-cxf-jaxrs-provider-DataSourceProvider-tp5745783p5745788.html
> Sent from the cxf-dev mailing list archive at Nabble.com.

-- 
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com


Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by iris ding <ir...@gmail.com>.
Johan,

Can I know the reason for your point?

Actually, the key reason for the issue 2 is not for available() usage. Let
me give an example for this issue:

If you deploy your application to Tomcat, the incomming InputStream might be
a tomcat class which implements ServletInputStream.

If you deploy your application to WebSphere, the incoming InputStream might
be a websphere specific class which implements ServletInputStream.

In other case, the incomming InputStream might be some other class which
implements InputStream directly. 

In such case if we do not do conversion and return the original incomming
InputStream, Users will loose control of it. He might need to use specific
API to operate the different stream.

Iris Ding 



--
View this message in context: http://cxf.547215.n5.nabble.com/two-issues-in-org-apache-cxf-jaxrs-provider-DataSourceProvider-tp5745783p5745788.html
Sent from the cxf-dev mailing list archive at Nabble.com.

Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by Johan Edstrom <se...@gmail.com>.
Issue 2 is not an issue.

On Jun 30, 2014, at 7:47 PM, iris ding <ir...@gmail.com> wrote:

> Thanks Daniel for your quick response. 
> 
> Although the javadoc for avaliable() is not promised to return the length.
> There also reasons we need to convert the incomming InputStream to a
> java.io.** .: The incomming inputStream might be a special type which only
> applicable to the jee container you used. In such case, if we return
> directly the original InputStream, users will loose control of the behaviour
> the returned InputStream. what do you think about this point?
> 
> Iris Ding
> 
> 
> 
> 
> --
> View this message in context: http://cxf.547215.n5.nabble.com/two-issues-in-org-apache-cxf-jaxrs-provider-DataSourceProvider-tp5745783p5745785.html
> Sent from the cxf-dev mailing list archive at Nabble.com.


Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by iris ding <ir...@gmail.com>.
Thanks Daniel for your quick response. 

Although the javadoc for avaliable() is not promised to return the length.
There also reasons we need to convert the incomming InputStream to a
java.io.** .: The incomming inputStream might be a special type which only
applicable to the jee container you used. In such case, if we return
directly the original InputStream, users will loose control of the behaviour
the returned InputStream. what do you think about this point?

Iris Ding




--
View this message in context: http://cxf.547215.n5.nabble.com/two-issues-in-org-apache-cxf-jaxrs-provider-DataSourceProvider-tp5745783p5745785.html
Sent from the cxf-dev mailing list archive at Nabble.com.

Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by Johan Edstrom <se...@gmail.com>.
The second?

Same reason I wouldn't change the JVM if I used Guava.
you are confusing impl with Interface.

/je

On Jun 30, 2014, at 9:02 PM, Daniel Kulp <dk...@apache.org> wrote:

> 
> I’m not sure the first one is something I’d consider bug either.   Per spec, we need to just support “DataSource” as a parameter type.  Not any of the subclasses or concrete implementations.   
> 
> Wink actually has a test that asserts that using:
> public DataSource postDataSource(FileDataSource ds)
> 
> would result in a HTTP failure.   That said, a classCastException is probably not the BEST error here, but it’s certainly not wrong.  
> 
> Anyway, not a critical bug.  Potentially a feature request.   Likely something you could implement your own provider for if you really need it.
> 
> Dan
> 
> 
> 
> On Jun 30, 2014, at 9:38 PM, Daniel Kulp <dk...@apache.org> wrote:
> 
>> 
>> On Jun 30, 2014, at 9:07 PM, iris ding <ir...@gmail.com> wrote:
>> 
>>> Hi Guys,
>>> 
>>> I think There are two issues in
>>> org.apache.cxf.jaxrs.provider.DataSourceProvider:
>>> Issue 1: ClassCastException if you post a FileDataSource in your resource
>>> class: 
>>> Issue 2: Return the original InputStream directly in
>>> InputStreamDataSource.getInputStream().
>>> 
>>> 
>>> I have created a JIRA in 
>>> https://issues.apache.org/jira/browse/CXF-5835 and put more details there.
>>> 
>>> Would you take a look at the problem and see whether we can make any
>>> modification for it?
>>> 
>> 
>> The second one is NOT a bug and should not be fixed.   That’s a bug in your code.  The javadoc for “available()”  specifically says:
>> 
>> "Note that while some implementations of InputStream will return the total number of bytes in the stream, many will not. It is never correct to use the return value of this method to allocate a buffer intended to hold all data in this stream."
>> 
>> CXF tries hard to stream everything (including attachments).  Thus, the “available()” method likely will only return the number of bytes that have been received so far by the server which could be MUCH less than the full length of the attachment.
>> 
>> -- 
>> Daniel Kulp
>> dkulp@apache.org - http://dankulp.com/blog
>> Talend Community Coder - http://coders.talend.com
> 
> -- 
> Daniel Kulp
> dkulp@apache.org - http://dankulp.com/blog
> Talend Community Coder - http://coders.talend.com
> 


Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by iris ding <ir...@gmail.com>.
Thanks Sergey. Let's keep an eye on what is going on for the JAXRS SPEC
clarification.

Iris Ding



--
View this message in context: http://cxf.547215.n5.nabble.com/two-issues-in-org-apache-cxf-jaxrs-provider-DataSourceProvider-tp5745783p5745974.html
Sent from the cxf-dev mailing list archive at Nabble.com.

Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by Sergey Beryozkin <sb...@gmail.com>.
On 02/07/14 10:58, Sergey Beryozkin wrote:
> Hi
> On 02/07/14 02:45, iris ding wrote:
>> Thanks Sergey for your fix for this issue.
>>
>> But actually I have a question for the fix as it is against spec for
>> support
>> of subclasses or concrete implementations for DataSource. I have put my
>> question in https://issues.apache.org/jira/browse/CXF-5835 as well.
>>
> I doubt it is against the spec for a default provider to give a best
> effort and support some of DataSource implementing classes from
> javax.activation OOB. It would probably be reasonable to throw an
> exception too but it is probably too harsh given that FileDataSource is
> the only well-known DataSource implementation that we can support for
> 'free' given that the spec requires support for File.
>
> Of course, we can say that if a user typed ByteArrayInputStream, then
> should we copy InputStream into ByteArrayInputStream too based on the
> fact ByteArrayInputStream is a well known type. But it won't work right
> now. However we'd support DOMSource even though the spec requires
> supporting Source only, etc.
>
> The only reason FileDataSource is supported now is that File has to be
> supported per the spec.
>
> That said I will request a clarification in the spec. My preference
> would be to have a text added along the lines of "Default Message Body
> Readers MAY optionally support well-known implementations of interfaces
> required to be supported by default..."
>
> Note it is the issue only for Message Body Readers, obviously Message
> Body Writer must support concrete implementations.

Opened
https://java.net/jira/browse/JAX_RS_SPEC-470

Cheers, Sergey

>> The other question is:
>> In ProviderFactory.initBaseFactory(), CXF will add DataSourceProvider by
>> default. Is there a way to remove the built-in CXF DataSourceProvider but
>> using my own one?
> The custom providers are always preferred, so just register your custom
> implementation.
>
> Cheers, Sergey
>>
>>
>>
>>
>> --
>> View this message in context:
>> http://cxf.547215.n5.nabble.com/two-issues-in-org-apache-cxf-jaxrs-provider-DataSourceProvider-tp5745783p5745869.html
>>
>> Sent from the cxf-dev mailing list archive at Nabble.com.
>>
>

Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by Sergey Beryozkin <sb...@gmail.com>.
Hi
On 02/07/14 02:45, iris ding wrote:
> Thanks Sergey for your fix for this issue.
>
> But actually I have a question for the fix as it is against spec for support
> of subclasses or concrete implementations for DataSource. I have put my
> question in https://issues.apache.org/jira/browse/CXF-5835 as well.
>
I doubt it is against the spec for a default provider to give a best 
effort and support some of DataSource implementing classes from 
javax.activation OOB. It would probably be reasonable to throw an 
exception too but it is probably too harsh given that FileDataSource is 
the only well-known DataSource implementation that we can support for 
'free' given that the spec requires support for File.

Of course, we can say that if a user typed ByteArrayInputStream, then 
should we copy InputStream into ByteArrayInputStream too based on the 
fact ByteArrayInputStream is a well known type. But it won't work right 
now. However we'd support DOMSource even though the spec requires 
supporting Source only, etc.

The only reason FileDataSource is supported now is that File has to be 
supported per the spec.

That said I will request a clarification in the spec. My preference 
would be to have a text added along the lines of "Default Message Body 
Readers MAY optionally support well-known implementations of interfaces 
required to be supported by default..."

Note it is the issue only for Message Body Readers, obviously Message 
Body Writer must support concrete implementations.
> The other question is:
> In ProviderFactory.initBaseFactory(), CXF will add DataSourceProvider by
> default. Is there a way to remove the built-in CXF DataSourceProvider but
> using my own one?
The custom providers are always preferred, so just register your custom 
implementation.

Cheers, Sergey
>
>
>
>
> --
> View this message in context: http://cxf.547215.n5.nabble.com/two-issues-in-org-apache-cxf-jaxrs-provider-DataSourceProvider-tp5745783p5745869.html
> Sent from the cxf-dev mailing list archive at Nabble.com.
>


Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by iris ding <ir...@gmail.com>.
Thanks Sergey for your fix for this issue.

But actually I have a question for the fix as it is against spec for support
of subclasses or concrete implementations for DataSource. I have put my
question in https://issues.apache.org/jira/browse/CXF-5835 as well.

The other question is: 
In ProviderFactory.initBaseFactory(), CXF will add DataSourceProvider by
default. Is there a way to remove the built-in CXF DataSourceProvider but
using my own one?




--
View this message in context: http://cxf.547215.n5.nabble.com/two-issues-in-org-apache-cxf-jaxrs-provider-DataSourceProvider-tp5745783p5745869.html
Sent from the cxf-dev mailing list archive at Nabble.com.

Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by Sergey Beryozkin <sb...@gmail.com>.
Hi
On 01/07/14 07:19, iris ding wrote:
> Thanks Daniel for digging out the issue.
>
> So how about below change to convert the ClassCastException to a 415
> response?
>
>      public T readFrom(Class<T> cls, Type genericType, Annotation[]
> annotations,
>                                 MediaType type,
>                                 MultivaluedMap<String, String> headers,
> InputStream is)
>          throws IOException,WebApplicationException {
>          DataSource ds = new InputStreamDataSource(is, type.toString());
>          try{
>          return cls.cast(DataSource.class.isAssignableFrom(cls) ? ds : new
> DataHandler(ds));
>          }
>          catch (ClassCastException e)
>          {
>          	
>          	WebApplicationException ee= new
> WebApplicationException(e,Response.status(415).build());
>          	throw ee;
>          	
>          }
>      }

I'll have a look into fixing the cast exception

>
> I also have a question here, you mentioned we can implement our own
> provider, How can we replace the CXF built in DataSourceProvider with our
> own?
>
you'd just register it in jaxrs:providers. The auto-discovery is also 
possible, for Spring only endpoints for now.

Cheers, Sergey
>
>
> --
> View this message in context: http://cxf.547215.n5.nabble.com/two-issues-in-org-apache-cxf-jaxrs-provider-DataSourceProvider-tp5745783p5745792.html
> Sent from the cxf-dev mailing list archive at Nabble.com.
>


Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by iris ding <ir...@gmail.com>.
Thanks Daniel for digging out the issue.

So how about below change to convert the ClassCastException to a 415
response?

    public T readFrom(Class<T> cls, Type genericType, Annotation[]
annotations, 
                               MediaType type, 
                               MultivaluedMap<String, String> headers,
InputStream is)
        throws IOException,WebApplicationException {
        DataSource ds = new InputStreamDataSource(is, type.toString());
        try{
        return cls.cast(DataSource.class.isAssignableFrom(cls) ? ds : new
DataHandler(ds));
        }
        catch (ClassCastException e)
        {
        	
        	WebApplicationException ee= new
WebApplicationException(e,Response.status(415).build());
        	throw ee;
        	
        }
    }

I also have a question here, you mentioned we can implement our own
provider, How can we replace the CXF built in DataSourceProvider with our
own? 



--
View this message in context: http://cxf.547215.n5.nabble.com/two-issues-in-org-apache-cxf-jaxrs-provider-DataSourceProvider-tp5745783p5745792.html
Sent from the cxf-dev mailing list archive at Nabble.com.

Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by Daniel Kulp <dk...@apache.org>.
I’m not sure the first one is something I’d consider bug either.   Per spec, we need to just support “DataSource” as a parameter type.  Not any of the subclasses or concrete implementations.   

Wink actually has a test that asserts that using:
public DataSource postDataSource(FileDataSource ds)

would result in a HTTP failure.   That said, a classCastException is probably not the BEST error here, but it’s certainly not wrong.  

Anyway, not a critical bug.  Potentially a feature request.   Likely something you could implement your own provider for if you really need it.

Dan



On Jun 30, 2014, at 9:38 PM, Daniel Kulp <dk...@apache.org> wrote:

> 
> On Jun 30, 2014, at 9:07 PM, iris ding <ir...@gmail.com> wrote:
> 
>> Hi Guys,
>> 
>> I think There are two issues in
>> org.apache.cxf.jaxrs.provider.DataSourceProvider:
>> Issue 1: ClassCastException if you post a FileDataSource in your resource
>> class: 
>> Issue 2: Return the original InputStream directly in
>> InputStreamDataSource.getInputStream().
>> 
>> 
>> I have created a JIRA in 
>> https://issues.apache.org/jira/browse/CXF-5835 and put more details there.
>> 
>> Would you take a look at the problem and see whether we can make any
>> modification for it?
>> 
> 
> The second one is NOT a bug and should not be fixed.   That’s a bug in your code.  The javadoc for “available()”  specifically says:
> 
> "Note that while some implementations of InputStream will return the total number of bytes in the stream, many will not. It is never correct to use the return value of this method to allocate a buffer intended to hold all data in this stream."
> 
> CXF tries hard to stream everything (including attachments).  Thus, the “available()” method likely will only return the number of bytes that have been received so far by the server which could be MUCH less than the full length of the attachment.
> 
> -- 
> Daniel Kulp
> dkulp@apache.org - http://dankulp.com/blog
> Talend Community Coder - http://coders.talend.com

-- 
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com


Re: two issues in org.apache.cxf.jaxrs.provider.DataSourceProvider

Posted by Daniel Kulp <dk...@apache.org>.
On Jun 30, 2014, at 9:07 PM, iris ding <ir...@gmail.com> wrote:

> Hi Guys,
> 
> I think There are two issues in
> org.apache.cxf.jaxrs.provider.DataSourceProvider:
> Issue 1: ClassCastException if you post a FileDataSource in your resource
> class: 
> Issue 2: Return the original InputStream directly in
> InputStreamDataSource.getInputStream().
> 
> 
> I have created a JIRA in 
> https://issues.apache.org/jira/browse/CXF-5835 and put more details there.
> 
> Would you take a look at the problem and see whether we can make any
> modification for it?
> 

The second one is NOT a bug and should not be fixed.   That’s a bug in your code.  The javadoc for “available()”  specifically says:

"Note that while some implementations of InputStream will return the total number of bytes in the stream, many will not. It is never correct to use the return value of this method to allocate a buffer intended to hold all data in this stream."

CXF tries hard to stream everything (including attachments).  Thus, the “available()” method likely will only return the number of bytes that have been received so far by the server which could be MUCH less than the full length of the attachment.

-- 
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com