#20638: Non-anonymous single-hop HS enabled tor doesn't detect already existing anonymous, HS at start-up --------------------------+------------------------------------ Reporter: ahf | Owner: teor Type: defect | Status: needs_revision Priority: Medium | Milestone: Tor: 0.2.9.x-final Component: Core Tor/Tor | Version: Tor: 0.2.9.3-alpha Severity: Normal | Resolution: Keywords: tor-hs, sos | Actual Points: 1.0 Parent ID: | Points: 0.5 Reviewer: | Sponsor: --------------------------+------------------------------------ Changes (by asn):
* status: needs_review => needs_revision Comment: Nice work teor. Patch seems to work for me. The poisoning detection now works without a need for SIGHUP. Some nitpicking around the patch: - I feel that the `else {` block in `rend_service_check_dir_and_add()` is a bit convoluted. For example, I think the `BUG(!s_list)` block can be turned into an assert, to assist with readability. - On the same note, in `rendservice.c` I now see three blocks of: {{{ /* If no special service list is provided, then just use the global one. */ /* ended up with more of this */ if (!service_list) { if (BUG(!rend_service_list)) { return -1; } s_list = rend_service_list; } else { s_list = service_list; } }}} This is one more than we started with (also see comment:4). I wonder, if we can abstract that logic into a `get_hs_service_list()` function that returns the service list if found, otherwise it returns NULL. - Now some comment nitpicking. Double comment here: {{{ if (service->directory != NULL) { /* Skip dupe for ephemeral services. */ /* Skip dupe for ephemeral services. */ }}} Also, what does this comment mean? I think it's one of those with a version string that will rot in the codebase: {{{ /* Ignore service failures until 030 */ }}} - Finally, I think it might be worthwhile to add a small paragraph to the commit msg of `59d525d29` to actually explain '''how''' the patch fixes the bug. I'm turning this into `needs_revision`, please feel free to fix whichever of the comments above you find reasonable. -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/20638#comment:9> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online _______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs