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
>

Reply via email to