[tor-bugs] #10481 [Tor]: connection_mark_unattached_ap_: checking always true edge_has_sent_end

[Tor Bug Tracker & Wiki]

#10481: connection_mark_unattached_ap_:  checking always true edge_has_sent_end
-----------------------------+------------------------------------
     Reporter:  cypherpunks  |      Owner:
         Type:  defect       |     Status:  new
     Priority:  normal       |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor          |    Version:
   Resolution:               |   Keywords:  tor-client 025-triaged
Actual Points:               |  Parent ID:
       Points:               |
-----------------------------+------------------------------------

Comment (by andrea):

 This code's an icky mess which comes with a comment to that effect and
 telling us to make it suck less:

 {{{
 /** An AP stream has failed/finished. If it hasn't already sent back
  * a socks reply, send one now (based on endreason). Also set
  * has_sent_end to 1, and mark the conn.
  */
 MOCK_IMPL(void,
 connection_mark_unattached_ap_,(entry_connection_t *conn, int endreason,
                                 int line, const char *file))
 {
   connection_t *base_conn = ENTRY_TO_CONN(conn);
   edge_connection_t *edge_conn = ENTRY_TO_EDGE_CONN(conn);
   tor_assert(base_conn->type == CONN_TYPE_AP);
   ENTRY_TO_EDGE_CONN(conn)->edge_has_sent_end = 1; /* no circ yet */

   /* If this is a rendezvous stream and it is failing without ever
    * being attached to a circuit, assume that an attempt to connect to
    * the destination hidden service has just ended.
    *
    * XXXX This condition doesn't limit to only streams failing
    * without ever being attached.  That sloppiness should be harmless,
    * but we should fix it someday anyway. */
   if ((edge_conn->on_circuit != NULL || edge_conn->edge_has_sent_end) &&
       connection_edge_is_rendezvous_stream(edge_conn)) {
     rend_client_note_connection_attempt_ended(
                                     edge_conn->rend_data->onion_address);
   }
 }}}

 The condition is equivalent to:

 {{{
 if (connection_edge_is_rendezvous_stream(edge_conn)) {
     rend_client_note_connection_attempt_ended(
                                     edge_conn->rend_data->onion_address);
   }
 }}}

 This probably causes some superfluous calls to
 rend_client_note_connection_attempt_ended(), but we should investigate
 exactly what side effects calling that can have and the circumstances
 under which this is called, and then remove the stupid.

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