Mailing-List: contact dev-help@cxf.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cxf.apache.org
Content-Type: text/plain; charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2070.6\))
Subject: Re: [Proposal] WS-Transfer/WS-Fragment implementation
From: Daniel Kulp <dkulp@apache.org>
In-Reply-To: <1423771411108-5754317.post@n5.nabble.com>
Date: Fri, 13 Feb 2015 15:23:12 -0500
Content-Transfer-Encoding: quoted-printable
Message-Id: <C3839D91-9DC6-4ADA-8BB6-943E89FD3917@apache.org>
References: <1415711832549-5750975.post@n5.nabble.com>
 <1417120280405-5751577.post@n5.nabble.com>
 <1423771411108-5754317.post@n5.nabble.com>
To: dev@cxf.apache.org,
 dudae <dudaerich@gmail.com>


> On Feb 12, 2015, at 3:03 PM, dudae <dudaerich@gmail.com> wrote:
> Do you check the implementation? Could you give me some feedback =
please?
> Thank you.

Just took a quick look at this=E2=80=A6 I completely forgot about this.=20=


There are a couple of obvious things that =E2=80=9Cjump out=E2=80=9D =
that are very quick/easy to fix.  Mostly removing the @author tags =
(highly discouraged at Apache) and there are a few files that need the =
Apache header added (the XML files).    There are also several warnings =
when pulled into Eclipse, but those aren=E2=80=99t big deals at all.    =
The other issue would be the use of the System.out for all the messages =
during the tests causing a lot of output on the console.   Again, easy =
fix and I understand why it=E2=80=99s there during development.

The main thing that jumps out at me as being problematic is the use of =
the JAX-WS handlers (org.apache.cxf.ws.transfer.shared.handlers).   =
Those cause problems as we have to completely break streaming to build =
the SOAPMessage that is passed into those.   I=E2=80=99d very strongly =
encourage flipping those to normal CXF  SOAP interceptors.   They seem =
to only operate on SOAP headers so they can call the getHeaders method =
on the SOAPMessage passed into the interceptor and pretty much allow the =
body to remain as is.  (likely streaming)   That said, I=E2=80=99m not =
sure those are even needed.  CXF=E2=80=99s MAPCodec.java already gathers =
the headers that have the ReferenceParameter attribute and adds them to =
the =E2=80=9CTo=E2=80=9D EndpointReference.   You may need to experiment =
a bit more to see if they are really getting through (and if not, figure =
out why).    Likewise, on the client, you could directly add them to the =
client RequestContext via the normal way of adding headers:

=
http://cxf.apache.org/faq.html#FAQ-HowcanIaddsoapheaderstotherequest/respo=
nse?

and avoid the JAX-WS handler/interceptor entirely.


Few more CXF things:
In XLSTResourceTransformer and TransferTools and a few other places, you =
are creating DocumentBuilderFactory/DocumentBuilder and a Transformer =
and such.   You really should use the utility methods we have in =
StaxUtils, XMLUtils, and XSLTUtils for much of that.    We try to make =
sure all the XML parsing and processing goes through those utilities so =
that we can control various aspects to prevent various attacks (like =
entity expansion attacks). =20

Other than that, it looks fairly good.     I hope the above gives you =
some stuff to think about.=20

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