andymc12 commented on a change in pull request #703:
URL: https://github.com/apache/cxf/pull/703#discussion_r499305938
##########
File path:
systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/BookServer20.java
##########
@@ -227,7 +227,7 @@ public void filter(ContainerRequestContext context) throws
IOException {
} else if (path.endsWith("books/check2")) {
replaceStream(context);
} else if (path.endsWith("books/checkNQuery")) {
- URI requestURI = URI.create(path.replace("NQuery", "2?a=b"));
+ URI requestURI = URI.create(path.replace("NQuery", "2"));
Review comment:
> I think although your change does fix the test, the flow is broken by
and large, please correct me if I am wrong. The user should be able to change
the Request URI in pre-matching filter, including the query string component,
but she won't be able to do that from now on.
Hmm... maybe I don't fully understand. I think the test was broken from the
start since it was effectively setting the RequestUri to
`/bookstore/books/check2?a=b?a=b` - the previous behavior (prior to #697) was
to simply ignore the query string entirely which was why the test passed even
though the URI was invalid. After #697 the test failed because we were
actually checking the query string.
Do you think we need to explicitly set `Message.QUERY_STRING` when the user
calls `containerRequestContext.setRequestUri(newUri)`? In that case then we
might also need to do some checking and "URI merging" in other places where
JAX-RS related code is expecting to see the full URI (including the query
string) - which is the point you were making, I think.
Do you know which places in the code are expecting `Message.REQUEST_URI` to
be the raw URI? I'd like to take a look to determine whether we should change
those parts to expect REQUEST_URI to be the full URI or whether we should
change the JAX-RS parts to merge the REQUEST_URI with the QUERY_STRING when
checked.
Thanks for the review and the analysis.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
