FYI - I've sent over a request for clarification on the spec: https://www.eclipse.org/lists/jaxrs-dev/msg00773.html
Jon On Fri, Jan 24, 2020 at 9:58 AM Jonathan Gallimore < [email protected]> wrote: > Hi Romain > > Thanks for the reply. That sounds reasonable - I'll raise it on the list > for the spec to seek clarification, and add assertions to the TCK if > appropriate. I had considered a check for "file" as the scheme (in fact > that was my first attempt :-) ), but was worried it maybe wasn't generic > enough. I can certainly submit a PR with that change and a test case. > > Cheers > > Jon > > On Thu, Jan 23, 2020 at 9:47 PM Romain Manni-Bucau <[email protected]> > wrote: > >> Hi Jon >> >> Sounds like the fix respects the spec now, >> >> https://docs.oracle.com/javaee/7/api/javax/ws/rs/core/UriBuilder.html#schemeSpecificPart-java.lang.String- >> implies there is no path so >> >> https://docs.oracle.com/javaee/7/api/javax/ws/rs/core/UriBuilder.html#path-java.lang.String- >> is a noop even if unnatural. >> >> Can request a spec clarification and tck challenge IMHO to let cxf fix the >> buildUriString and setUriParts methods. >> >> A likely trivial way to fix is to test file:// protocol, tck will pass and >> behavior will be more natural IMHO. >> >> Le jeu. 23 janv. 2020 à 21:59, Jonathan Gallimore < >> [email protected]> a écrit : >> >> > Hi >> > >> > I'm looking into an issue where appending a path to file:// URI with >> > UriBuilder doesn't work - this has been flagged up in TomEE 8.0.1 which >> is >> > using CXF 3.3.4, but works ok with TomEE 7.1.1 (which uses CXF 3.1.18). >> > >> > In essence, this fails: >> > >> > >> UriBuilder.fromUri("file:///~/calendar").path("extra").build().toString() - >> > the result is file:///~/calendar. without the "/extra" on the end. This >> is >> > because the schemeSpecificPart field is set in this if block: >> > >> > >> https://github.com/apache/cxf/blob/e6ec5b785e67bbd2464796119a9cdcacf59598c8/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java#L642-L645 >> > and >> > is used when building the URI, so the appended path is lost. >> > >> > I've attempted to patch this, by adding a check for a path as well as a >> > query in this if block: >> > >> > diff --git >> > >> > >> a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java >> > >> > >> b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java >> > index 8de34fbb0d..26e7ca357d 100644 >> > --- >> > >> > >> a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java >> > +++ >> > >> > >> b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/UriBuilderImpl.java >> > @@ -651,8 +651,9 @@ public class UriBuilderImpl extends UriBuilder >> > implements Cloneable { >> > } else { >> > schemeSpecificPart = uri.getSchemeSpecificPart(); >> > } >> > - if (scheme != null && host == null && (query == null || >> > query.isEmpty()) && userInfo == null >> > - && uri.getSchemeSpecificPart() != null) { >> > + if (scheme != null && host == null >> > + && queryEmpty() && pathEmpty() && userInfo == null >> > + && uri.getSchemeSpecificPart() != null) { >> > schemeSpecificPart = uri.getSchemeSpecificPart(); >> > } >> > String theFragment = uri.getFragment(); >> > @@ -661,6 +662,14 @@ public class UriBuilderImpl extends UriBuilder >> > implements Cloneable { >> > } >> > } >> > >> > + private boolean queryEmpty() { >> > + return query == null || query.isEmpty(); >> > + } >> > + >> > + private boolean pathEmpty() { >> > + return paths == null || paths.isEmpty(); >> > + } >> > + >> > >> > This fixes the issue with the file URI above, but causes this test >> failure: >> > >> > [ERROR] UriBuilderImplTest.testURItoStringMatchesOriginalURI:1631 >> > expected:<myscheme://[not.really.a.host:fakePort]/> but >> > was:<myscheme://[]/> >> > >> > I note that this check was added in this commit: >> > >> > >> https://github.com/apache/cxf/commit/e6ec5b785e67bbd2464796119a9cdcacf59598c8 >> > , >> > and that references some TCK tests. Is the fakePort scenario necessary >> to >> > pass the TCK, or is there other rationale for that case? >> > >> > I'm very happy to work on a PR to fix along with tests, but would >> > appreciate a little guidance if there's a specific approach to take. >> I'll >> > keep looking to find a way to handle the failing assert I mention above, >> > but if I've missed something blindingly obvious, please do let me know! >> > >> > Many thanks >> > >> > Jon >> > >> >
