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 > > >
