Hi,
thank you for your comments. I fixed mentioned issues, see comments bellow.
Dňa 13.02.2015 o 21:23 Daniel Kulp napísal(a):
On Feb 12, 2015, at 3:03 PM, dudae<[email protected]> wrote:
Do you check the implementation? Could you give me some feedback please?
Thank you.
Just took a quick look at this… I completely forgot about this.
There are a couple of obvious things that “jump out” 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’t 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’s there during development.
Should I fixed all PMD warnings? For example, "A method should have only
one exit point, and that should be the last statement in the method". I
think that more exit points (return statements) do some methods more
readable.
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’d 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’m not sure those are even needed. CXF’s MAPCodec.java already gathers the
headers that have the ReferenceParameter attribute and adds them to the “To”
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/response?
and avoid the JAX-WS handler/interceptor entirely.
The ReferenceParameter is possible to send through the request context.
When I tried to send it through the standard headers, it isn't getting
through because MAPCodec removes all WS-Addressing headers (see
discardMAPs method).
Solution. Client side:
AddressingProperties addrProps = new AddressingProperties();
EndpointReferenceType endpoint = new EndpointReferenceType();
endpoint.setReferenceParameters(refParams);
endpoint.setAddress(ContextUtils.getAttributedURI(RESOURCE_ADDRESS));
addrProps.setTo(endpoint);
((BindingProvider)
proxy).getRequestContext().put(JAXWSAConstants.CLIENT_ADDRESSING_PROPERTIES,
addrProps);
Server side:
AddressingProperties addrProps = (AddressingProperties)
((WrappedMessageContext) context
.getMessageContext()).getWrappedMessage()
.getContextualProperty(JAXWSAConstants.ADDRESSING_PROPERTIES_INBOUND);
ReferenceParametersType refParams =
addrProps.getToEndpointReference().getReferenceParameters();
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).
I removed TransferTools. I use for
- parsing, serializing XML - StaxUtils
- creating elements - DOMUtils.createDocument().createElement
- transforming - XSLTUtils
- xpath - XPathUtils
However in FragmentDialectLanguageXPath10 I don't use the XPathUtils,
because I need to recognize, when the XPath throw the exception and when
not. See https://www.java.net/node/681793
Other than that, it looks fairly good. I hope the above gives you some
stuff to think about.
Erich
