Merge branch 'tor-github/pr/696'

changes/ticket28816

@@ -0,0 +1,4 @@
+  o Code simplification and refactoring:
+    - Introduce a connection_dir_buf_add() helper function that checks for
+      compress_state of dir_connection_t and automatically writes a string to
+      directory connection with or without compression. Resolves issue 28816.

src/core/mainloop/connection.c

@@ -4341,6 +4341,23 @@ connection_write_to_buf_impl_,(const char *string, size_t len,
   connection_write_to_buf_commit(conn, written);
 }
 
+/**
+ * Write a <b>string</b> (of size <b>len</b> to directory connection
+ * <b>dir_conn</b>. Apply compression if connection is configured to use
+ * it and finalize it if <b>done</b> is true.
+ */
+void
+connection_dir_buf_add(const char *string, size_t len,
+                       dir_connection_t *dir_conn, int done)
+{
+  if (dir_conn->compress_state != NULL) {
+    connection_buf_add_compress(string, len, dir_conn, done);
+    return;
+  }
+
+  connection_buf_add(string, len, TO_CONN(dir_conn));
+}
+
 void
 connection_buf_add_compress(const char *string, size_t len,
                             dir_connection_t *conn, int done)

src/core/mainloop/connection.h

@@ -226,6 +226,8 @@ MOCK_DECL(void, connection_write_to_buf_impl_,
 /* DOCDOC connection_write_to_buf */
 static void connection_buf_add(const char *string, size_t len,
                                     connection_t *conn);
+void connection_dir_buf_add(const char *string, size_t len,
+                            dir_connection_t *dir_conn, int done);
 static inline void
 connection_buf_add(const char *string, size_t len, connection_t *conn)
 {

src/feature/dircache/dircache.c

@@ -1068,13 +1068,11 @@ handle_get_status_vote(dir_connection_t *conn, const get_handler_args_t *args)
       if (compress_method != NO_METHOD) {
         conn->compress_state = tor_compress_new(1, compress_method,
                            choose_compression_level(estimated_len));
-        SMARTLIST_FOREACH(items, const char *, c,
-                 connection_buf_add_compress(c, strlen(c), conn, 0));
-        connection_buf_add_compress("", 0, conn, 1);
-      } else {
-        SMARTLIST_FOREACH(items, const char *, c,
-                         connection_buf_add(c, strlen(c), TO_CONN(conn)));
       }
+
+      SMARTLIST_FOREACH(items, const char *, c,
+                        connection_dir_buf_add(c, strlen(c), conn,
+                                               c_sl_idx == c_sl_len - 1));
     } else {
       SMARTLIST_FOREACH(dir_items, cached_dir_t *, d,
           connection_buf_add(compress_method != NO_METHOD ?
@@ -1325,19 +1323,13 @@ handle_get_keys(dir_connection_t *conn, const get_handler_args_t *args)
     if (compress_method != NO_METHOD) {
       conn->compress_state = tor_compress_new(1, compress_method,
                                               choose_compression_level(len));
-      SMARTLIST_FOREACH(certs, authority_cert_t *, c,
-            connection_buf_add_compress(
-                c->cache_info.signed_descriptor_body,
-                c->cache_info.signed_descriptor_len,
-                conn, 0));
-      connection_buf_add_compress("", 0, conn, 1);
-    } else {
-      SMARTLIST_FOREACH(certs, authority_cert_t *, c,
-            connection_buf_add(c->cache_info.signed_descriptor_body,
-                                    c->cache_info.signed_descriptor_len,
-                                    TO_CONN(conn)));
     }
-  keys_done:
+
+    SMARTLIST_FOREACH(certs, authority_cert_t *, c,
+          connection_dir_buf_add(c->cache_info.signed_descriptor_body,
+                                 c->cache_info.signed_descriptor_len,
+                                 conn, c_sl_idx == c_sl_len - 1));
+ keys_done:
     smartlist_free(certs);
     goto done;
   }

src/feature/dircache/dirserv.c

@@ -583,11 +583,9 @@ spooled_resource_flush_some(spooled_resource_t *spooled,
       /* Absent objects count as "done". */
       return SRFS_DONE;
     }
-    if (conn->compress_state) {
+
-      connection_buf_add_compress((const char*)body, bodylen, conn, 0);
+    connection_dir_buf_add((const char*)body, bodylen, conn, 0);
-    } else {
+
-      connection_buf_add((const char*)body, bodylen, TO_CONN(conn));
-    }
     return SRFS_DONE;
   } else {
     cached_dir_t *cached = spooled->cached_dir_ref;
@@ -622,14 +620,10 @@ spooled_resource_flush_some(spooled_resource_t *spooled,
     if (BUG(remaining < 0))
       return SRFS_ERR;
     ssize_t bytes = (ssize_t) MIN(DIRSERV_CACHED_DIR_CHUNK_SIZE, remaining);
-    if (conn->compress_state) {
+
-      connection_buf_add_compress(
+    connection_dir_buf_add(ptr + spooled->cached_dir_offset,
-              ptr + spooled->cached_dir_offset,
+                           bytes, conn, 0);
-              bytes, conn, 0);
+
-    } else {
-      connection_buf_add(ptr + spooled->cached_dir_offset,
-                              bytes, TO_CONN(conn));
-    }
     spooled->cached_dir_offset += bytes;
     if (spooled->cached_dir_offset >= (off_t)total_len) {
       return SRFS_DONE;