connection_mark_unattached_ap_: checking always true edge_has_sent_end

  ENTRY_TO_EDGE_CONN(conn)->edge_has_sent_end = 1; /* no circ yet */

  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);
  }

changed milestone to %Tor: unspecified

added cleanup component::core tor/tor milestone::Tor: unspecified priority::medium refactor severity::normal status::new technical-debt tor-client type::defect labels

Trac:
Summary: connection_mark_unattached_ap: checking always true edge_has_sent_end to connection_mark_unattached_ap: checking always true edge_has_sent_end

Is there any bad effect here? If so, this should maybe be 024-backport as well.

Trac:
Keywords: N/A deleted, tor-client added
Milestone: N/A to Tor: 0.2.5.x-final

Trac:
Keywords: tor-client deleted, tor-client 025-triaged added

Defer to 0.2.6, since as far as we can tell this is harmless.

Trac:
Milestone: Tor: 0.2.5.x-final to Tor: 0.2.6.x-final

(The code here seems confused and confusing in a lot of other ways beyond the issue here. We should try to untangle it while we're messing with it)

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.

Trac:
Milestone: Tor: 0.2.6.x-final to Tor: 0.2.???

Milestone renamed

Trac:
Milestone: Tor: 0.2.??? to Tor: 0.3.???

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

Trac:
Keywords: tor-client 025-triaged deleted, tor-client, tor-03-unspecified-201612, 025-triaged added
Milestone: Tor: 0.3.??? to Tor: unspecified

Remove an old triaging keyword.

Trac:
Keywords: tor-03-unspecified-201612 deleted, N/A added

remove an old triage keyword.

Trac:
Keywords: 025-triaged deleted, N/A added

Trac:
Reviewer: N/A to N/A
Keywords: tor-client deleted, tor-client cleanup refactor technical-debt added
Sponsor: N/A to N/A
Severity: N/A to Normal

moved to tpo/core/tor#10481 (closed)