reta commented on a change in pull request #703:
URL: https://github.com/apache/cxf/pull/703#discussion_r499180730
##########
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 was debugging this failure and it seems like the change with respect
to Request URI
```
HttpUtils.resetRequestURI(m, requestUri.toString());
```
violates some CXF flow (it does not mean the flow is correct, it just
probably means we need changes in other places).
In the nutshell, my findings are as such:
- CXF assumes the `m.get(Message.REQUEST_URI)` has only URI raw path
component
- CXF assumes the `m.get(Message.QUERY_STRING)` has only URI query string
component
The fact that we now set the full URI (with a query string) breaks the way
CXF treats the `m.get(Message.REQUEST_URI)` and test fails because as you
correctly pointing out, the query string is messed up
`/bookstore/books/check2?a=b?a=b`.
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.
----------------------------------------------------------------
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]
