Camel
  1. Camel
  2. CAMEL-5458

Jetty/HTTP Pathless Consumer matchOnUriPrefix=true Breaks Producer bridgeEndpoint=true

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Not a Problem
    • Affects Version/s: 2.10.0
    • Fix Version/s: 2.11.0
    • Component/s: camel-http, camel-jetty
    • Labels:
      None
    • Estimated Complexity:
      Unknown

      Description

      When a Jetty/HTTP endpoint that has the matchOnUriPrefix=true option set and is configured to match any URI (no path is specified) the HttpHelper concatenates the consumer path to the producer endpoint creating an HttpOperationUnsupportedException.

      For example the following will process as expected and the HttpHelper class will create the http://localhost:10021/proxy/ping consumer URI:

      from("jetty:http://localhost:10020/ping?matchOnUriPrefix=true")
          .to("jetty:http://localhost:10021/proxy/ping?bridgeEndpoint=true");
      

      While this code will create http://localhost:10021/proxy/ping/ping and create the exception (the only difference is there isn't a path on the consumer):

      from("jetty:http://localhost:10020?matchOnUriPrefix=true")
          .to("jetty:http://localhost:10021/proxy/ping?bridgeEndpoint=true");
      

      Test case and patch will be uploaded shortly.

      Thanks,
      Scott ES
      http://fusesource.com

      1. CAMEL-5458.patch
        19 kB
        Scott England-Sullivan
      2. CAMEL-5458-v2.patch
        26 kB
        Scott England-Sullivan
      3. CAMEL-5458-willem.patch
        27 kB
        Willem Jiang
      4. JettyMatchOnUriPrefixBridgedRouteTest.java
        10 kB
        Scott England-Sullivan

        Issue Links

          Activity

          Hide
          Claus Ibsen added a comment -

          This works as designed.

          I created another ticket for adding a new url rewrite plugin functionality.

          Show
          Claus Ibsen added a comment - This works as designed. I created another ticket for adding a new url rewrite plugin functionality.
          Hide
          Claus Ibsen added a comment -

          I wonder if we could introduce a pluggable API and then maybe add a Camel component for the url rewrite filter project
          http://tuckey.org/urlrewrite/

          I would assume it got all the bells and whistles, so maybe we can hook into it, to leverage its rules, and compute the url for us.

          Show
          Claus Ibsen added a comment - I wonder if we could introduce a pluggable API and then maybe add a Camel component for the url rewrite filter project http://tuckey.org/urlrewrite/ I would assume it got all the bells and whistles, so maybe we can hook into it, to leverage its rules, and compute the url for us.
          Hide
          Scott England-Sullivan added a comment -

          I was trying to accomplish the same effect without changing the component using a distribution list. I think your suggestion is cleaner though.

          I will take a look at it next week when I get back in town. At a customer site this week.

          Show
          Scott England-Sullivan added a comment - I was trying to accomplish the same effect without changing the component using a distribution list. I think your suggestion is cleaner though. I will take a look at it next week when I get back in town. At a customer site this week.
          Hide
          Claus Ibsen added a comment -

          Just thinking out loud. Maybe we need some simple way of doing light-weight mod_rewrite. Some way of specifying url mapping ruls.
          And then you can refer to that in the url configuration .to("jetty....?...mappingRules=#myMappingRules")

          Show
          Claus Ibsen added a comment - Just thinking out loud. Maybe we need some simple way of doing light-weight mod_rewrite. Some way of specifying url mapping ruls. And then you can refer to that in the url configuration .to("jetty....?...mappingRules=#myMappingRules")
          Hide
          Scott England-Sullivan added a comment -

          This Jetty Unit tests that show the behavior of mixing matchOnUriPrefix and bridgeEndpoint. Test 6 & 8 will fail when attempting to match to root.

          Show
          Scott England-Sullivan added a comment - This Jetty Unit tests that show the behavior of mixing matchOnUriPrefix and bridgeEndpoint. Test 6 & 8 will fail when attempting to match to root.
          Hide
          Claus Ibsen added a comment -

          Ad 1)
          Is the exception on the producer, eg when it calls :10021 ? or from the consumer?

          Ad 2)
          So you want all kind of requests from 10020 to be redirected to a hardcoded url on 10021, which is /proxy/ping ?

          Show
          Claus Ibsen added a comment - Ad 1) Is the exception on the producer, eg when it calls :10021 ? or from the consumer? Ad 2) So you want all kind of requests from 10020 to be redirected to a hardcoded url on 10021, which is /proxy/ping ?
          Hide
          Scott England-Sullivan added a comment -

          There two issues then:

          1. It will thrown an exception if you make a request to root. If you issue a request to *http://localhost:10020* the following route will throw an HttpOperationUnsupportedException. The code doesn't handle a root path of "/" or "".:
            from("jetty:http://localhost:10020?matchOnUriPrefix=true")
                .to("jetty:http://localhost:10021/proxy/ping?bridgeEndpoint=true");
          2. The behavior I am trying to achieve is probably better thought of as redirection. If you change or alter the headers any error will display the producer endpoint and not the original path used to connect to the consumer endpoint. Therefore, what is needed is that the producer should ignore all headers and make the request to the given endpoint and change nothing in the headers. While I agree that I don't want another property I am not 100% convinced how to achieve this behavior.

          I have a fix for item 1. I don't know what is the best approach for 2.

          Thoughts?

          Show
          Scott England-Sullivan added a comment - There two issues then: It will thrown an exception if you make a request to root. If you issue a request to * http://localhost:10020* the following route will throw an HttpOperationUnsupportedException. The code doesn't handle a root path of "/" or "".: from( "jetty:http: //localhost:10020?matchOnUriPrefix= true " ) .to( "jetty:http: //localhost:10021/proxy/ping?bridgeEndpoint= true " ); The behavior I am trying to achieve is probably better thought of as redirection. If you change or alter the headers any error will display the producer endpoint and not the original path used to connect to the consumer endpoint. Therefore, what is needed is that the producer should ignore all headers and make the request to the given endpoint and change nothing in the headers. While I agree that I don't want another property I am not 100% convinced how to achieve this behavior. I have a fix for item 1. I don't know what is the best approach for 2. Thoughts?
          Hide
          Claus Ibsen added a comment - - edited

          It will always bridge to the path you requested, so if you request /ping, then the path /ping is appended to the "to" endpoint uri.
          So if you call /pong, then it becomes /proxy/ping/pong.

          If you always want to call the same uri, then you should maybe not bridge the endpoints, but a plain to instead. And then you should remove the CamelHttp headers beforehand (or possible there is some option you can configure to suppress the uri header in the from).

          Show
          Claus Ibsen added a comment - - edited It will always bridge to the path you requested, so if you request /ping, then the path /ping is appended to the "to" endpoint uri. So if you call /pong, then it becomes /proxy/ping/pong. If you always want to call the same uri, then you should maybe not bridge the endpoints, but a plain to instead. And then you should remove the CamelHttp headers beforehand (or possible there is some option you can configure to suppress the uri header in the from).
          Hide
          Scott England-Sullivan added a comment -

          Hi Claus,

          To restate the issue:

          When a Jetty/HTTP endpoint that has the matchOnUriPrefix=true option set and is configured to match any URI (no path is specified) the HttpHelper concatenates the consumer path to the producer endpoint creating an HttpOperationUnsupportedException.

          For example when sending a request to *http://localhost:10020/ping* the following route will process as expected:

          from("jetty:http://localhost:10020/ping?matchOnUriPrefix=true")
              .to("jetty:http://localhost:10021/proxy/ping?bridgeEndpoint=true");
          

          The following route though will forward requests to *http://localhost:10021/proxy/ping/ping*, note the second ping, and throw an exception:

          from("jetty:http://localhost:10020?matchOnUriPrefix=true")
              .to("jetty:http://localhost:10021/proxy/ping?bridgeEndpoint=true");
          

          The documentation says:
          bridgeEndpoint | false | If the option is true, then the Exchange.HTTP_URI header is ignored, and use the endpoint's URI for request. You may also set the throwExcpetionOnFailure to be false to let the AhcProducer send all the fault response back

          This is pretty vague but as I read it, I see that it should redirect requests to the endpoint I have specified.

          Scott ES
          http://fusesource.com

          Show
          Scott England-Sullivan added a comment - Hi Claus, To restate the issue: When a Jetty/HTTP endpoint that has the matchOnUriPrefix=true option set and is configured to match any URI (no path is specified) the HttpHelper concatenates the consumer path to the producer endpoint creating an HttpOperationUnsupportedException. For example when sending a request to * http://localhost:10020/ping* the following route will process as expected: from( "jetty:http: //localhost:10020/ping?matchOnUriPrefix= true " ) .to( "jetty:http: //localhost:10021/proxy/ping?bridgeEndpoint= true " ); The following route though will forward requests to * http://localhost:10021/proxy/ping/ping* , note the second ping, and throw an exception: from( "jetty:http: //localhost:10020?matchOnUriPrefix= true " ) .to( "jetty:http: //localhost:10021/proxy/ping?bridgeEndpoint= true " ); The documentation says: bridgeEndpoint | false | If the option is true, then the Exchange.HTTP_URI header is ignored, and use the endpoint's URI for request . You may also set the throwExcpetionOnFailure to be false to let the AhcProducer send all the fault response back This is pretty vague but as I read it, I see that it should redirect requests to the endpoint I have specified. Scott ES http://fusesource.com
          Hide
          Claus Ibsen added a comment -

          Can we avoid adding more options to confuse people? What is scott's problem? People have bridged/proxied http for a long time with Camel without any issues.

          Show
          Claus Ibsen added a comment - Can we avoid adding more options to confuse people? What is scott's problem? People have bridged/proxied http for a long time with Camel without any issues.
          Hide
          Willem Jiang added a comment -

          It sounds good and I'm looking forward your new patch.
          BTW, for the fix path feature, it could still be useful if the user don't want the PATH header bother the request URI creation.

          Show
          Willem Jiang added a comment - It sounds good and I'm looking forward your new patch. BTW, for the fix path feature, it could still be useful if the user don't want the PATH header bother the request URI creation.
          Hide
          Scott England-Sullivan added a comment -

          Hi Willem,

          So it sounds like we have two things here. One is your improvement and the other is the broken docs and logic when the match base path is empty ("") or root . This breaks the bridgeEndpoint. I can create a patch to fix just that part and still allow backwards compatibility with the current capabilities. I will also update the documentation on the wiki to reflect the accurate behavior.

          With that it seems to make sense that you should open an improvement ticket for your new feature so we have a separate and clear documentation trail. I would like suggest changing the name to either bridgeEndpointFixedPath as it is still a proxy like bridgeEndpoint but with a fixed behavior.

          Thoughts?

          Show
          Scott England-Sullivan added a comment - Hi Willem, So it sounds like we have two things here. One is your improvement and the other is the broken docs and logic when the match base path is empty ("") or root . This breaks the bridgeEndpoint. I can create a patch to fix just that part and still allow backwards compatibility with the current capabilities. I will also update the documentation on the wiki to reflect the accurate behavior. With that it seems to make sense that you should open an improvement ticket for your new feature so we have a separate and clear documentation trail. I would like suggest changing the name to either bridgeEndpointFixedPath as it is still a proxy like bridgeEndpoint but with a fixed behavior. Thoughts?
          Hide
          Willem Jiang added a comment -

          Hi Scott,

          For the camel-http producer it will build up the request URI based on the HTTP_URI header or the endpoint URI plus the path info, that is the default behavior.

          I think the document is not rightly, and when we enable the matchOnUriPrefix option, the bridgeEndpoint just ignore the Exchange.HTTP_URI, and use the endopint URI with the path infor to build up the request URI. In this way camel can provide the typical proxy feature out of box.

          In my patch, I just introduced a usingFixPath option to ignore the path info, in this way it will not break anything before.

          Regards,

          Willem

          Show
          Willem Jiang added a comment - Hi Scott, For the camel-http producer it will build up the request URI based on the HTTP_URI header or the endpoint URI plus the path info, that is the default behavior. I think the document is not rightly, and when we enable the matchOnUriPrefix option, the bridgeEndpoint just ignore the Exchange.HTTP_URI, and use the endopint URI with the path infor to build up the request URI. In this way camel can provide the typical proxy feature out of box. In my patch, I just introduced a usingFixPath option to ignore the path info, in this way it will not break anything before. Regards, Willem
          Hide
          Scott England-Sullivan added a comment -

          Willem,

          The documentation is:

          bridgeEndpoint | false | If the option is true, then the Exchange.HTTP_URI header is ignored, and use the endpoint's URI for request. You may also set the throwExcpetionOnFailure to be false to let the AhcProducer send all the fault response back.

          There is nothing about appending the current path to the supplied endpoint. It states that it will use the endpoints URI. So maybe the question is, which endpoint is the documentation referring to?

          Also, if you remove all the patches and run just my unit test, it will show you several ways it breaks using the current logic.

          Either way you look at it, the functionality is definitely broken on some level and is not delivering on the documented intent.

          I am not sure what the best way to handle that is but I think these questions need to be answered first.

          I appreciate your help and patients,

          Best Regards,
          Scott ES

          Show
          Scott England-Sullivan added a comment - Willem, The documentation is: bridgeEndpoint | false | If the option is true, then the Exchange.HTTP_URI header is ignored, and use the endpoint's URI for request . You may also set the throwExcpetionOnFailure to be false to let the AhcProducer send all the fault response back. There is nothing about appending the current path to the supplied endpoint. It states that it will use the endpoints URI . So maybe the question is, which endpoint is the documentation referring to? Also, if you remove all the patches and run just my unit test, it will show you several ways it breaks using the current logic. Either way you look at it, the functionality is definitely broken on some level and is not delivering on the documented intent. I am not sure what the best way to handle that is but I think these questions need to be answered first. I appreciate your help and patients, Best Regards, Scott ES
          Hide
          Willem Jiang added a comment -

          Hi Scott,

          For your case, the relative path is "" if the request is http://localhost:10020/foo, but if you request is http://localhost:10020/foo/bar, the relative path is "/bar", Camel should not send the request to http://localhost:10021/proxy/bar any more , the right url should be http://localhost:10021/proxy/bar/bar.

          Show
          Willem Jiang added a comment - Hi Scott, For your case, the relative path is "" if the request is http://localhost:10020/foo , but if you request is http://localhost:10020/foo/bar , the relative path is "/bar", Camel should not send the request to http://localhost:10021/proxy/bar any more , the right url should be http://localhost:10021/proxy/bar/bar .
          Hide
          Scott England-Sullivan added a comment -

          Hey Willem,

          I think you are focused on the broken behavior only. Try the following:

          from("jetty:http://localhost:10020/foo?matchOnUriPrefix=true")
              .to("jetty:http://localhost:10021/proxy/bar?bridgeEndpoint=true");
          

          If you send a request to http://localhost:10020/foo I bet it will send it to http://localhost:10021/proxy/bar as expected and as documented. bridgeEndpoint states it will use what is defined in the endpoint not in any variable. On that alone it is broken.

          Show
          Scott England-Sullivan added a comment - Hey Willem, I think you are focused on the broken behavior only. Try the following: from( "jetty:http: //localhost:10020/foo?matchOnUriPrefix= true " ) .to( "jetty:http: //localhost:10021/proxy/bar?bridgeEndpoint= true " ); If you send a request to http://localhost:10020/foo I bet it will send it to http://localhost:10021/proxy/bar as expected and as documented. bridgeEndpoint states it will use what is defined in the endpoint not in any variable. On that alone it is broken.
          Hide
          Willem Jiang added a comment -

          @Scott,
          I think the change still break the feature that camel have.
          As you know if we want to setup a proxy, we need to pass the path info for the bridge endpoint, it is the proxy behavior by default.

          Let take your use case in the JIRA description as an example:

          from("jetty:http://localhost:10020/ping?matchOnUriPrefix=true")
              .to("jetty:http://localhost:10021/proxy/ping?bridgeEndpoint=true");
          

          If you send the request to "http://localhost:10020/ping" the path info is "", so camel route the request to "http://localhost:10021/proxy/ping"

          from("jetty:http://localhost:10020?matchOnUriPrefix=true")
              .to("jetty:http://localhost:10021/proxy/ping?bridgeEndpoint=true");
          

          If you send the request to "http://localhost:10020/ping" the path info is "/ping", so camel route the request to "http://localhost:10021/proxy/ping/ping"

          So I don't think this is an bug of Camel. That is why I plan to introduce the usingFixPath which ignore the PATH information when routing the message to the bridge endpoint.

          Show
          Willem Jiang added a comment - @Scott, I think the change still break the feature that camel have. As you know if we want to setup a proxy, we need to pass the path info for the bridge endpoint, it is the proxy behavior by default. Let take your use case in the JIRA description as an example: from( "jetty:http: //localhost:10020/ping?matchOnUriPrefix= true " ) .to( "jetty:http: //localhost:10021/proxy/ping?bridgeEndpoint= true " ); If you send the request to "http://localhost:10020/ping" the path info is "", so camel route the request to "http://localhost:10021/proxy/ping" from( "jetty:http: //localhost:10020?matchOnUriPrefix= true " ) .to( "jetty:http: //localhost:10021/proxy/ping?bridgeEndpoint= true " ); If you send the request to "http://localhost:10020/ping" the path info is "/ping", so camel route the request to "http://localhost:10021/proxy/ping/ping" So I don't think this is an bug of Camel. That is why I plan to introduce the usingFixPath which ignore the PATH information when routing the message to the bridge endpoint.
          Hide
          Scott England-Sullivan added a comment -

          I went back through and after some testing found a better way to handle it. I also applied it to camel-ach & camel-http4. I did a search through all the components that also had a dependency on jetty/http and for matchOnUriPrefix and bridgeEndpoint options. Ran an built them all against the new patch and everything built cleanly. This shouldn't break any backwards compatibility given that it didn't work in the first place.

          Let me know if you see any other issues.

          Show
          Scott England-Sullivan added a comment - I went back through and after some testing found a better way to handle it. I also applied it to camel-ach & camel-http4. I did a search through all the components that also had a dependency on jetty/http and for matchOnUriPrefix and bridgeEndpoint options. Ran an built them all against the new patch and everything built cleanly. This shouldn't break any backwards compatibility given that it didn't work in the first place. Let me know if you see any other issues.
          Hide
          Scott England-Sullivan added a comment -

          Patch revision

          Show
          Scott England-Sullivan added a comment - Patch revision
          Hide
          Scott England-Sullivan added a comment -

          Thanks Willem. I will take a look at it today. I will also see if I can come up with a fix that won't require a new property.

          Show
          Scott England-Sullivan added a comment - Thanks Willem. I will take a look at it today. I will also see if I can come up with a fix that won't require a new property.
          Hide
          Willem Jiang added a comment -

          It's the JettyHttpTest#testGetWithRelativePath.
          When the camel-http producer working in proxy model, it should build the request with the camel path info, like the test just showed, you patch just broke the behavior we have before.

          Here is the patch that I'm planing to apply, I also update the code of camel-http4 and camel-ahc components which has the same issue as the camel-http.

          Show
          Willem Jiang added a comment - It's the JettyHttpTest#testGetWithRelativePath. When the camel-http producer working in proxy model, it should build the request with the camel path info, like the test just showed, you patch just broke the behavior we have before. Here is the patch that I'm planing to apply, I also update the code of camel-http4 and camel-ahc components which has the same issue as the camel-http.
          Hide
          Scott England-Sullivan added a comment -

          Hi Willem,

          Can you point me to what is broken? Both the HTTP and Jetty component built without error. The new unit test I supplied tested bridgeEndpoint with matchOnUriPrefix set to true and false and URIs that contained a matched context, mismatched context and ANY. The only one that is broken without the patch is "ANY" where the consumer jetty endpoint is jetty:http://address:port or jetty:http://address:port/. In both of these cases the incoming URI path is appended regardless. This isn't the documented behavior nor expected behavior given that it works with a context.

          Also, is adding a new option to fix a broken behavior the right option?

          Let me know what you found that is broken and I will take a further look.

          Thanks,
          Scott ES
          http://fusesource.com

          Show
          Scott England-Sullivan added a comment - Hi Willem, Can you point me to what is broken? Both the HTTP and Jetty component built without error. The new unit test I supplied tested bridgeEndpoint with matchOnUriPrefix set to true and false and URIs that contained a matched context, mismatched context and ANY. The only one that is broken without the patch is "ANY" where the consumer jetty endpoint is jetty: http://address:port or jetty: http://address:port/ . In both of these cases the incoming URI path is appended regardless. This isn't the documented behavior nor expected behavior given that it works with a context. Also, is adding a new option to fix a broken behavior the right option? Let me know what you found that is broken and I will take a further look. Thanks, Scott ES http://fusesource.com
          Hide
          Willem Jiang added a comment -

          Hi Scott,

          Your patch break the old camel proxy behavior, so I introduced an option "usingFixPath" option to implement the feature that you want.

          Willem

          Show
          Willem Jiang added a comment - Hi Scott, Your patch break the old camel proxy behavior, so I introduced an option "usingFixPath" option to implement the feature that you want. Willem
          Hide
          Scott England-Sullivan added a comment -

          Hello,

          I have uploaded a patch. It contains the following:

          Camel-HTTP:
          HttpHelper.createURL(Exchange exchange, HttpEndpoint endpoint) now uses only the Endpoints defined URI and ignores the headers as documented.

          Camel-Jetty:
          JettyContentExchange.getUrl() did not set the ":" after the getScheme() and before the "//".
          Added JettyMatchOnUriPrefixBridgedRouteTest that covers all variations of matched and unmatched bridged endpoint URIs.

          Please let me know if you have any comments.

          Thanks,
          Scott ES
          http://fusesource.com

          Show
          Scott England-Sullivan added a comment - Hello, I have uploaded a patch. It contains the following: Camel-HTTP: HttpHelper.createURL(Exchange exchange, HttpEndpoint endpoint) now uses only the Endpoints defined URI and ignores the headers as documented. Camel-Jetty: JettyContentExchange.getUrl() did not set the ":" after the getScheme() and before the "//". Added JettyMatchOnUriPrefixBridgedRouteTest that covers all variations of matched and unmatched bridged endpoint URIs. Please let me know if you have any comments. Thanks, Scott ES http://fusesource.com
          Hide
          Scott England-Sullivan added a comment -

          Contains fixes for bridgeEndpoint when combined with matchOnUriPrefix and associated unit test.

          Show
          Scott England-Sullivan added a comment - Contains fixes for bridgeEndpoint when combined with matchOnUriPrefix and associated unit test.
          Hide
          Scott England-Sullivan added a comment -

          After further debugging I found the error was further up in the consumer and not in the recipient list.

          Show
          Scott England-Sullivan added a comment - After further debugging I found the error was further up in the consumer and not in the recipient list.

            People

            • Assignee:
              Claus Ibsen
              Reporter:
              Scott England-Sullivan
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development