r11673@catbus: nickm | 2007-02-06 14:40:07 -0500

 Report stream end events where a resolve succeeded or where we got a socks protocol error correctly, rather than calling both of them "INTERNAL".  Turn ALREADY_SOCKS_REPLIED into a flag rather than a reason.  This will help debug 367 part 2 a little.


svn:r9511

ChangeLog

@@ -4,6 +4,8 @@ Changes in version 0.1.2.8-alpha - 2007-??-??
       clear the corresponding on_circuit variable, and remember later that
       we don't need to send a redundant CLOSED event.  (Resolves part 3 of
       bug 367.)
+    - Report events where a resolve succeeded or where we got a socks
+      protocol error correctly, rather than calling both of them "INTERNAL".
 
 
 Changes in version 0.1.2.7-alpha - 2007-02-06

src/or/connection_edge.c

@@ -50,7 +50,7 @@ _connection_mark_unattached_ap(edge_connection_t *conn, int endreason,
   }
 
   if (!conn->socks_request->has_finished) {
-    if (endreason == END_STREAM_REASON_ALREADY_SOCKS_REPLIED)
+    if (endreason & END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED)
       log_warn(LD_BUG,
                "Bug: stream (marked at %s:%d) sending two socks replies?",
                file, line);
@@ -1216,7 +1216,8 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
                                              strlen(socks->address),
                                              socks->address, -1);
       connection_mark_unattached_ap(conn,
-                                    END_STREAM_REASON_ALREADY_SOCKS_REPLIED);
+                                 END_STREAM_REASON_DONE |
+                                 END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED);
       return 0;
     }
   } else {
@@ -1307,7 +1308,8 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
         connection_ap_handshake_socks_resolved(conn,RESOLVED_TYPE_ERROR,
                                                0,NULL,-1);
         connection_mark_unattached_ap(conn,
-                                      END_STREAM_REASON_ALREADY_SOCKS_REPLIED);
+                                END_STREAM_REASON_SOCKSPROTOCOL |
+                                END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED);
         return -1;
       }
       if (tor_inet_aton(socks->address, &in)) { /* see if it's an IP already */
@@ -1315,7 +1317,8 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
         connection_ap_handshake_socks_resolved(conn,RESOLVED_TYPE_IPV4,4,
                                                (char*)&answer,-1);
         connection_mark_unattached_ap(conn,
-                                      END_STREAM_REASON_ALREADY_SOCKS_REPLIED);
+                                END_STREAM_REASON_DONE |
+                                END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED);
         return 0;
       }
       rep_hist_note_used_resolve(time(NULL)); /* help predict this next time */
@@ -1373,7 +1376,8 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
       connection_ap_handshake_socks_resolved(conn,RESOLVED_TYPE_ERROR,
                                              0,NULL,-1);
       connection_mark_unattached_ap(conn,
-                                    END_STREAM_REASON_ALREADY_SOCKS_REPLIED);
+                                END_STREAM_REASON_SOCKSPROTOCOL |
+                                END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED);
       return -1;
     }
 
@@ -1574,7 +1578,8 @@ connection_ap_handshake_process_socks(edge_connection_t *conn)
                                           END_STREAM_REASON_SOCKSPROTOCOL);
     }
     connection_mark_unattached_ap(conn,
-                                  END_STREAM_REASON_ALREADY_SOCKS_REPLIED);
+                              END_STREAM_REASON_SOCKSPROTOCOL |
+                              END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED);
     return -1;
   } /* else socks handshake is done, continue processing */
 

src/or/control.c

@@ -3156,7 +3156,6 @@ stream_end_reason_to_string(int reason)
     case END_STREAM_REASON_TORPROTOCOL: return "TORPROTOCOL";
     case END_STREAM_REASON_NOTDIRECTORY: return "NOTDIRECTORY";
 
-    case END_STREAM_REASON_ALREADY_SOCKS_REPLIED: return "INTERNAL";
     case END_STREAM_REASON_CANT_ATTACH: return "CANT_ATTACH";
     case END_STREAM_REASON_NET_UNREACHABLE: return "NET_UNREACHABLE";
     case END_STREAM_REASON_SOCKSPROTOCOL: return "SOCKS_PROTOCOL";

src/or/or.h

@@ -518,8 +518,7 @@ typedef enum {
  * and are not intended to be put in relay end cells. They are here
  * to be more informative when sending back socks replies to the
  * application. */
-/** DOCDOC */
-#define END_STREAM_REASON_ALREADY_SOCKS_REPLIED 256
+/* XXXX 256 is no longer used; feel free to reuse it. */
 /** DOCDOC */
 #define END_STREAM_REASON_CANT_ATTACH 257
 /** DOCDOC */
@@ -540,6 +539,10 @@ typedef enum {
 /** Bitwise-or this with the argument to control_event_stream_status
  * to indicate that we already sent a CLOSED stream event. */
 #define END_STREAM_REASON_FLAG_ALREADY_SENT_CLOSED 1024
+/** Bitwise-or this with endreason to indicate that we already sent
+ * a socks reply, and no further reply needs to be sent from
+ * connection_mark_unattached_ap(). */
+#define END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED 2048
 
 /* DOCDOC */
 #define RESOLVED_TYPE_HOSTNAME 0

src/or/relay.c

@@ -584,7 +584,7 @@ connection_edge_end_reason_str(int reason)
 socks5_reply_status_t
 connection_edge_end_reason_socks5_response(int reason)
 {
-  switch (reason) {
+  switch (reason & END_STREAM_REASON_MASK) {
     case 0:
       return SOCKS5_SUCCEEDED;
     case END_STREAM_REASON_MISC:
@@ -612,8 +612,6 @@ connection_edge_end_reason_socks5_response(int reason)
     case END_STREAM_REASON_TORPROTOCOL:
       return SOCKS5_GENERAL_ERROR;
 
-    case END_STREAM_REASON_ALREADY_SOCKS_REPLIED:
-      return SOCKS5_SUCCEEDED; /* never used */
     case END_STREAM_REASON_CANT_ATTACH:
       return SOCKS5_GENERAL_ERROR;
     case END_STREAM_REASON_NET_UNREACHABLE:
@@ -922,7 +920,8 @@ connection_edge_process_relay_cell_not_open(
                    cell->payload+RELAY_HEADER_SIZE+2, /*answer*/
                    ttl);
     connection_mark_unattached_ap(conn,
-                                  END_STREAM_REASON_ALREADY_SOCKS_REPLIED);
+                              END_STREAM_REASON_DONE |
+                              END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED);
     return 0;
   }