Camel
  1. Camel
  2. CAMEL-4857

Endpoint URI normalization: information in path is lost

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.9.0
    • Fix Version/s: 2.11.0
    • Component/s: camel-core
    • Labels:
      None
    • Estimated Complexity:
      Unknown

      Description

      A project with a test case is here: https://github.com/alaz/camel290_uri

      The test case with the problem description (the problem occured during migration of camel-beanstalk https://github.com/osinka/camel-beanstalk component onto 2.9.0):

      UriTest.java
      package camel290.uri;
      
      import java.util.Map;
      import org.apache.camel.Consumer;
      import org.apache.camel.Endpoint;
      import org.apache.camel.Processor;
      import org.apache.camel.Producer;
      import org.apache.camel.impl.DefaultComponent;
      import org.apache.camel.impl.DefaultEndpoint;
      import org.apache.camel.test.CamelTestSupport;
      import org.junit.Before;
      import org.junit.Test;
      import static org.junit.Assert.*;
      
      public class UriTest extends CamelTestSupport {
      
        /**
         * An URI of Camel Beanstalk component consists of a hostname, port and a list
         * of tube names. Tube names are separated by "+" character (which is more or less
         * usualy used on the Web to make lists), but every tube name may contain URI special
         * characters like ? or +
         */
      
        class MyEndpoint extends DefaultEndpoint {
          String uri = null;
          String remaining = null;
      
          public MyEndpoint(final String uri, final String remaining) {
            this.uri = uri;
            this.remaining = remaining;
          }
      
          public Producer createProducer() throws Exception {
            throw new UnsupportedOperationException("Not supported yet.");
          }
      
          public Consumer createConsumer(Processor prcsr) throws Exception {
            throw new UnsupportedOperationException("Not supported yet.");
          }
      
          public boolean isSingleton() {
            return true;
          }
        }
      
        class MyComponent extends DefaultComponent {
          @Override
          protected Endpoint createEndpoint(final String uri, final String remaining, final Map<String, Object> parameters) throws Exception {
            return new MyEndpoint(uri, remaining);
          }
        }
      
        @Before
        @Override
        public void setUp() throws Exception {
          super.setUp();
          context.addComponent("my", new MyComponent());
        }
      
        @Test
        public void testExclamationInUri() {
          /**
           * %3F is not an ?, it's part of tube name.
           */
          MyEndpoint endpoint = context.getEndpoint("my:host:11303/tube1+tube%2B+tube%3F", MyEndpoint.class);
          assertNotNull("endpoint", endpoint);
        }
      
        @Test
        public void testPath() {
          /**
           * Here a tube name is "tube+" and written in URI as "tube%2B", but it gets
           * normalized, so that an endpoint sees "tube1+tube+"
           */
          MyEndpoint endpoint = context.getEndpoint("my:host:11303/tube1+tube%2B", MyEndpoint.class);
          assertEquals("Path contains several tube names, every tube name may have + or ? characters", "host:11303/tube1+tube%2B", endpoint.remaining);
        }
      }
      

        Issue Links

          Activity

          Hide
          Claus Ibsen added a comment -

          Introduced useRawUri method to Component which gives component writers full power.

          Show
          Claus Ibsen added a comment - Introduced useRawUri method to Component which gives component writers full power.
          Hide
          Claus Ibsen added a comment -

          A patch which allows component writers to control if the uri should be raw or encoded (default as now).

          Show
          Claus Ibsen added a comment - A patch which allows component writers to control if the uri should be raw or encoded (default as now).
          Hide
          Christian Müller added a comment -

          I will go through this until end of this week...

          Show
          Christian Müller added a comment - I will go through this until end of this week...
          Hide
          Babak Vahdat added a comment -

          @Alexander I'm absolutely aware of the fact that through providing that link I did abash you, I'm REALLY sorry for that and please accept my apologies

          However my ONLY INTENTION was to show how frustrating an update could be, that's it! IMHO we had better postponed these URI issues and discussions for the Camel 3 roadmap to AVOID taking any risks and also breaking the existing behaviour given through the minor releases.

          Show
          Babak Vahdat added a comment - @Alexander I'm absolutely aware of the fact that through providing that link I did abash you, I'm REALLY sorry for that and please accept my apologies However my ONLY INTENTION was to show how frustrating an update could be, that's it! IMHO we had better postponed these URI issues and discussions for the Camel 3 roadmap to AVOID taking any risks and also breaking the existing behaviour given through the minor releases.
          Hide
          Alexander Azarov added a comment - - edited

          Babak Vahdat, please do not build your proofs based on my old emotions. In the end of a day, I'm rational in my decisions.

          I perfectly understand every release may bring some bugs. Hadrian Zbarcea explains there were incomplete test coverage of URIs.

          I still wonder why no Camel users have been affected by the same issue, since URI is the main transport of transmitting settings into Camel components. Anyway (a) the number of camel-beanstalk component users is very small and no one complained; (b) the number of voters/watchers of this issue shows no significant interest. Hence, if nobody needs it, it seems logical to postpone the fix.

          Show
          Alexander Azarov added a comment - - edited Babak Vahdat , please do not build your proofs based on my old emotions. In the end of a day, I'm rational in my decisions. I perfectly understand every release may bring some bugs. Hadrian Zbarcea explains there were incomplete test coverage of URIs. I still wonder why no Camel users have been affected by the same issue, since URI is the main transport of transmitting settings into Camel components. Anyway (a) the number of camel-beanstalk component users is very small and no one complained; (b) the number of voters/watchers of this issue shows no significant interest. Hence, if nobody needs it, it seems logical to postpone the fix.
          Hide
          Hadrian Zbarcea added a comment -

          @Babak,

          Although we never were 100% backward compatible in minor releases, the intent is to be 100% backwards compatible. The incompatibility only crept in, I believe, because there are no unit tests for enough variations of URIs. When the changes got committed all the tests passed and it's not like existing tests were deleted or we introduced incompatibilities on purpose. We do strive to be 100% backwards compatible on patch releases though and we are, with very rare exceptions.

          I was the one publishing that release, and believe it or not, vanity wasn't among the reasons. Sad? Maybe. I have a long list myself.

          That said, I totally agree that the the camel-core should be more stable and less incompatibilities should be introduced on minor releases. We can discuss on dev@ if and how this could be achieved.

          Show
          Hadrian Zbarcea added a comment - @Babak, Although we never were 100% backward compatible in minor releases, the intent is to be 100% backwards compatible. The incompatibility only crept in, I believe, because there are no unit tests for enough variations of URIs. When the changes got committed all the tests passed and it's not like existing tests were deleted or we introduced incompatibilities on purpose. We do strive to be 100% backwards compatible on patch releases though and we are, with very rare exceptions. I was the one publishing that release, and believe it or not, vanity wasn't among the reasons. Sad? Maybe. I have a long list myself. That said, I totally agree that the the camel-core should be more stable and less incompatibilities should be introduced on minor releases. We can discuss on dev@ if and how this could be achieved.
          Hide
          Babak Vahdat added a comment - - edited

          IMHO the CORE question should not be if Camel uses URIs correctly or not but more important than that if Camel is REALLY 100% backward-compatible by it's minor releases which to my understanding is much much more important than to be 100% IETF RFC_XYZ complaint.

          In this concrete case the user reporting this issue (on Jan. 3rd) was struggling to upgrade to a newer Camel version a day before (on Jan. 2nd) which then ended up with:
          https://plus.google.com/103504600126086444972/posts/5q1aFtYLryi

          @Alexander please correct me if I'm wrong.

          Anyway, I think this's really sad

          Show
          Babak Vahdat added a comment - - edited IMHO the CORE question should not be if Camel uses URIs correctly or not but more important than that if Camel is REALLY 100% backward-compatible by it's minor releases which to my understanding is much much more important than to be 100% IETF RFC_XYZ complaint. In this concrete case the user reporting this issue (on Jan. 3rd) was struggling to upgrade to a newer Camel version a day before (on Jan. 2nd) which then ended up with: https://plus.google.com/103504600126086444972/posts/5q1aFtYLryi @Alexander please correct me if I'm wrong. Anyway, I think this's really sad
          Hide
          Hadrian Zbarcea added a comment -

          I beg to differ.

          The question is not if Camel is too restrictive, but rather if camel uses URIs or not. What a URI is and isn't is clearly defined by a spec. The changes for CAMEL-4256 were intended to still support the invalid uris we now use in Camel and provide a migration path for 3.0. If some scenarios were missed, that can be fixed.

          However the question still remains, does Camel use URIs or not? Today, I wouldn't know how to define the concept of URI flexibility. I totally agree that that component writers should have complete control on how to handle URIs but those should be URIs to start with. That's not the case today, is it?

          Show
          Hadrian Zbarcea added a comment - I beg to differ. The question is not if Camel is too restrictive, but rather if camel uses URIs or not. What a URI is and isn't is clearly defined by a spec. The changes for CAMEL-4256 were intended to still support the invalid uris we now use in Camel and provide a migration path for 3.0. If some scenarios were missed, that can be fixed. However the question still remains, does Camel use URIs or not? Today, I wouldn't know how to define the concept of URI flexibility. I totally agree that that component writers should have complete control on how to handle URIs but those should be URIs to start with. That's not the case today, is it?
          Hide
          Claus Ibsen added a comment -

          I noticed that CAMEL-4256 or thereabouts also broke some previous URI validation. For example we have a check for double ampersand, that is now being normalized into an empty parameter "" -> null.

          Show
          Claus Ibsen added a comment - I noticed that CAMEL-4256 or thereabouts also broke some previous URI validation. For example we have a check for double ampersand, that is now being normalized into an empty parameter "" -> null.
          Hide
          Claus Ibsen added a comment -

          CAMEL-4256 is the real cause. It is too restrictive in terms of URI flexibility. We need to open up this so the component writers have the power back again, and can totally decide how to handle uri's in their components.

          Show
          Claus Ibsen added a comment - CAMEL-4256 is the real cause. It is too restrictive in terms of URI flexibility. We need to open up this so the component writers have the power back again, and can totally decide how to handle uri's in their components.
          Hide
          Willem Jiang added a comment -

          The issue is caused by CAMEL-4425, now camel is try to turn the encoded uri string to be a normalized one. That is why the "%2B" is changed to "+", "%3F" is changed to "?" at the end.

          It think we need to find a way to resolve the issue of using unsafe uri code in camel uri.

          Willem

          Show
          Willem Jiang added a comment - The issue is caused by CAMEL-4425 , now camel is try to turn the encoded uri string to be a normalized one. That is why the "%2B" is changed to "+", "%3F" is changed to "?" at the end. It think we need to find a way to resolve the issue of using unsafe uri code in camel uri. Willem

            People

            • Assignee:
              Claus Ibsen
              Reporter:
              Alexander Azarov
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development