[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #13252 [Tor Browser]: Tor Browser on OS X should not store data into the application bundle
#13252: Tor Browser on OS X should not store data into the application bundle
-----------------------------------+--------------------------------
Reporter: torosx | Owner: mcs
Type: defect | Status: needs_revision
Priority: Medium | Milestone:
Component: Tor Browser | Version:
Severity: Normal | Resolution:
Keywords: TorBrowserTeam201603R | Actual Points:
Parent ID: #6540 | Points:
Reviewer: | Sponsor: None
-----------------------------------+--------------------------------
Changes (by mcs):
* status: needs_review => needs_revision
Comment:
Replying to [comment:57 arthuredelstein]:
> Replying to [comment:55 mcs]:
>
> `xpcom/io/TorFileUtils.cpp`:
>
> {{{
> + // When crawling up the hierarchy, components named "." do not count.
> }}}
> Is this a bug in Mozilla's nsILocalFile::GetParent implementation?
You could argue that it is, but the answer may depend on your point of
view. Kathy and I commonly encounter dots in the executable path because
we start from the command line a lot when debugging; most people are not
affected by this kind of issue. The code in TorFileUtils.cpp is not new
but relocated from elsewhere.
> When `useOSLocation`==false, you actively create the "TorBrowser-Data"
directory:
> {{{
> + rv = tbDataDir->AppendNative(tbDataLeafName);
> + NS_ENSURE_SUCCESS(rv, rv);
> + bool exists = false;
> + rv = tbDataDir->Exists(&exists);
> + if (NS_SUCCEEDED(rv) && !exists)
> + rv = tbDataDir->Create(nsIFile::DIRECTORY_TYPE, 0700);
> }}}
> but it looks like you don't create it when `useOSLocation`==true.
You are correct. Firefox will create that directory; the only reason we
try to create TorBrowser-Data when useOSLocation==false is to find out
whether we can. We will add a comment to explain this.
> On a possibly related issue, I found the following line confusing:
> {{{
> + nsresult rv = NS_NewNativeLocalFile(EmptyCString(), true,
> + getter_AddRefs(tbDataDir));
> }}}
> I guess this is just for checking that it is possible to write to the
parent directory where the "TorBrowser-Data" directory will be? A comment
here might help.
We will add a comment. That code is the same as some existing Mozilla code
that is nearby. The call to NS_NewNativeLocalFile() gives us an nsIFile
object whose path is soon after set to point to the correct location by
the calls to InitWithFSRef() and AppendNative().
> `/toolkit/xre/nsXREDirProvider.cpp`:
>
> {{{
> + // Since the TorBrowser-Data directory may be shared among different
> + // installations of the application, embed the app path in the update
dir
> + // so that the update history is partitioned.
> }}}
> Is this potentially true for Linux or Windows as well? If I install `Tor
Browser (FR)` and `Tor Browser (JA)` in the same directory, for example.
Perhaps that's a pretty unlikely scenario, though.
It is possible but much less likely on Linux or Windows. When we switch to
the "side-by-side" approach on those platforms, the TorBrowser-Data
directory will be created inside the directory that contains the Browser
directory and startup script/Windows shortcut. Because our packaging
already includes that top level "container" directory (which was not true
on Mac OS), Kathy and I do not think people will install another copy of
Tor Browser inside the same directory.
> Apart from these issues, this patch looks OK to me.
Thanks for your review! We will post a new patch with the comment changes
you suggested.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/13252#comment:58>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs