From 73f7310d9b7fdca11278b139336a0671be1ee883 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Fri, 15 Jun 2007 04:20:51 +0000 Subject: [PATCH] Directories no longer return a "304 not modified" when they don't have the networkstatus the client asked for. Also fix a memory leak when returning 304 not modified. [Bugfixes on 0.2.0.2-alpha] svn:r10607 --- ChangeLog | 3 +++ src/or/directory.c | 2 ++ src/or/dirserv.c | 26 ++++++++++++++------------ 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1e7b5d530..b9e02d28f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -21,6 +21,9 @@ Changes in version 0.2.0.3-alpha - 2007-??-?? o Minor bugfixes (directory): - Fix another crash bug related to extra-info caching. (Bug found by Peter Palfrader.) [Bugfix on 0.2.0.2-alpha] + - Directories no longer return a "304 not modified" when they don't + have the networkstatus the client asked for. Also fix a memory + leak when returning 304 not modified. [Bugfixes on 0.2.0.2-alpha] o Minor bugfixes (dns): - Fix a crash when DNSPort is set more than once. (Patch from Robert diff --git a/src/or/directory.c b/src/or/directory.c index 83a7a7b0b..546b7e2cc 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -1806,6 +1806,8 @@ directory_handle_command_get(dir_connection_t *conn, const char *headers, } if (dirserv_statuses_are_old(dir_fps, if_modified_since)) { write_http_status_line(conn, 304, "Not modified"); + /* no need to free dir_fps's elements, since + * dirserv_statuses_are_old() already did. */ smartlist_free(dir_fps); return 0; } diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 69f73b99f..4d4b6902f 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -2538,29 +2538,31 @@ dirserv_test_reachability(int try_all) ctr = (ctr + 1) % 128; } -/** Return true iff every networkstatus listed in fps is older - * than cutoff. */ +/** Remove from fps every networkstatus key where both + * a) we have a networkstatus document and + * b) it is not newer than cutoff. + * + * Return 1 if no keys remain, else return 0. + */ int dirserv_statuses_are_old(smartlist_t *fps, time_t cutoff) { - SMARTLIST_FOREACH(fps, const char *, digest, + SMARTLIST_FOREACH(fps, char *, digest, { cached_dir_t *d; if (router_digest_is_me(digest) && the_v2_networkstatus) d = the_v2_networkstatus; else d = digestmap_get(cached_v2_networkstatus, digest); - if (d && d->published > cutoff) - return 0; - /* XXX020 The above is causing my dir cache to send "304 Not modified" - * to clients that never specified an If-Modified-Since header. That's - * because d isn't defined for the networkstatus the client is asking - * for, so we end up returning 1. Perhaps the right fix above is - * "if (!d || d->published >= cutoff)"? -RD - */ + if (d && d->published <= cutoff) { + tor_free(digest); + SMARTLIST_DEL_CURRENT(fps, digest); + } }); - return 1; + if (smartlist_len(fps)) + return 0; /* some items were not here or were not old */ + return 1; /* all items were here and old */ } /** Return an approximate estimate of the number of bytes that will