[tbb-bugs] #16624 [Applications/Tor Browser]: Improper key passed to nsHttpChannel::DoInvalidateCacheEntry()?

[Tor Bug Tracker & Wiki]

#16624: Improper key passed to nsHttpChannel::DoInvalidateCacheEntry()?
--------------------------------------+--------------------------
 Reporter:  mikeperry                 |          Owner:  tbb-team
     Type:  task                      |         Status:  new
 Priority:  Medium                    |      Milestone:
Component:  Applications/Tor Browser  |        Version:
 Severity:  Normal                    |     Resolution:
 Keywords:  tbb-mozilla-merge         |  Actual Points:
Parent ID:                            |         Points:
 Reviewer:                            |        Sponsor:
--------------------------------------+--------------------------
Description changed by arthuredelstein:

Old description:

> During the cache2 review in #13035, mcs noticed that an empty key was
> being passed to nsHttpChannel::DoInvalidateCacheEntry().
>
> {{{
> nsHttpChannel::DoInvalidateCacheEntry() to use our modified (isolated)
> cache keys. That would involve passing a non-empty string as the second
> parameter to cacheStorage->AsyncDoomURI() within that method. This is not
> new code and not something we patched in the past... and Kathy and I do
> not understand the implications of not patching it. But it seems like the
> wrong key is being used there.
> }}}
>
> I replied:
> {{{
> I have not dug through all of the eviction code (there sure are a lot of
> codepaths involved there), but my initial take is that since Mozilla has
> been using this same extension key to isolate caching for POST requests,
> it probably is not a serious issue to omit it, since the original code
> would have been experiencing similar problems even before our isolation
> made further use of this key...
> }}}
>
> We should ask Mozilla for an opinion. This may be a bug in their code,
> too.

New description:

 During the cache2 review in #13035, mcs noticed that an empty key was
 being passed to nsHttpChannel::DoInvalidateCacheEntry().
 > nsHttpChannel::DoInvalidateCacheEntry() to use our modified (isolated)
 cache keys. That would involve passing a non-empty string as the second
 parameter to cacheStorage->AsyncDoomURI() within that method. This is not
 new code and not something we patched in the past... and Kathy and I do
 not understand the implications of not patching it. But it seems like the
 wrong key is being used there.

 I replied:
 > I have not dug through all of the eviction code (there sure are a lot of
 codepaths involved there), but my initial take is that since Mozilla has
 been using this same extension key to isolate caching for POST requests,
 it probably is not a serious issue to omit it, since the original code
 would have been experiencing similar problems even before our isolation
 made further use of this key...


 We should ask Mozilla for an opinion. This may be a bug in their code,
 too.

--

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/16624#comment:5>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online