[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #19757 [Applications/Tor Browser]: Make a menu to add onion and auth-cookie to TB

#19757: Make a menu to add onion and auth-cookie to TB
-------------------------------------------------+-------------------------
 Reporter:  mrphs                                |          Owner:  brade
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  Medium                               |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  ux-team, tbb-usability, tor-hs,      |  Actual Points:  5
  TorBrowserTeam202001R                          |
Parent ID:  #30000                               |         Points:  8
 Reviewer:                                       |        Sponsor:
                                                 |  Sponsor27-must
-------------------------------------------------+-------------------------

Comment (by pospeselr):

 > https://gitweb.torproject.org/user/brade/tor-
 browser.git/tree/browser/components/onionservices/content/savedKeysDialog.js?h=bug19757-01&id=fb12d169bfe97b5a71a9135ad1efe25d39a1c097#n56
 >{{{
 >// Remove in reverse index order to avoid issues caused by index changes.
 >for (let i = indexesToDelete.length - 1; i >= 0; --i) {
 >  await this._deleteOneKey(torController, indexesToDelete[i]);
 >}
 >}}}

 This feels a bit fishy to me. We're saving off a list of indices and then
 individually removing them from both {{{this._keyInfoList}}} and
 {{{this._tree}}}. This happens in an async context, so wouldn't it be
 possible for the indices to become invalidated if keys were to be added in
 the middle of a large batch of deletions?

 I *think* a better way to do this would be to first create a list of the
 hsAdresses we want to remove,  do our synchronous work updating
 {{{this._keyInfoList}}} and {{{this._tree}}}, then call
 {{{onionAuthRemove}}} with all of our hsAddresses (assuming they can't all
 be batched together into one call to tor).

 Alternatively, we could call {{{loadSavedKeys()}}} after a multi-delete to
 ensure the UI matches the tor backing store.

 Apart from that, these changes look good to me.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19757#comment:25>
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