shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Les Hazlewood" <...@hazlewood.com>
Subject Re: ***UNCHECKED*** Testcase for NPE problem in JSecurity
Date Thu, 04 Sep 2008 16:08:43 GMT
Hi Emmanuel,
I agree - this particular end-user messaged Peter and myself directly, and
although I almost always do what you recommend, for some reason I let this
one slip ;)  But yes, I'll try to remember that in the future.

I think we're very close to 0.9 final - just this issue that this user
reported and one other
(http://issues.jsecurity.org/browse/JSEC-125)<http://issues.jsecurity.org/browse/JSEC-125>.
 Once those are done, and 0.9 final is released, we can migrate over to the
ASF SVN asap.  Probably in a week or two at the most?

On Thu, Sep 4, 2008 at 11:57 AM, Emmanuel Lecharny <elecharny@gmail.com>wrote:

> Hi Les,
>
> thanks for the feedback.
>
> At some point, it would be better if we can redirect users having problems
> to JIRA, as you can attach stack traces, logs, code, etc., and have a better
> tracking system.
>
> Btw, any estimate about when the code will be moved to ASF svn ?
>
> Thanks !
>
> Les Hazlewood wrote:
>
>> ------- Begin copy from previous emails so it can be archived on the dev
>> list -------------
>> Hello Les,
>> Hello Peter (I put you on cc in case you are interested),
>>
>> I extracted the test-case. Please find attached the war-file.
>> It uses the in-memory DB so you can just throw it into you
>> tomcat web-app.
>>
>> In our enviroment we can reproduce the problem as follow:
>>
>>  - Tomcat 6.0.x  (on a Linux-System (tested with Debian and Gentoo), on
>> windows we can't reproduce)
>>  - Firefox 2     (Firefox 3 doesn't seem to get much of these NPEs)
>>  - You can login with the user "mos"  (password is also "mos")
>>  - Hit the "EDIT" link for username oder email field
>>  - Edit something and press "SAVE"
>>  - If the bug occur the app sent a 500 error and the page doesn't refresh
>> (progress bar stay on the page)
>>
>> We log to stdout, so you can find some information in the catelina.out for
>> example.
>> Our JSECURITY-Logging looks like this:
>>
>> ---
>> [10:43:02] [DEBUG] [http-8080-2] [?:?] Principal: 'KeUser#1 [username:
>> mos]'. JSESSIONID-Cookie-Value:
>> ["3616C0699338213544A383573602A896"]
>> Session values:
>> org.jsecurity.web.session.WebSession_INET_ADDRESS_SESSION_KEY=/127.0.0.1
>> org.codehaus.groovy.grails.FLASH_SCOPE=[:]
>>
>> org.jsecurity.web.DefaultWebSecurityManager_PRINCIPALS_SESSION_KEY=[KeUser#1
>> [username: mos]]
>> org.jsecurity.web.DefaultWebSecurityManager_AUTHENTICATED_SESSION_KEY=true
>> ---
>>
>> The 'KeUser#1 [username: mos]' is a call to
>> 'SecurityUtils.getSubject()?.principal'.
>> The other information are the cookie value and the session attributes.
>>
>> In case of an error you see the following:
>>
>> ---
>> [10:43:02] [DEBUG] [http-8080-1] [?:?] Principal: 'null'.
>> JSESSIONID-Cookie-Value: ["3616C0699338213544A383573602A896"]
>> Session values:
>> org.jsecurity.web.session.WebSession_INET_ADDRESS_SESSION_KEY=/127.0.0.1
>> org.codehaus.groovy.grails.FLASH_SCOPE=[:]
>>
>> org.jsecurity.web.DefaultWebSecurityManager_PRINCIPALS_SESSION_KEY=[KeUser#1
>> [username: mos]]
>> org.jsecurity.web.DefaultWebSecurityManager_AUTHENTICATED_SESSION_KEY=true
>> [10:43:02] [ERROR] [http-8080-1] [GrailsExceptionResolver.java:55]
>> java.lang.NullPointerException
>>       at
>>
>> JsecurityGrailsPlugin.accessControlMethod(JsecurityGrailsPlugin.groovy:377)
>> ---
>>
>> Note:  The session information is valid and complete, but the JSecurity
>> subject is 'null'.
>> Could it be somehow related to the "Thread binding" of the subject?
>>
>> Thanks a lot for your support
>>
>> If you have any direct questions or hints, you are very welcome to contact
>> me via Skype:  "mo.scheele"
>>
>> Cheers
>> mos
>>
>> Peter's response:
>>
>> OK, try the attached JAR file. You just need to replace the existing
>> one in "WEB-INF/lib". If it works, the problem is down to
>> ThreadContext using an InheritableThreadLocal rather than a standard
>> ThreadLocal. I don't know whether the InheritableThreadLocal is
>> important, but I suspect it's not safe in an environment where one
>> doesn't have control over the threads, for example in a servlet
>> container.
>>
>> ------- End copy from previous emails so it can be archived on the dev
>> list
>> -------------
>>
>> On Thu, Sep 4, 2008 at 11:02 AM, Peter Ledbrook <peter@g2one.com> wrote:
>>
>>
>>
>>> How did you narrow it down to that?  I'm just curious - could be useful
>>>>
>>>>
>>> in
>>>
>>>
>>>> my own debugging ;)
>>>>
>>>>
>>> Breakpoint in JSecurityFilters where it binds the security manager to
>>> the thread. I noticed that if there was a long enough pause, I was
>>> getting exceptions in other threads due to the ServletRequest not
>>> being bound to the thread. It seemed that as soon as execution
>>> switched to a different thread, the exception was thrown.
>>>
>>>
>>
>> I could
>>
>>
>>> never work out what was really going on, but I suspected that the
>>> request was being unbound on one thread before it was being accessed
>>> on another. Anyway, I took a wild guess at the ThreadLocal and tried
>>> it with the parent class.
>>>
>>>
>>>
>>>> Also, I'm (somewhat) certain InheritableThreadLocal is not an invalid
>>>> choice, especially in security scenarios - if the parent thread has
>>>>
>>>>
>>> security
>>>
>>>
>>>> data bound to it, any child threads it spawns should also have the same
>>>> security data.  But there might be a problem if any of the child threads
>>>> modify the security data in any way - or if either the parent or child
>>>> thread clears out the ThreadContext before the others are done using it.
>>>> But according to the Servlet spec and JEE specs, requests and
>>>>
>>>>
>>> transactions
>>>
>>>
>>>> (respectively) have a 1:1 correspondence with a single thread, so this
>>>> parent/child thread corruption should never happen.
>>>>
>>>>
>>> I agree. I can't see why this would be a problem unless the
>>> ThreadContext is being manipulated by the parent of all the
>>> request-handling threads. Even then, it seems to me that only the
>>> initial values should be affected - once a request thread binds new
>>> values, it should be independent of any of the others. I don't get it,
>>> but the change seems to work for me.
>>>
>>>
>>
>>
>> Absolutely bizarre.  I have a strong suspicion this has to do with
>> Tomcat's
>> thread pool behavior - maybe a single request thread is created, and
>> others
>> are spawned based off of it for the pool.  Maybe the parent and its
>> children
>> are in the same pool - if so, and either the parent or child(ren) call
>> ThreadContext.clear()  or ThreadContext.remove*, it would have an adverse
>> effect on either the parent or child(ren).  Again, this would only be the
>> case if my assumptions about the thread pool are correct.
>>
>> In any event, a few months ago, I changed from ThreadLocal to
>> InheritableThreadLocal with the assumption that it was a good idea for
>> security systems as I described above.  If there are parent/child issues,
>> any child should receive a clone of the parent, ensuring modification of
>> the
>> child's Map state does not affect the parent's and vice versa.
>>
>> I've made that change to ThreadContext and just committed it.  Peter,
>> could
>> you please update and test it out to ensure that it is a valid solution (I
>> still have deployment problems with that .war and can't test it).  Also
>> MO,
>> if you check out jsecurity's trunk, you could build and deploy the created
>> jsecurity-jdk14.jar file and see yourself.
>>
>> Please let me know what happens.
>>
>> Thanks,
>>
>> Les
>>
>>
>> By the way, the web app doesn't throw an NPE - it simply logs the fact
>>
>>
>>> that it would have done without an extra check added by mos :)
>>>
>>>
>>>
>>>> I'm also curious why you recommended trying the jsecurity-jdk14.jar -
>>>> JDK
>>>> 1.4.2 has both the ThreadContext and InheritableThreadContext classes,
>>>> so
>>>> there shouldn't be any difference between that .jar and jsecurity.jar
>>>> (1.5+).
>>>>
>>>>
>>> The plugin is packaged with the JDK1.4 version of the library, i.e.
>>> it's a like-for-like replacement. Nothing to do with potential
>>> differences between the JAR files.
>>>
>>>
>>
>>
>> Ah, ok, cool.  Thanks for the clarification.
>>
>>
>>
>>> Cheers,
>>>
>>> Peter
>>>
>>> --
>>> Software Engineer
>>> G2One, Inc.
>>> http://www.g2one.com/
>>>
>>>
>>>
>>
>>
>>
>
>
> --
> --
> cordialement, regards,
> Emmanuel L├ęcharny
> www.iktek.com
> directory.apache.org
>
>
>
Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message