ffang commented on code in PR #1822:
URL: https://github.com/apache/cxf/pull/1822#discussion_r1572882731
##########
rt/transports/http/src/main/java/org/apache/cxf/transport/http/AbstractHTTPDestination.java:
##########
@@ -410,8 +410,12 @@ private boolean isWSAddressingReplyToSpecified(Exchange
ex) {
public Principal getUserPrincipal() {
try {
return req.getUserPrincipal();
- } catch (Exception ex) {
- return null;
+ } catch (Exception e) {
Review Comment:
Hi Guys,
I think I figured out the root cause of the NPE.
This will happen in case of oneway or ReplyTo is specified when
ws-addressing is used, which means we need to switch thread context on the
server side. Basically it's pause the current interceptor chain and resume it
in another thread, and before the resumed current interceptor chain is
completed, the underlying transport might discard any data on the original
stream, that's why we need to cache the InputStream in this case and use it
later if necessary. And this peace of code is in
org.apache.cxf.ws.addressing.impl.InternalContextUtils.rebaseResponse.
Specifically we have
```
if (ContextUtils.retrieveAsyncPostResponseDispatch(inMessage)
&& !robust) {
//need to suck in all the data from the input stream
as
//the transport might discard any data on the stream
when this
//thread unwinds or when the empty response is sent
back
DelegatingInputStream in =
inMessage.getContent(DelegatingInputStream.class);
if (in != null) {
in.cacheInput();
}
}
```
Which in turn will call into
org.apache.cxf.transport.http.AbstractHTTPDestination.setupMessage
```
DelegatingInputStream in = new
DelegatingInputStream(req.getInputStream()) {
public void cacheInput() {
if (!cached && (exchange.isOneWay() ||
isWSAddressingReplyToSpecified(exchange))) {
//For one-ways and WS-Addressing invocations with
ReplyTo address,
//we need to cache the values of the HttpServletRequest
//so they can be queried later for things like paths and
schemes
//and such like that.
//Please note, exchange used to always get the "current"
message
exchange.getInMessage().put(HTTP_REQUEST, new
HttpServletRequestSnapshot(req));
}
if (req.getContentLengthLong() != 0) {
super.cacheInput();
}
}
private boolean isWSAddressingReplyToSpecified(Exchange ex) {
AddressingProperties map =
ContextUtils.retrieveMAPs(ex.getInMessage(), false, false, false);
return map != null &&
!ContextUtils.isGenericAddress(map.getReplyTo());
}
};
```
So in this case the HttpServletRequestSnapshot which is a wrapper of
original HttpServletRequest is saved in InMessage, and we should always use
pre-saved HTTP_REQUEST from InMessage to populate SecurityContext, as pre-saved
HTTP_REQUEST from InMessage could be the original HttpServletRequest or the
Cached HttpServletRequest when it's necessary.
We don't see this change is necessary for Jetty 11 and before.
Just send a PR https://github.com/apache/cxf/pull/1823 to address this for
Jetty12.
Thanks and Best Regards
Freeman
--
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.
To unsubscribe, e-mail: dev-unsubscr...@cxf.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
