Camel
  1. Camel
  2. CAMEL-4954

Camel 2.9.0 incapable of working with % in endpoint URIs

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.9.0
    • Fix Version/s: 2.9.1, 2.10.0
    • Component/s: camel-core
    • Labels:
      None
    • Environment:

      Mac 10.7 Java 1.6.0_29

    • Estimated Complexity:
      Unknown

      Description

      In the class org.apache.camel.util.URISupport which will be used to resolve endpoints (DefaultCamelContext#normalizeEndpointUri) the method parseParameters will be called.
      At first the java.net.Uri#getQuery will be called with according to the javadoc "Returns the decoded query component of this URI" returns a decoded URI. If that fails the java.net.Uri#getSchemeSpecificPart method will be called which according to the javadoc "Returns the decoded scheme-specific part of this URI." returns a decoded URI.
      So to summarize we get in any case a decoded URI.
      This URI will then be than in the method org.apache.camel.util.URISupport#parseQuery(String) again decoded with java.net.URLDecoder#decode(String,String).
      This code leads to the following behaviour:
      If a % is properly encoded with %25test the %25test will be substituted by the first call to %test and the decoded again which leads to an Exception.

      In the http://svn.apache.org/viewvc?view=revision&revision=1166508 commit you can see that the % was uncommented from org.apache.camel.util#UnsafeUriCharactersEncoder. Maybe this is related.

      However... Double encoding of URIs seems quite odd. With any URI char there is no issue with that. But with % the % will be decoded again, which makes a % unusable in Camel.

      1. PercentTest.java
        0.7 kB
        Sebastian Rühl

        Issue Links

          Activity

          Hide
          Sebastian Rühl added a comment -

          @Hadrian I wasn't able to build the trunk by myself (the camel build tips won't help) so I can't test it. But thanks for the tip for camel-jasypt.

          @Willem Im quite not sure of the content of patch 1240189 but could you link to the hudson output (or a place where the test output can be found)?

          Because its reverted shouldn't then this Issue reopened?

          Show
          Sebastian Rühl added a comment - @Hadrian I wasn't able to build the trunk by myself (the camel build tips won't help) so I can't test it. But thanks for the tip for camel-jasypt. @Willem Im quite not sure of the content of patch 1240189 but could you link to the hudson output (or a place where the test output can be found)? Because its reverted shouldn't then this Issue reopened?
          Hide
          Willem Jiang added a comment -

          Revert the patch of 1240189 in camel 2.8.x branch to fix the build.

          Show
          Willem Jiang added a comment - Revert the patch of 1240189 in camel 2.8.x branch to fix the build.
          Hide
          Willem Jiang added a comment -

          @Hadrian,

          A lot of test failed because of patch merging into the Camel 2.8.x.
          Can you take a look at it ?

          Show
          Willem Jiang added a comment - @Hadrian, A lot of test failed because of patch merging into the Camel 2.8.x. Can you take a look at it ?
          Hide
          Hadrian Zbarcea added a comment -

          @Sebastian, I committed a fix, actually more like a workaround on trunk. It took a while because I had to run the full tests to make sure I didn't break something. A better fix is possible and get rid of the double decode, but that only after we fix all components to not rely on unsafe URIs and then we can remove the double decode and more importantly the normalize.

          It should work for you now. It would be great if you could test it from trunk. I will merge to 2.9.1 in a few hours.

          Show
          Hadrian Zbarcea added a comment - @Sebastian, I committed a fix, actually more like a workaround on trunk. It took a while because I had to run the full tests to make sure I didn't break something. A better fix is possible and get rid of the double decode, but that only after we fix all components to not rely on unsafe URIs and then we can remove the double decode and more importantly the normalize. It should work for you now. It would be great if you could test it from trunk. I will merge to 2.9.1 in a few hours.
          Hide
          Hadrian Zbarcea added a comment -

          Thanks a lot for reporting this and especially for providing a unit test. That is a problem I will fix today. In general however, the aim is to get rid of the normalizeEndpointUri. It is used because the uri design for some components allows the creation of invalid URIs that cannot be passed, so using it was a workaround in the first place (from almost the very beginning). The endpoint creation is quite convoluted and is due for some refactoring anyway, but I thought it could wait until 3.0.

          The goal for 3.0 is to ensure that all components only accept valid URIs, so it's the responsibility of the user to encode whatever needs encoding, as it the case with any other technology the uses URIs. There is a new method a component can override now, preProcessUri(String) that would convert (and log) the original invalid Uri into a valid one, which could be used instead of the original one. This should also help with migration.

          On a side note, using clear passwords in the uri is not the most secure thing to do. You may want to take a look at camel-jasypt. It may provide a different, safer, workaround.

          Show
          Hadrian Zbarcea added a comment - Thanks a lot for reporting this and especially for providing a unit test. That is a problem I will fix today. In general however, the aim is to get rid of the normalizeEndpointUri. It is used because the uri design for some components allows the creation of invalid URIs that cannot be passed, so using it was a workaround in the first place (from almost the very beginning). The endpoint creation is quite convoluted and is due for some refactoring anyway, but I thought it could wait until 3.0. The goal for 3.0 is to ensure that all components only accept valid URIs, so it's the responsibility of the user to encode whatever needs encoding, as it the case with any other technology the uses URIs. There is a new method a component can override now, preProcessUri(String) that would convert (and log) the original invalid Uri into a valid one, which could be used instead of the original one. This should also help with migration. On a side note, using clear passwords in the uri is not the most secure thing to do. You may want to take a look at camel-jasypt. It may provide a different, safer, workaround.
          Hide
          Sebastian Rühl added a comment -

          Attached jUnit test which demonstrates the issue.

          Show
          Sebastian Rühl added a comment - Attached jUnit test which demonstrates the issue.
          Hide
          Sebastian Rühl added a comment -

          FYI %2525test leads to the excepted behavior (%test).

          Show
          Sebastian Rühl added a comment - FYI %2525test leads to the excepted behavior (%test).
          Hide
          Sebastian Rühl added a comment -

          Maybe this issue is a side effect

          Show
          Sebastian Rühl added a comment - Maybe this issue is a side effect

            People

            • Assignee:
              Hadrian Zbarcea
              Reporter:
              Sebastian Rühl
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development