[tor-bugs] #14846 [Tor]: Controller: retrieve an HS descriptor of a service run by a user

[Tor Bug Tracker & Wiki]

#14846: Controller: retrieve an HS descriptor of a service run by a user
-------------------------+-------------------------------------------------
     Reporter:  dgoulet  |      Owner:  donncha
         Type:           |     Status:  needs_revision
  enhancement            |  Milestone:  Tor: 0.2.8.x-final
     Priority:  trivial  |    Version:  Tor: 0.2.7
    Component:  Tor      |   Keywords:  SponsorR, tor-hs, controller,
   Resolution:           |  027-triaged-1-in, SponsorS, TorCoreTeam201509
Actual Points:           |  Parent ID:  #3521
       Points:  small    |
-------------------------+-------------------------------------------------
Changes (by dgoulet):

 * status:  needs_review => needs_revision


Comment:

 Replying to [comment:26 special]:
 > I think the GETINFO should be consistent with the existing
 `hs/client/desc/id/<ADDR>` option. How about `hs/service/desc/id/<ADDR>`?

 Agree with special here.

 >
 > Trivial typo in the changelog (for -> from).
 >
 > Which replica of the descriptor you get is an implementation detail of
 the code, and the controller can't get both. Does that matter? I'm
 guessing no.

 Code suggest that you'll get both actually. See
 `control_event_hs_descriptor_created()`, the replica is outputed in the
 control message.

 >
 > From a read through of the code, this looks good to me.

 Any chance we can modify control-spec.txt also as we merge this feature? I
 see a branch for this ticket in your github but pointing it here
 explicitely with the latest branch ready for merge would be good once we
 make the change above.

 In `rend_cache_clean()`, I see this being added with static for which I
 don't think it's desired here, you just want a temporary "cache pointer"
 for the lifetime of the function.

 {{{
 +  static strmap_t *cache = NULL;
 }}}

 The side effect here would be that it's initialize to NULL *once* and then
 the if/else after sets it to whatever cache we need. The next call to that
 function, `cache` will point to the latest assignement and *NOT* be
 initialized to NULL thus always passing the assert() test after even if
 the cache type is wrong.

 The rest lgtm!

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