Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Not a Problem
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Estimated Complexity:
      Unknown

      Description

      in tomee/openejb we need to patch the uri builder to be able to satisfied some cases (UriBuilder.fromPath(null) for instance)

      here is the patched builder working for us: http://svn.apache.org/repos/asf/openejb/trunk/openejb/server/openejb-cxf-rs/src/main/java/org/apache/openejb/server/cxf/rs/OpenEJBRuntimeDelegateImpl.java

      any way to get it merged with cxf?

        Activity

        Hide
        Sergey Beryozkin added a comment -

        What exactly is happening in this case ?
        According to
        http://jsr311.java.net/nonav/releases/1.1/javax/ws/rs/core/UriBuilder.html#fromPath(java.lang.String)

        "java.lang.IllegalArgumentException - if path is null"

        Do you not see this exception ?

        Show
        Sergey Beryozkin added a comment - What exactly is happening in this case ? According to http://jsr311.java.net/nonav/releases/1.1/javax/ws/rs/core/UriBuilder.html#fromPath(java.lang.String ) "java.lang.IllegalArgumentException - if path is null" Do you not see this exception ?
        Hide
        Sergey Beryozkin added a comment -

        This test passes for me locally:

        @Test(expected = IllegalArgumentException.class)
            public void testFromNullPath() {
                UriBuilder.fromPath(null);        
                fail();
            }
        
        Show
        Sergey Beryozkin added a comment - This test passes for me locally: @Test(expected = IllegalArgumentException.class) public void testFromNullPath() { UriBuilder.fromPath( null ); fail(); }
        Hide
        Romain Manni-Bucau added a comment -

        well i have not the test right now (will try to have a look a bit later) but this exception is fine.

        using some allowed combination (calling fromPath(null)) you'll get an uri looking to http:/foo:12434/bar instead of http://...

        that's was the issue (and why we patched the builder this way).

        Show
        Romain Manni-Bucau added a comment - well i have not the test right now (will try to have a look a bit later) but this exception is fine. using some allowed combination (calling fromPath(null)) you'll get an uri looking to http:/foo:12434/bar instead of http:// ... that's was the issue (and why we patched the builder this way).
        Hide
        Sergey Beryozkin added a comment -

        OK, please help with reproducing the issues and we'll get the fixes applied asap

        Show
        Sergey Beryozkin added a comment - OK, please help with reproducing the issues and we'll get the fixes applied asap
        Hide
        Romain Manni-Bucau added a comment -

        managed to reproduce the issue simply doing it:

        System.out.println(UriBuilder.fromPath("http://localhost:8080").build());

        it got me: "http:/localhost:8080"

        Show
        Romain Manni-Bucau added a comment - managed to reproduce the issue simply doing it: System.out.println(UriBuilder.fromPath("http://localhost:8080").build()); it got me: "http:/localhost:8080"
        Hide
        Sergey Beryozkin added a comment -

        I've committed the test, see:
        http://svn.apache.org/viewvc?rev=1333527&view=rev

        What is different in your case ?

        Show
        Sergey Beryozkin added a comment - I've committed the test, see: http://svn.apache.org/viewvc?rev=1333527&view=rev What is different in your case ?
        Hide
        Romain Manni-Bucau added a comment -

        yep i didn't reproduce it too, maybe a contextual issue?

        now i have the following:

        UriBuilder.fromUri("http://foo:1234").scheme(null).build().getRawSchemeSpecificPart() != null

        i'll dig further

        Show
        Romain Manni-Bucau added a comment - yep i didn't reproduce it too, maybe a contextual issue? now i have the following: UriBuilder.fromUri("http://foo:1234").scheme(null).build().getRawSchemeSpecificPart() != null i'll dig further
        Hide
        Romain Manni-Bucau added a comment -

        here what i get doing it:

        UriBuilder.fromPath("http://localhost:8080").path("/

        {a}/{b}/{c}/{a}

        ").
        buildFromEncoded("1", "2", "3")

        in org.apache.cxf.jaxrs.impl.UriBuilderImpl#buildPath i get the following paths:
        http:, localhost:8080,

        {a}

        ....(then expected ones)

        so then with the current algorithm it adds only one '/' after http:

        Show
        Romain Manni-Bucau added a comment - here what i get doing it: UriBuilder.fromPath("http://localhost:8080").path("/ {a}/{b}/{c}/{a} "). buildFromEncoded("1", "2", "3") in org.apache.cxf.jaxrs.impl.UriBuilderImpl#buildPath i get the following paths: http:, localhost:8080, {a} ....(then expected ones) so then with the current algorithm it adds only one '/' after http:
        Hide
        Sergey Beryozkin added a comment -

        The following test works for me locally, we have few of tests like this one:

        @Test
            public void testFromEncodedDuplicateVar3() {
                String expected = "http://localhost:8080/1/2/3/1";
                URI uri = UriBuilder.fromPath("http://localhost:8080")
                                    .path("/{a}/{b}/{c}/{a}")
                                    .buildFromEncoded("1", "2", "3");
        
                assertEquals(expected, uri.toString());        
            }
        

        Can you get a breakpoint inside UriBuilderImpl.doPath(...) ? You should hit a block specifically checking if a given path represents a URI and if yes then it gelegates to a uri handler. May be also ping me on IRC ?

        Show
        Sergey Beryozkin added a comment - The following test works for me locally, we have few of tests like this one: @Test public void testFromEncodedDuplicateVar3() { String expected = "http: //localhost:8080/1/2/3/1" ; URI uri = UriBuilder.fromPath( "http: //localhost:8080" ) .path( "/{a}/{b}/{c}/{a}" ) .buildFromEncoded( "1" , "2" , "3" ); assertEquals(expected, uri.toString()); } Can you get a breakpoint inside UriBuilderImpl.doPath(...) ? You should hit a block specifically checking if a given path represents a URI and if yes then it gelegates to a uri handler. May be also ping me on IRC ?
        Hide
        Sergey Beryozkin added a comment -

        > UriBuilder.fromUri("http://foo:1234").scheme(null).build().getRawSchemeSpecificPart() != null

        I don't even know what that is supposed to produce, but UriBuilder does allow it.
        I can imagine using UriBuilder.scheme() in combination with UriBuilder.schemeSpecificPart, when building a uri from scratch.

        What do you think

        UriBuilder.fromUri("http://foo:1234").scheme(null).build()

        should produce ? I'd consider throwing IllegalArgumentException in cases where the scheme and schemeSpecificParts have already been initialized from the existing URI

        Show
        Sergey Beryozkin added a comment - > UriBuilder.fromUri("http://foo:1234").scheme(null).build().getRawSchemeSpecificPart() != null I don't even know what that is supposed to produce, but UriBuilder does allow it. I can imagine using UriBuilder.scheme() in combination with UriBuilder.schemeSpecificPart, when building a uri from scratch. What do you think UriBuilder.fromUri("http://foo:1234").scheme(null).build() should produce ? I'd consider throwing IllegalArgumentException in cases where the scheme and schemeSpecificParts have already been initialized from the existing URI
        Hide
        Romain Manni-Bucau added a comment -

        I think it should produce a getRawSchemeSpecificPart() == null

        @Test
        public void testFromEncodedDuplicateVar3() {
        String expected = "http://localhost:8080/1/2/3/1";
        URI uri = UriBuilder.fromPath("http://localhost:8080")
        .path("/

        {a}/{b}/{c}/{a}

        ")
        .buildFromEncoded("1", "2", "3");

        assertEquals(expected, uri.toString());
        }

        doesn't pass for me. I'm using the snapshot.

        i'm on irc (rmb/rmannibucau) if you want

        Show
        Romain Manni-Bucau added a comment - I think it should produce a getRawSchemeSpecificPart() == null @Test public void testFromEncodedDuplicateVar3() { String expected = "http://localhost:8080/1/2/3/1"; URI uri = UriBuilder.fromPath("http://localhost:8080") .path("/ {a}/{b}/{c}/{a} ") .buildFromEncoded("1", "2", "3"); assertEquals(expected, uri.toString()); } doesn't pass for me. I'm using the snapshot. i'm on irc (rmb/rmannibucau) if you want
        Hide
        Sergey Beryozkin added a comment - - edited

        Lets see how CXF 2.5.x build reacts to http://svn.apache.org/viewvc?rev=1333860&view=rev

        As for the 'UriBuilder.fromUri("http://foo:1234").scheme(null).build().getRawSchemeSpecificPart() != null', I think that setting the scheme to 'null' should only affect the 'http' part from the original URI which would leave the scheme specific part as 'foo:1234'.

        Right now I can see CXF printing "//localhost:8080" in this case which is useless. I think the right outcome should be either "http://foo:1234" where UriBuilder detects that scheme specific part is set but the actual scheme is not and thus defaults to 'http' or throw IllegalArgumentException at scheme(null)

        Show
        Sergey Beryozkin added a comment - - edited Lets see how CXF 2.5.x build reacts to http://svn.apache.org/viewvc?rev=1333860&view=rev As for the 'UriBuilder.fromUri("http://foo:1234").scheme(null).build().getRawSchemeSpecificPart() != null', I think that setting the scheme to 'null' should only affect the 'http' part from the original URI which would leave the scheme specific part as 'foo:1234'. Right now I can see CXF printing "//localhost:8080" in this case which is useless. I think the right outcome should be either "http://foo:1234" where UriBuilder detects that scheme specific part is set but the actual scheme is not and thus defaults to 'http' or throw IllegalArgumentException at scheme(null)
        Hide
        Glen Mazza added a comment -

        Testing with Jersey 1.12:

        testing: System.out.println(UriBuilder.fromUri("http://foo:1234").scheme(null).build().toString())"
        returns //foo:1234

        testing: System.out.println(UriBuilder.fromPath("").replacePath("http://localhost:8080").path("/

        {a}/{b}/{c}/{a}

        ").buildFromEncoded("1", "2", "3").toString());
        returns http://localhost:8080/1/2/3/1

        Show
        Glen Mazza added a comment - Testing with Jersey 1.12: testing: System.out.println(UriBuilder.fromUri("http://foo:1234").scheme(null).build().toString())" returns //foo:1234 testing: System.out.println(UriBuilder.fromPath("").replacePath("http://localhost:8080").path("/ {a}/{b}/{c}/{a} ").buildFromEncoded("1", "2", "3").toString()); returns http://localhost:8080/1/2/3/1
        Hide
        Sergey Beryozkin added a comment -

        Hi Glen, thanks for the help, next time I'll run Jersey tests myself

        So, the way Jersey manages scheme(null) is consistent with the way CXF does it so I guess we can not worry about it.

        Minor update to the way CXF handles UriBuilder.replacePath when the http based URIs are provided is on the way

        Show
        Sergey Beryozkin added a comment - Hi Glen, thanks for the help, next time I'll run Jersey tests myself So, the way Jersey manages scheme(null) is consistent with the way CXF does it so I guess we can not worry about it. Minor update to the way CXF handles UriBuilder.replacePath when the http based URIs are provided is on the way
        Hide
        Romain Manni-Bucau added a comment -

        @Glen what does return jersey for uri.getRawSchemeSpecificPart() when scheme(null) was called?

        Show
        Romain Manni-Bucau added a comment - @Glen what does return jersey for uri.getRawSchemeSpecificPart() when scheme(null) was called?
        Hide
        Sergey Beryozkin added a comment -

        Romain - it would return the same value you see with CXF given that UriBuilder.build produces URI

        Show
        Sergey Beryozkin added a comment - Romain - it would return the same value you see with CXF given that UriBuilder.build produces URI
        Hide
        Glen Mazza added a comment -

        Romain, to do your own testing on Jersey, very simple, just download the Jersey samples (http://maven.java.net/service/local/artifact/maven/redirect?r=releases&g=com.sun.jersey.samples&a=jersey-samples&v=1.12&e=zip&c=project), navigate to the helloworld sample and alter its Main.getBaseURI() method adding in a System.out.println(UriBuilder...whatever you want.toString()) and then run mvn compile exec:java from that sample (as stated in its README). You'll see the results from a terminal window.

        Show
        Glen Mazza added a comment - Romain, to do your own testing on Jersey, very simple, just download the Jersey samples ( http://maven.java.net/service/local/artifact/maven/redirect?r=releases&g=com.sun.jersey.samples&a=jersey-samples&v=1.12&e=zip&c=project ), navigate to the helloworld sample and alter its Main.getBaseURI() method adding in a System.out.println(UriBuilder...whatever you want.toString()) and then run mvn compile exec:java from that sample (as stated in its README). You'll see the results from a terminal window.
        Hide
        Sergey Beryozkin added a comment -

        UriBuilder.replacePath now delegates to the uri handler when it sees absolute http uris

        Show
        Sergey Beryozkin added a comment - UriBuilder.replacePath now delegates to the uri handler when it sees absolute http uris
        Hide
        Romain Manni-Bucau added a comment -

        doesnt work with jersey too for me

        Show
        Romain Manni-Bucau added a comment - doesnt work with jersey too for me
        Hide
        Romain Manni-Bucau added a comment -

        just tried the trunk

        it is really better but i got one issue: UriBuilder.fromPath(null) doesn't throw any exception. It should throw a IllegalArgumentException, no?

        Show
        Romain Manni-Bucau added a comment - just tried the trunk it is really better but i got one issue: UriBuilder.fromPath(null) doesn't throw any exception. It should throw a IllegalArgumentException, no?
        Hide
        Sergey Beryozkin added a comment -

        Hi, I think

        > UriBuilder.fromPath(null) doesn't throw any exception. It should throw a IllegalArgumentException, no?

        Yes - which actually highlights the problem with the API implementation that you use, recall you mentioning
        URIBuilder.fromPath() calls replacePath() [1] on the newly created UriBuilder, where supplying 'null' is perfectly valid.
        Effectively we are talking about the API in question breaking the 'negative' part of the UriBuilder.fromPath contract

        [1] http://docs.oracle.com/javaee/6/api/javax/ws/rs/core/UriBuilder.html#replacePath%28java.lang.String%29

        Show
        Sergey Beryozkin added a comment - Hi, I think > UriBuilder.fromPath(null) doesn't throw any exception. It should throw a IllegalArgumentException, no? Yes - which actually highlights the problem with the API implementation that you use, recall you mentioning URIBuilder.fromPath() calls replacePath() [1] on the newly created UriBuilder, where supplying 'null' is perfectly valid. Effectively we are talking about the API in question breaking the 'negative' part of the UriBuilder.fromPath contract [1] http://docs.oracle.com/javaee/6/api/javax/ws/rs/core/UriBuilder.html#replacePath%28java.lang.String%29
        Hide
        Romain Manni-Bucau added a comment -

        i'll open a geronimo jira

        Show
        Romain Manni-Bucau added a comment - i'll open a geronimo jira
        Hide
        Romain Manni-Bucau added a comment -
        Show
        Romain Manni-Bucau added a comment - opened GERONIMO-6344
        Hide
        Sergey Beryozkin added a comment -

        Hi Romain, ping me please on IRC/email re the info on how to deal with the schema(null) issue

        Show
        Sergey Beryozkin added a comment - Hi Romain, ping me please on IRC/email re the info on how to deal with the schema(null) issue
        Hide
        Sergey Beryozkin added a comment -
        Show
        Sergey Beryozkin added a comment - Romain, please also watch http://java.net/jira/browse/JAX_RS_SPEC-194
        Hide
        Sergey Beryozkin added a comment -

        Hi Romain, we are preparing for 2.6.1 so I'm closing it as Not A problem for now, given UriBuilder implementation is consistent with the RI one. Please feel free to reopen if you can get some confirmation proving otherwise. Would also appreciate if you can update whenever you get a chance in what you have found with respect to setting a schema component to null. cheers

        Show
        Sergey Beryozkin added a comment - Hi Romain, we are preparing for 2.6.1 so I'm closing it as Not A problem for now, given UriBuilder implementation is consistent with the RI one. Please feel free to reopen if you can get some confirmation proving otherwise. Would also appreciate if you can update whenever you get a chance in what you have found with respect to setting a schema component to null. cheers

          People

          • Assignee:
            Sergey Beryozkin
            Reporter:
            Romain Manni-Bucau
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development