CXF
  1. CXF
  2. CXF-1243 Resolve JBoss common jax-ws testsuite issues
  3. CXF-1623

Wrong org.apache.cxf.jaxws.handler.AnnotationHandlerChainBuilder.protocolMatches() method implementation or preconditions

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1
    • Fix Version/s: 2.1.1
    • Component/s: None
    • Labels:
      None

      Activity

      Richard Opalka created issue -
      Hide
      Richard Opalka added a comment -

      The following method is either wrongly implemented or it has wrong preconditions:

      private boolean protocolMatches(Element el, String id) {
      if (id == null)

      { return true; }

      String name = el.getTextContent().trim();
      if ("##SOAP11_HTTP".equals(name))

      { name = "http://schemas.xmlsoap.org/wsdl/soap/http"; }

      else if ("##SOAP11_HTTP_MTOM".equals(name))

      { name = "http://schemas.xmlsoap.org/wsdl/soap/http?mtom=true"; }

      else if ("##SOAP12_HTTP".equals(name))

      { name = "http://www.w3.org/2003/05/soap/bindings/HTTP/"; }

      else if ("##SOAP12_HTTP_MTOM".equals(name))

      { name = "http://www.w3.org/2003/05/soap/bindings/HTTP/?mtom=true"; }

      else if ("##XML_HTTP".equals(name))

      { name = "http://www.w3.org/2004/08/wsdl/http"; }

      return name.contains(id);
      }

      Show
      Richard Opalka added a comment - The following method is either wrongly implemented or it has wrong preconditions: private boolean protocolMatches(Element el, String id) { if (id == null) { return true; } String name = el.getTextContent().trim(); if ("##SOAP11_HTTP".equals(name)) { name = "http://schemas.xmlsoap.org/wsdl/soap/http"; } else if ("##SOAP11_HTTP_MTOM".equals(name)) { name = "http://schemas.xmlsoap.org/wsdl/soap/http?mtom=true"; } else if ("##SOAP12_HTTP".equals(name)) { name = "http://www.w3.org/2003/05/soap/bindings/HTTP/"; } else if ("##SOAP12_HTTP_MTOM".equals(name)) { name = "http://www.w3.org/2003/05/soap/bindings/HTTP/?mtom=true"; } else if ("##XML_HTTP".equals(name)) { name = "http://www.w3.org/2004/08/wsdl/http"; } return name.contains(id); }
      Hide
      Richard Opalka added a comment -

      After execution of line: String name = el.getTextContent().trim();
      the name string variable is initialized to "##SOAP11_HTTP".
      But the string id (method parameter) holds value "http://schemas.xmlsoap.org/soap/"
      (just FYI this value is set in JaxWsServerFactoryBean.createBindingInfo() method
      on line 126 in CXF 2.1 source distribution.)
      Thus the last line in the above method badly returns false instead of expected true.

      Show
      Richard Opalka added a comment - After execution of line: String name = el.getTextContent().trim(); the name string variable is initialized to "##SOAP11_HTTP". But the string id (method parameter) holds value "http://schemas.xmlsoap.org/soap/" (just FYI this value is set in JaxWsServerFactoryBean.createBindingInfo() method on line 126 in CXF 2.1 source distribution.) Thus the last line in the above method badly returns false instead of expected true.
      Hide
      Richard Opalka added a comment -

      The questions are:

      Q1: Is this method implementation correct?

      I personally think yes but it has wrong preconditions.
      It expects id to be one of 5 enumerated values there.

      Q2: Is the hack in JaxWsServerFactoryBean.createBindingInfo() method necessary?

      At least I know this hack breaks the above method (it's preconditions).
      I'm talking about this piece of code:
      ...
      if (binding == null)

      { binding = jaxBid; setBindingId(binding); // this sets correct value first (in my case "http://schemas.xmlsoap.org/wsdl/soap/http" }

      if (binding.equals(SOAPBinding.SOAP11HTTP_BINDING)

      binding.equals(SOAPBinding.SOAP11HTTP_MTOM_BINDING)
      //
      binding.equals(SOAPBinding.SOAP12HTTP_BINDING)
      binding.equals(SOAPBinding.SOAP12HTTP_MTOM_BINDING)) {
      binding = "http://schemas.xmlsoap.org/soap/";
      setBindingId(binding); // but here this hack resets this value to "http://schemas.xmlsoap.org/soap/" and this hacked value later in the execution breaks AnnotationHandlerChainBuilder.protocolMatches() method preconditions
      ...
      Show
      Richard Opalka added a comment - The questions are: Q1: Is this method implementation correct? I personally think yes but it has wrong preconditions. It expects id to be one of 5 enumerated values there. Q2: Is the hack in JaxWsServerFactoryBean.createBindingInfo() method necessary? At least I know this hack breaks the above method (it's preconditions). I'm talking about this piece of code: ... if (binding == null) { binding = jaxBid; setBindingId(binding); // this sets correct value first (in my case "http://schemas.xmlsoap.org/wsdl/soap/http" } if (binding.equals(SOAPBinding.SOAP11HTTP_BINDING) binding.equals(SOAPBinding.SOAP11HTTP_MTOM_BINDING) // binding.equals(SOAPBinding.SOAP12HTTP_BINDING) binding.equals(SOAPBinding.SOAP12HTTP_MTOM_BINDING)) { binding = "http://schemas.xmlsoap.org/soap/"; setBindingId(binding); // but here this hack resets this value to "http://schemas.xmlsoap.org/soap/" and this hacked value later in the execution breaks AnnotationHandlerChainBuilder.protocolMatches() method preconditions ...
      Richard Opalka made changes -
      Field Original Value New Value
      Summary Wrong method implementation in AnnotationHandlerChainBuilder Wrong org.apache.cxf.jaxws.handler.AnnotationHandlerChainBuilder.protocolMatches() method implementation or preconditions
      Hide
      Daniel Kulp added a comment -

      I'll have to dig into the code some more to figure out a correct fix, but I'll try to answer the questions...

      1) Possibly. It may depend on what is needed to get the fix in place.

      2) In JAXWS, the binding (soap/xml) and the transport (http) are merged into a single string. In CXF, they are completely separate so that things like SOAP over JMS and stuff can properly be modeled. Thus, that code maps the JAX-WS merged binding into the form we need that just says "soap" so the proper CXF binding can be picked up. That code is probably "correct", although it's possible that it should be set to "http://schemas.xmlsoap.org/wsdl/soap/" instead of "http://schemas.xmlsoap.org/soap/". If that is done, I think your use case would work fine as the name.contains(id) would actually pass.

      Show
      Daniel Kulp added a comment - I'll have to dig into the code some more to figure out a correct fix, but I'll try to answer the questions... 1) Possibly. It may depend on what is needed to get the fix in place. 2) In JAXWS, the binding (soap/xml) and the transport (http) are merged into a single string. In CXF, they are completely separate so that things like SOAP over JMS and stuff can properly be modeled. Thus, that code maps the JAX-WS merged binding into the form we need that just says "soap" so the proper CXF binding can be picked up. That code is probably "correct", although it's possible that it should be set to "http://schemas.xmlsoap.org/wsdl/soap/" instead of "http://schemas.xmlsoap.org/soap/". If that is done, I think your use case would work fine as the name.contains(id) would actually pass.
      Richard Opalka made changes -
      Affects Version/s 2.1 [ 12312402 ]
      Daniel Kulp made changes -
      Assignee Daniel Kulp [ dkulp ]
      Daniel Kulp made changes -
      Resolution Fixed [ 1 ]
      Status Open [ 1 ] Resolved [ 5 ]
      Fix Version/s 2.1.1 [ 12312971 ]
      Daniel Kulp made changes -
      Status Resolved [ 5 ] Closed [ 6 ]
      Hide
      Richard Opalka added a comment -

      Thanks for the fix

      Show
      Richard Opalka added a comment - Thanks for the fix
      Mark Thomas made changes -
      Workflow jira [ 12432322 ] Default workflow, editable Closed status [ 12604050 ]
      Transition Time In Source Status Execution Times Last Executer Last Execution Date
      Open Open Resolved Resolved
      10d 5h 58m 1 Daniel Kulp 12/Jun/08 21:18
      Resolved Resolved Closed Closed
      20d 21h 1m 1 Daniel Kulp 03/Jul/08 18:19

        People

        • Assignee:
          Daniel Kulp
          Reporter:
          Richard Opalka
        • Votes:
          0 Vote for this issue
          Watchers:
          0 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development