[opensource-dev] Review Request: VWR-24333: Hardening against use of getLindenUserDir() before logging in.

Merov Linden merov at lindenlab.com
Tue Jan 18 11:39:05 PST 2011


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/91/#review190
-----------------------------------------------------------

Ship it!


I haven't search for all instances of getLindenUserDir() in the whole code but, reading the diff, this all looks good to me.

- Merov


On Jan. 14, 2011, 1:05 p.m., Aleric Inglewood wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/91/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2011, 1:05 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> Without this patch, getLindenUserDir() is sometimes used without
> checking if it returns an empty value, resulting in trying to open
> an empty file and then gracefully recovering from that. So, this
> patch doesn't really fix a bug. However it might prevent one in the
> future. Note that this DID lead to a bug in Viewer 1 code, so it's
> possible. The main threat is that some singleton class that uses
> getLindenUserDir (indirectly) is instantiated before logging in:
> A singleton is only created once and when it is initialized with
> an empty getLindenUserDir() then that can remain.
> 
> This patch aborts when the viewer is compiled in debug mode (not
> in release mode, when it will do what it did before this patch)
> and when getLindenUserDir() is called before it was initialized,
> unless the developer explicitely passes 'true' (empty_ok) to
> getLindenUserDir, signaling that they considered the possibility
> that the function is being called before the user logged in.
> 
> 
> This addresses bug VWR-24333.
>     http://jira.secondlife.com/browse/VWR-24333
> 
> 
> Diffs
> -----
> 
>   indra/llvfs/lldir.h 422f636c3343 
>   indra/llvfs/lldir.cpp 422f636c3343 
>   indra/newview/llappviewer.cpp 422f636c3343 
>   indra/newview/llbottomtray.cpp 422f636c3343 
>   indra/newview/llfilepicker.cpp 422f636c3343 
>   indra/newview/llnavigationbar.cpp 422f636c3343 
>   indra/newview/llsearchhistory.h 422f636c3343 
>   indra/newview/llurlhistory.cpp 422f636c3343 
>   indra/newview/llviewerinventory.cpp 422f636c3343 
>   indra/newview/llviewermedia.cpp 422f636c3343 
>   indra/newview/llvoiceclient.cpp 422f636c3343 
> 
> Diff: http://codereview.secondlife.com/r/91/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Aleric
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110118/e7bf778a/attachment.htm 


More information about the opensource-dev mailing list