commit cb54e44587473782c2865c3ea4aca6e0666943a8 Author: George Kadianakis desnacked@riseup.net Date: Thu Jun 27 17:50:56 2013 +0300

Fix a number of issues with the #5040 code.

- Don't leak if a transport proxy sends us a TRANSPORT command more than once.

- Don't use smartlist_string_isin() in geoip_get_transport_history(). (pointed out by Nick)

- Use the 'join' argument of smartlist_join_strings() instead of trying to write the separator on our own. (pointed out by Nick)

- Document 'ext_or_transport' a bit better. (pointed out by Nick)

- Be a bit more consistent with the types of the values of 'transport_counts'. (pointed out by Nick) --- src/or/ext_orport.c | 6 ++++++ src/or/geoip.c | 13 +++++-------- src/or/or.h | 5 +++-- 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/or/ext_orport.c b/src/or/ext_orport.c index f83002c..8fd9b77 100644 --- a/src/or/ext_orport.c +++ b/src/or/ext_orport.c @@ -460,6 +460,12 @@ connection_ext_or_handle_cmd_transport(or_connection_t *conn, return -1; }

+ /* If ext_or_transport is already occupied (because the PT sent two + * TRANSPORT commands), deallocate the old name and keep the new + * one */ + if (conn->ext_or_transport) + tor_free(conn->ext_or_transport); + conn->ext_or_transport = transport_str; return 0; } diff --git a/src/or/geoip.c b/src/or/geoip.c index 7244c56..737512f 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -807,7 +807,6 @@ geoip_get_transport_history(void) const char *transport_name = NULL; smartlist_t *string_chunks = smartlist_new(); char *the_string = NULL; - int i = 0;

/* If we haven't seen any clients yet, return NULL. */ if (HT_EMPTY(&client_history)) @@ -841,7 +840,7 @@ geoip_get_transport_history(void) strmap_set(transport_counts, transport_name, ptr);

/* If it's the first time we see this transport, note it. */ - if (!smartlist_contains_string(transports_used, transport_name)) + if (val == 1) smartlist_add(transports_used, tor_strdup(transport_name));

log_debug(LD_GENERAL, "Client from '%s' with transport '%s'. " @@ -857,20 +856,18 @@ geoip_get_transport_history(void) /* Loop through all seen transports. */ SMARTLIST_FOREACH_BEGIN(transports_used, const char *, transport_name) { void *transport_count_ptr = strmap_get(transport_counts, transport_name); - unsigned int transport_count = (uintptr_t) transport_count_ptr; - i++; /* counter so that we don't add a comma if it's the last transport. */ + unsigned int transport_count = (intptr_t) transport_count_ptr;

log_debug(LD_GENERAL, "We got %u clients with transport '%s'.", transport_count, transport_name);

- smartlist_add_asprintf(string_chunks, "%s=%u%s", + smartlist_add_asprintf(string_chunks, "%s=%u", transport_name, round_to_next_multiple_of(transport_count, - granularity), - i != smartlist_len(transports_used) ? "," : ""); + granularity)); } SMARTLIST_FOREACH_END(transport_name);

- the_string = smartlist_join_strings(string_chunks, "", 0, NULL); + the_string = smartlist_join_strings(string_chunks, ",", 0, NULL);

log_debug(LD_GENERAL, "Final bridge-ip-transports string: '%s'", the_string);

diff --git a/src/or/or.h b/src/or/or.h index 7916c47..9b519a7 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1452,8 +1452,9 @@ typedef struct or_connection_t { char *ext_or_conn_id; /** Client hash of the Extended ORPort authentication scheme */ char *ext_or_auth_correct_client_hash; - /** Name of the pluggable transport that is obfuscating this - connection. If no pluggable transports are used, it's NULL. */ + /** String carrying the name of the pluggable transport + * (e.g. "obfs2") that is obfuscating this connection. If no + * pluggable transports are used, it's NULL. */ char *ext_or_transport;

char *nickname; /**< Nickname of OR on other side (if any). */