Bug 1022069 - Undeploying a web application removes the http request PolicyContextHandler globally
Undeploying a web application removes the http request PolicyContextHandler g...
Product: JBoss Enterprise Application Platform 6
Classification: JBoss
Component: Web (Show other bugs)
Unspecified Unspecified
unspecified Severity unspecified
: ER7
: EAP 6.2.0
Assigned To: Rémy Maucherat
Radim Hatlapatka
Russell Dickenson
Depends On:
  Show dependency treegraph
Reported: 2013-10-22 11:13 EDT by Eric Wittmann
Modified: 2013-12-15 11:13 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2013-12-15 11:13:44 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Eric Wittmann 2013-10-22 11:13:45 EDT
Description of problem:
Undeployment of a web application will remove the PolicyContextHandler that returns the current http request.  This makes sense if it were able to remove the handler just for the deployment unit being undeployed.  However, the handlers are static and global, so removing it has the effect of removing it for all deployment units.

Steps to Reproduce:
See: https://github.com/EricWittmann/eap-bug-1016591

There is a readme there that describes how to reproduce the problem.

Additional info:
This was discovered because JBoss Overlord has a custom login module that uses the PolicyContext to get the current http request.  This login module works fine until *any* web app is undeployed, at which point all Overlord authentication that depends on this login module no longer works.

Reference bug:  https://bugzilla.redhat.com/show_bug.cgi?id=1016591
Comment 1 Eric Wittmann 2013-10-22 11:16:02 EDT
It should be noted that I plan to workaround this problem in Overlord by introducing a Valve (configured in jboss-web.xml) which will do nothing except stash the current Http Request in a thread local variable so that the login module has access to it.

It should work fine, but it would be better of the PolicyContext bug were fixed so that the additional valve were not necessary.
Comment 2 Brian Stansberry 2013-10-22 13:53:54 EDT
This happens in JBossWebRealmService.start()

// Register the active request PolicyContextHandler
HttpServletRequestPolicyContextHandler handler = 
    new HttpServletRequestPolicyContextHandler();
PolicyContext.registerHandler(SecurityConstants.WEB_REQUEST_KEY, handler, true);

and this in stop():

// remove handler
Set handlerKeys = PolicyContext.getHandlerKeys();

The problem is JBossWebRealmService is a service scoped to a deployment. Those calls seem more appropriate for a service scoped to the web container.
Comment 5 Rémy Maucherat 2013-10-23 12:28:21 EDT
So these two things could be moved to WebServerService.start/stop, for example ? I agree they do seem static and detached from the deployment.
Comment 6 Brian Stansberry 2013-10-23 12:52:18 EDT
(In reply to Rémy Maucherat from comment #5)
> So these two things could be moved to WebServerService.start/stop, for
> example ? I agree they do seem static and detached from the deployment.

Sounds good. I don't see any other services that make more sense than that. I looked at the PolicyContext impl and all registerHandler does is do a put in a static map. So I can't see any downside to moving this out of the deployment-scoped service.

Semi-OT -- the stop behavior of PolicyContext.getHandlerKeys().remove(...) is kind of a hack. The PolicyContext.getHandlerKeys() API doesn't say anything about the returned Set being usable for that purpose. There's no other API to accomplish the purpose though. Probably the call should be in a try/catch though so if an impl rejects the remove the failure doesn't propagate. If it was rejected it doesn't really harm anything, since the VM is either shutting down, or in a reload case on the next start call the 'true' param to registerHandler will replace the old handler with the new one.
Comment 7 Rémy Maucherat 2013-10-25 05:48:37 EDT
Comment 8 Dimitris Andreadis 2013-10-28 10:39:00 EDT
Dev-acking on behalf of Rémy (or please shout).
Comment 9 Radim Hatlapatka 2013-11-04 09:25:15 EST
Verified with EAP 6.2.0.ER7

Note You need to log in before you can comment on or make changes to this bug.