You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by so...@apache.org on 2005/08/22 19:22:32 UTC

svn commit: r235759 - in /httpd/mod_smtpd/trunk: mod_smtpd.h smtp.h smtp_core.c smtp_protocol.c

Author: soc-rian
Date: Mon Aug 22 10:22:27 2005
New Revision: 235759

URL: http://svn.apache.org/viewcvs?rev=235759&view=rev
Log:
mod_smtpd overhaul:
1. new structs: smtpd_conn_rec, smtpd_trans_rec
2. different i/o API: smtpd_getline, smtpd_respond_oneline,
   smtpd_respond_multiline.

Modified:
    httpd/mod_smtpd/trunk/mod_smtpd.h
    httpd/mod_smtpd/trunk/smtp.h
    httpd/mod_smtpd/trunk/smtp_core.c
    httpd/mod_smtpd/trunk/smtp_protocol.c

Modified: httpd/mod_smtpd/trunk/mod_smtpd.h
URL: http://svn.apache.org/viewcvs/httpd/mod_smtpd/trunk/mod_smtpd.h?rev=235759&r1=235758&r2=235759&view=diff
==============================================================================
--- httpd/mod_smtpd/trunk/mod_smtpd.h (original)
+++ httpd/mod_smtpd/trunk/mod_smtpd.h Mon Aug 22 10:22:27 2005
@@ -63,7 +63,7 @@
     SMTPD_STATE_GOT_HELO,
     SMTPD_STATE_GOT_MAIL,
     SMTPD_STATE_GOT_RCPT
-  } smtpd_request_state;
+  } smtpd_trans_state;
 
   typedef enum {
     SMTPD_PROTOCOL_SMTP,
@@ -72,78 +72,90 @@
 
   typedef struct smtpd_return_data {
     apr_pool_t *p;
-    char *msg;
+    /* list of messages
+       null terminated */
+    char **msgs;
   } smtpd_return_data;
 
-  typedef struct smtpd_request_rec {
+  typedef struct smtpd_trans_rec {
     apr_pool_t *p;
-    conn_rec *c;
-    server_rec *s;
-
-    /* spooled data file pointer */
-    apr_file_t *tfp;
 
     /* where are we in the current transaction */
-    smtpd_request_state state_vector;
+    smtpd_trans_state state_vector;
 
     /* is this esmtp or smtp */
     /* by default smtp */
     smtpd_protocol_type extended;
 
-    apr_array_header_t *extensions;
+    /* hostname we were helo'd with */
+    char *helo;
 
     /* string of who this mail is from */
     char *mail_from;
 
+    /* recipient list */
     apr_array_header_t *rcpt_to;
 
-    /* hostname we were helo'd with */
-    char *helo;
-  } smtpd_request_rec;
+    /* spooled data file pointer */
+    apr_file_t *tfp;
+  } smtpd_trans_rec;
 
+  typedef struct smtpd_conn_rec {
+    apr_pool_t *p;
+    conn_rec *c;
+    server_rec *s;
+   
+    /* extensions registered with this connection */
+    apr_array_header_t *extensions;
+
+    /* current transaction */
+    smtpd_trans_rec *transaction;
 
+    /* bb in */
+    apr_bucket_brigade *bb_in;
+
+    /* bb out */
+    apr_bucket_brigade *bb_out;
+  } smtpd_conn_rec;
   
   /* public */
-  SMTPD_DECLARE_NONSTD(smtpd_request_rec *)
-  smtpd_get_request_rec(request_rec *r);
-       
   SMTPD_DECLARE_NONSTD(void)
-  smtpd_register_extension(smtpd_request_rec *, const char *line);
+  smtpd_register_extension(smtpd_conn_rec *scr, const char *line);
        
   SMTPD_DECLARE_NONSTD(void)
-  smtpd_reset_transaction(request_rec *r);
+  smtpd_reset_transaction(smtpd_conn_rec *scr);
 
   APR_DECLARE_EXTERNAL_HOOK(smtpd, SMTPD, smtpd_retcode,
 			    unrecognized_command,
-			    (request_rec *r, smtpd_return_data *in,
+			    (smtpd_conn_rec *scr, smtpd_return_data *in,
 			     char *command, char *data));
   APR_DECLARE_EXTERNAL_HOOK(smtpd, SMTPD, smtpd_retcode, connect,
-			    (request_rec *r, smtpd_return_data *in));
+			    (smtpd_conn_rec *scr, smtpd_return_data *in));
   APR_DECLARE_EXTERNAL_HOOK(smtpd, SMTPD, smtpd_retcode, reset_transaction,
-			    (request_rec *r));
+			    (smtpd_conn_rec *scr));
   APR_DECLARE_EXTERNAL_HOOK(smtpd, SMTPD, smtpd_retcode, helo,
-			    (request_rec *r, smtpd_return_data *in,
+			    (smtpd_conn_rec *scr, smtpd_return_data *in,
 			     char *str));
   APR_DECLARE_EXTERNAL_HOOK(smtpd, SMTPD, smtpd_retcode, ehlo,
-			    (request_rec *r, smtpd_return_data *in,
+			    (smtpd_conn_rec *scr, smtpd_return_data *in,
 			     char *str));
   APR_DECLARE_EXTERNAL_HOOK(smtpd, SMTPD, smtpd_retcode, mail,
-			    (request_rec *r, smtpd_return_data *in,
+			    (smtpd_conn_rec *scr, smtpd_return_data *in,
 			     char *str));
   APR_DECLARE_EXTERNAL_HOOK(smtpd, SMTPD, smtpd_retcode, rcpt,
-			    (request_rec *r, smtpd_return_data *in,
+			    (smtpd_conn_rec *scr, smtpd_return_data *in,
 			     char *str));
   APR_DECLARE_EXTERNAL_HOOK(smtpd, SMTPD, smtpd_retcode, vrfy,
-			    (request_rec *r, smtpd_return_data *in,
+			    (smtpd_conn_rec *scr, smtpd_return_data *in,
 			     char *str));
   APR_DECLARE_EXTERNAL_HOOK(smtpd, SMTPD, smtpd_retcode, quit,
-			    (request_rec *r, smtpd_return_data *in));
+			    (smtpd_conn_rec *scr, smtpd_return_data *in));
   APR_DECLARE_EXTERNAL_HOOK(smtpd, SMTPD, smtpd_retcode, data,
-  			    (request_rec *r, smtpd_return_data *in));
+  			    (smtpd_conn_rec *scr, smtpd_return_data *in));
   APR_DECLARE_EXTERNAL_HOOK(smtpd, SMTPD, smtpd_retcode, data_post,
-			    (request_rec *r, smtpd_return_data *in));
+			    (smtpd_conn_rec *scr, smtpd_return_data *in));
   APR_DECLARE_EXTERNAL_HOOK(smtpd, SMTPD, smtpd_retcode, queue,
-			    (request_rec *r, smtpd_return_data *in));
+			    (smtpd_conn_rec *scr, smtpd_return_data *in));
   
 
 #ifdef __cplusplus

Modified: httpd/mod_smtpd/trunk/smtp.h
URL: http://svn.apache.org/viewcvs/httpd/mod_smtpd/trunk/smtp.h?rev=235759&r1=235758&r2=235759&view=diff
==============================================================================
--- httpd/mod_smtpd/trunk/smtp.h (original)
+++ httpd/mod_smtpd/trunk/smtp.h Mon Aug 22 10:22:27 2005
@@ -23,7 +23,7 @@
 #endif
 
   /* SMTP handlers registration */
-#define HANDLER_PROTOTYPE request_rec *r, char *buffer, smtpd_return_data *in_data, void *data
+#define HANDLER_PROTOTYPE smtpd_conn_rec *scr, char *buffer, smtpd_return_data *in_data, void *data
 
 #define HANDLER_FUNC(name)  smtpd_handler_##name
 #define HANDLER_DECLARE(name) SMTPD_DECLARE(int) HANDLER_FUNC(name) (HANDLER_PROTOTYPE)
@@ -43,13 +43,22 @@
   } smtpd_handler_st;
 
   void
-  smtpd_process_connection_internal(request_rec *r);
+  smtpd_process_connection_internal(smtpd_conn_rec *str);
 
   void
-  smtpd_clear_request_rec(smtpd_request_rec *);
+  smtpd_clear_trans_rec(smtpd_trans_rec *);
 
   apr_hash_t *
   smtpd_get_handlers(void);
+
+  apr_status_t
+  smtpd_getline(smtpd_conn_rec *scr, char *data, apr_size_t dlen);
+
+  apr_status_t
+  smtpd_respond_multiline(smtpd_conn_rec *scr, int code, char *messages[]);
+
+  apr_status_t
+  smtpd_respond_oneline(smtpd_conn_rec *scr, int code, char *message);
     
   HANDLER_DECLARE(ehlo);
   HANDLER_DECLARE(helo);

Modified: httpd/mod_smtpd/trunk/smtp_core.c
URL: http://svn.apache.org/viewcvs/httpd/mod_smtpd/trunk/smtp_core.c?rev=235759&r1=235758&r2=235759&view=diff
==============================================================================
--- httpd/mod_smtpd/trunk/smtp_core.c (original)
+++ httpd/mod_smtpd/trunk/smtp_core.c Mon Aug 22 10:22:27 2005
@@ -47,114 +47,199 @@
 /* Implement 'smtpd_run_unrecognized_command'. */
 APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(smtpd, SMTPD, smtpd_retcode,
 				    unrecognized_command,
-				    (request_rec *r, smtpd_return_data *in,
+				    (smtpd_conn_rec *scr, smtpd_return_data *in,
 				     char *command, char *data),
-				    (r, in, command, data),
+				    (scr, in, command, data),
 				    SMTPD_DECLINED, SMTPD_DECLINED);
 /* Implement 'smtpd_run_connect'. */
 APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(smtpd, SMTPD, smtpd_retcode, connect,
-				    (request_rec *r, smtpd_return_data *in),
-				    (r, in),
+				    (smtpd_conn_rec *scr, smtpd_return_data *in),
+				    (scr, in),
 				    SMTPD_DECLINED, SMTPD_DECLINED);
 /* Implement 'smtpd_run_reset_transaction'. */
 APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(smtpd, SMTPD, smtpd_retcode,
 				    reset_transaction,
-				    (request_rec *r),
-				    (r),
+				    (smtpd_conn_rec *scr),
+				    (scr),
 				    SMTPD_DECLINED, SMTPD_DECLINED);
 /* Implement 'smtpd_run_helo'. */
 APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(smtpd, SMTPD, smtpd_retcode, helo,
-				    (request_rec *r, smtpd_return_data *in,
+				    (smtpd_conn_rec *scr, smtpd_return_data *in,
 				     char *str),
-				    (r, in, str),
+				    (scr, in, str),
 				    SMTPD_DECLINED, SMTPD_DECLINED);
 /* Implement 'smtpd_run_ehlo'. */
 APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(smtpd, SMTPD, smtpd_retcode, ehlo,
-				    (request_rec *r, smtpd_return_data *in,
+				    (smtpd_conn_rec *scr, smtpd_return_data *in,
 				     char *str),
-				    (r, in, str),
+				    (scr, in, str),
 				    SMTPD_DECLINED, SMTPD_DECLINED);
 /* Implement 'smtpd_run_mail'. */
 APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(smtpd, SMTPD, smtpd_retcode, mail,
-				    (request_rec *r, smtpd_return_data *in,
+				    (smtpd_conn_rec *scr, smtpd_return_data *in,
 				     char *str),
-				    (r, in, str),
+				    (scr, in, str),
 				    SMTPD_DECLINED, SMTPD_DECLINED);
 /* Implement 'smtpd_run_rcpt'. */
 APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(smtpd, SMTPD, smtpd_retcode, rcpt,
-				    (request_rec *r, smtpd_return_data *in,
+				    (smtpd_conn_rec *scr, smtpd_return_data *in,
 				     char *str),
-				    (r, in, str),
+				    (scr, in, str),
 				    SMTPD_DECLINED, SMTPD_DECLINED);
 /* Implement 'smtpd_run_vrfy'. */
 APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(smtpd, SMTPD, smtpd_retcode, vrfy,
-				    (request_rec *r, smtpd_return_data *in,
+				    (smtpd_conn_rec *scr, smtpd_return_data *in,
 				     char *str),
-				    (r, in, str),
+				    (scr, in, str),
 				    SMTPD_DECLINED, SMTPD_DECLINED);
 /* Implement 'smtpd_run_quit'. */
 APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(smtpd, SMTPD, smtpd_retcode, quit,
-				    (request_rec *r, smtpd_return_data *in),
-				    (r, in),
+				    (smtpd_conn_rec *scr, smtpd_return_data *in),
+				    (scr, in),
 				    SMTPD_DECLINED, SMTPD_DECLINED);
 /* Implement 'smtpd_run_data'. */
 APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(smtpd, SMTPD, smtpd_retcode, data,
-				    (request_rec *r, smtpd_return_data *in),
-				    (r, in),
+				    (smtpd_conn_rec *scr, smtpd_return_data *in),
+				    (scr, in),
 				    SMTPD_DECLINED, SMTPD_DECLINED);
 /* Implement 'smtpd_run_data_post'. */
 APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(smtpd, SMTPD, smtpd_retcode,
 				    data_post,
-				    (request_rec *r, smtpd_return_data *in),
-				    (r, in),
+				    (smtpd_conn_rec *scr, smtpd_return_data *in),
+				    (scr, in),
 				    SMTPD_DECLINED, SMTPD_DECLINED);
 /* Implement 'smtpd_run_queue'. */
 APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(smtpd, SMTPD, smtpd_retcode, queue,
-				    (request_rec *r, smtpd_return_data *in),
-				    (r, in),
+				    (smtpd_conn_rec *scr, smtpd_return_data *in),
+				    (scr, in),
 				    SMTPD_DECLINED, SMTPD_DECLINED);
 				      
 /* public methods */
 /* functions other modules can use */
-				      
-/* accessor */
-SMTPD_DECLARE_NONSTD(smtpd_request_rec *)
-smtpd_get_request_rec(request_rec *r) {
-  return ap_get_module_config(r->request_config, &smtpd_module);
-}
 
 /* should be called at smtpd_hook_connect */
 SMTPD_DECLARE_NONSTD(void)
-smtpd_register_extension(smtpd_request_rec *sr, const char *line)
+smtpd_register_extension(smtpd_conn_rec *scr, const char *line)
 {
-  (*((char **)apr_array_push(sr->extensions))) = apr_pstrdup(sr->p, line);
+  (*((char **)apr_array_push(scr->extensions))) =
+    apr_pstrdup(scr->p, line);
 }
 
 /* how to reset the transaction */
 SMTPD_DECLARE_NONSTD(void)
-smtpd_reset_transaction(request_rec *r) {
+smtpd_reset_transaction(smtpd_conn_rec *scr) {
   /* REVIEW: don't know whether to run clear request first
    * then run reset hooks, or run reset hooks then clear request
    * depends on whether hooks want to save info before it gets cleared out
    * or if they want to overwrite default values in the request rec
-   * smtpd_run_reset_transaction(r);
    */
-  smtpd_clear_request_rec(smtpd_get_request_rec(r));
+  smtpd_run_reset_transaction(scr);
+
+  smtpd_clear_trans_rec(scr->transaction);
 }
 
 /* friend methods */
 /* only our sources can call these */
 
+apr_status_t
+smtpd_getline(smtpd_conn_rec *scr, char *data, apr_size_t dlen)
+{
+  apr_status_t rc;
+  apr_bucket *e;
+  const char *str;
+  char *pos, *last_char = data;
+  apr_size_t len, bytes_handled = 0;
+
+  while (1) {
+    rc = ap_get_brigade(scr->c->input_filters, scr->bb_in, AP_MODE_GETLINE,
+			APR_BLOCK_READ, 0);
+    if (rc != APR_SUCCESS) return rc;
+    
+    while(!APR_BRIGADE_EMPTY(scr->bb_in)) {
+      e = APR_BRIGADE_FIRST(scr->bb_in);
+      
+      rc = apr_bucket_read(e, &str, &len, APR_BLOCK_READ);
+      if (rc != APR_SUCCESS) return rc;      
+      apr_bucket_delete(e);
+           
+      if (len == 0) continue;
+ 
+      /* Would this overrun our buffer?  If so, we'll die. */
+      if (dlen < bytes_handled + len) {
+	if (data) {
+	  /* ensure this string is NUL terminated */
+	  if (bytes_handled > 0) {
+	    data[bytes_handled-1] = '\0';
+	  }
+	  else {
+	    data[0] = '\0';
+	  }
+	}
+	return APR_ENOSPC;
+      }
+
+      /* Just copy the rest of the data to the end of the old buffer. */
+      pos = data + bytes_handled;
+      memcpy(pos, str, len);
+      last_char = pos + len - 1;
+            
+      /* We've now processed that new data - update accordingly. */
+      bytes_handled += len;
+    }
+    
+    /* brigade was empty from the beginning, no block? */
+    if (!bytes_handled)
+      return APR_EGENERAL;
+
+    /* If we got a full line of input, stop reading */
+    if (last_char && (*last_char == APR_ASCII_LF)) {
+      break;
+    }
+  }
+
+  /* Now NUL-terminate the string at the end of the line; 
+   * if the last-but-one character is a CR, terminate there */
+  if (last_char > data && last_char[-1] == APR_ASCII_CR) {
+    last_char--;
+  }
+
+  *last_char = '\0';
+  return APR_SUCCESS;
+}
+
+apr_status_t
+smtpd_respond_multiline(smtpd_conn_rec *scr, int code, char *messages[])
+{
+  char *str;
+  int i;
+
+  for (i = 0; messages[i]; i++) {
+    str = messages[i+1] ? "%d %s\r\n" : "%d-%s\r\n";
+    ap_fprintf(scr->c->output_filters, scr->bb_out, str, code, messages[i]);
+    ap_fflush(scr->c->output_filters, scr->bb_out);
+  }
+
+  return APR_SUCCESS;
+}
+
+apr_status_t
+smtpd_respond_oneline(smtpd_conn_rec *scr, int code, char *message)
+{
+  ap_fprintf(scr->c->output_filters, scr->bb_out, "%d %s\r\n", code, message);
+  ap_fflush(scr->c->output_filters, scr->bb_out);
+
+  return APR_SUCCESS;
+}
+
 void
-smtpd_clear_request_rec(smtpd_request_rec *sr) {
-  apr_pool_clear(sr->p);
-  sr->state_vector = SMTPD_STATE_GOT_NOTHING;
-  sr->tfp = NULL;
-  sr->extended = SMTPD_PROTOCOL_SMTP;
-  sr->extensions = apr_array_make(sr->p, 5, sizeof(char *));
-  sr->rcpt_to = apr_array_make(sr->p, 5, sizeof(char *));
-  sr->mail_from = NULL;
-  sr->helo = NULL;
+smtpd_clear_trans_rec(smtpd_trans_rec *str) {
+  apr_pool_clear(str->p);
+  str->state_vector = SMTPD_STATE_GOT_NOTHING;
+  str->tfp = NULL;
+  str->extended = SMTPD_PROTOCOL_SMTP;
+  str->rcpt_to = apr_array_make(str->p, 5, sizeof(char *));
+  str->mail_from = NULL;
+  str->helo = NULL;
 }
 
 apr_hash_t *
@@ -182,70 +267,48 @@
 }
 
 /* Creates the main request record for the connection */
-static request_rec *
-smtpd_create_request(conn_rec *conn)
+static smtpd_conn_rec *
+smtpd_create_conn_rec(conn_rec *conn)
 {
   apr_pool_t *p;
-  request_rec *r;
+  smtpd_conn_rec *scr;
   apr_pool_t *sp;
-  smtpd_request_rec *sr;
+  smtpd_trans_rec *str;
 
-  r = apr_pcalloc(conn->pool, sizeof(*r));
+  scr = apr_pcalloc(conn->pool, sizeof(*scr));
   apr_pool_create(&p, conn->pool);
-  r->pool            = p;
-  r->connection      = conn;
-  r->server          = conn->base_server;
-
-  r->user            = NULL;
-  r->ap_auth_type    = NULL;
-
-  r->allowed_methods = ap_make_method_list(p, 2);
-
-  r->headers_in      = apr_table_make(r->pool, 25);
-  r->subprocess_env  = apr_table_make(r->pool, 25);
-  r->headers_out     = apr_table_make(r->pool, 12);
-  r->err_headers_out = apr_table_make(r->pool, 5);
-  r->notes           = apr_table_make(r->pool, 5);
-
-  /* Must be set before we run create request hook */
-  r->request_config  = ap_create_request_config(r->pool);
-  
-  r->proto_output_filters = conn->output_filters;
-  r->output_filters  = r->proto_output_filters;
-  r->proto_input_filters = conn->input_filters;
-  r->input_filters   = r->proto_input_filters;
-  r->per_dir_config  = r->server->lookup_defaults;
-  
-  r->sent_bodyct     = 0;
+  scr->p            = p;
+  scr->c            = conn;
+  scr->s            = conn->base_server;
 
-  r->read_length     = 0;
-  r->read_body       = REQUEST_NO_BODY;
+  scr->extensions = apr_array_make(scr->p, 5, sizeof(char *));
 
-  r->status          = HTTP_OK;
-  r->the_request     = NULL;
+  scr->bb_in = apr_brigade_create(scr->p, scr->c->bucket_alloc);
+  scr->bb_out = apr_brigade_create(scr->p, scr->c->bucket_alloc);
 
-  apr_socket_t *csd=((core_net_rec *)r->input_filters->ctx)->client_socket;
+  //  r->request_config  = ap_create_request_config(r->pool);
+  
+  apr_socket_t *csd =
+    ((core_net_rec *)conn->input_filters->ctx)->client_socket;
   apr_socket_timeout_set(csd, APR_INT64_C(10000000000000));
 
-  /* create custom smtpd rec */
-  sr = apr_pcalloc(r->pool, sizeof(*sr));
+  /* create transaction rec */
+  str = apr_pcalloc(scr->p, sizeof(*str));
 
-  apr_pool_create(&sp, r->pool);
-  sr->p = sp;
-  sr->c = conn;
-  sr->s = conn->base_server;
-  smtpd_clear_request_rec(sr);
+  apr_pool_create(&sp, scr->p);
+  str->p = sp;
+  smtpd_clear_trans_rec(str);
 
-  ap_set_module_config(r->request_config, &smtpd_module, sr);
+  scr->transaction = str;
 
-  return r;
+  return scr;
 }
 
 /* process connection hook */
 static int
 process_smtp_connection(conn_rec *c)
 {
-  request_rec *r;
+  smtpd_conn_rec *scr;
   smtpd_svr_config_rec *pConfig =
     ap_get_module_config(c->base_server->module_config,
 			 &smtpd_module);
@@ -254,10 +317,10 @@
     return DECLINED;
   }
 
-  r = smtpd_create_request(c);
-  ap_update_child_status(r->connection->sbh, SERVER_BUSY_KEEPALIVE, r);
+  scr = smtpd_create_conn_rec(c);
+  //  ap_update_child_status(scr->c->sbh, SERVER_BUSY_KEEPALIVE, r);
 
-  smtpd_process_connection_internal(r);
+  smtpd_process_connection_internal(scr);
 
   return OK;
 }

Modified: httpd/mod_smtpd/trunk/smtp_protocol.c
URL: http://svn.apache.org/viewcvs/httpd/mod_smtpd/trunk/smtp_protocol.c?rev=235759&r1=235758&r2=235759&view=diff
==============================================================================
--- httpd/mod_smtpd/trunk/smtp_protocol.c (original)
+++ httpd/mod_smtpd/trunk/smtp_protocol.c Mon Aug 22 10:22:27 2005
@@ -37,13 +37,13 @@
 extern module AP_MODULE_DECLARE_DATA smtpd_module;
 
 inline static int
-smtpd_handle_unrecognized_command(request_rec *r, smtpd_return_data *in_data,
+smtpd_handle_unrecognized_command(smtpd_conn_rec *scr,
+				  smtpd_return_data *in_data,
 				  char *command, char *data);
 
-
 #define BUFFER_STR_LEN 1024
 void
-smtpd_process_connection_internal(request_rec *r)
+smtpd_process_connection_internal(smtpd_conn_rec *scr)
 {
   apr_pool_t *p;
   apr_hash_t *handlers = smtpd_get_handlers();
@@ -53,76 +53,84 @@
   char *command;
   smtpd_return_data in_data;
   smtpd_svr_config_rec *pConfig =
-    ap_get_module_config(r->server->module_config,
+    ap_get_module_config(scr->s->module_config,
 			 &smtpd_module);
 
-  apr_pool_create(&p, r->pool);
+  apr_pool_create(&p, scr->p);
   in_data.p = p;
 
-  in_data.msg = NULL;
-  switch(smtpd_run_connect(r, &in_data)) {
+  in_data.msgs = NULL;
+  switch(smtpd_run_connect(scr, &in_data)) {
   case SMTPD_DENY:
-    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, r->server,
+    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, scr->s,
 		 "Connection Denied");
-    ap_rprintf(r, "%d %s\r\n", 550, in_data.msg ? in_data.msg :
-	       "Connection from you denied, bye bye.");
+    if (in_data.msgs) {
+      smtpd_respond_multiline(scr, 550, in_data.msgs);
+    } else {
+      smtpd_respond_oneline(scr, 550, "Connection from you denied, bye bye.");
+    }
     goto end;
   case SMTPD_DENYSOFT:
-    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, r->server,
+    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, scr->s,
 		 "Connection Denied");
-    ap_rprintf(r, "%d %s\r\n", 450, in_data.msg ? in_data.msg :
-	       "Connection from you temporarily denied, bye bye.");
+    if (in_data.msgs) {
+      smtpd_respond_multiline(scr, 450, in_data.msgs);
+    } else {
+      smtpd_respond_oneline(scr, 450, "Connection from you temporarily "
+			    "denied, bye bye.");
+    }
     goto end;
   case SMTPD_DONE:
     break;
   case SMTPD_DONE_DISCONNECT:
     goto end;
   default:
-    ap_rprintf(r, "%d %s %s\r\n", 220, ap_get_server_name(r), pConfig->sId);
-    ap_rflush(r);
+    sprintf(buffer, "%s %s", scr->s->server_hostname, pConfig->sId);
+    smtpd_respond_oneline(scr, 220, buffer);
+    break;
   }
 
-  while (ap_getline(buffer, BUFFER_STR_LEN,
-		    r, 0) != -1) {
+  while (smtpd_getline(scr, buffer, BUFFER_STR_LEN) == APR_SUCCESS) {
     apr_pool_clear(p);
     command = ap_getword_white_nc(p, &buffer);
     ap_str_tolower(command);
     handle_func = apr_hash_get(handlers, command, APR_HASH_KEY_STRING);
 
-    in_data.msg = NULL;
+    in_data.msgs = NULL;
     /* command not recognized */
     if (!handle_func)  {
-      if (!smtpd_handle_unrecognized_command(r, &in_data, command, buffer))
+      if (!smtpd_handle_unrecognized_command(scr, &in_data, command, buffer))
 	break;
     } else {
-      if (!handle_func->func(r, buffer, &in_data, handle_func->data))
+      if (!handle_func->func(scr, buffer, &in_data, handle_func->data))
 	break;
     }
     
     buffer = cmdbuff;
-    ap_rflush(r);
   }
 
  end:
-  /* flush any output we may have before disconnecting */
-  ap_rflush(r);
   return;
 }
 
 inline static int
-smtpd_handle_unrecognized_command(request_rec *r, smtpd_return_data *in_data,
+smtpd_handle_unrecognized_command(smtpd_conn_rec *scr,
+				  smtpd_return_data *in_data,
 				  char *command, char *data) {
-  switch(smtpd_run_unrecognized_command(r, in_data, command, data)) {
+  switch(smtpd_run_unrecognized_command(scr, in_data, command, data)) {
   case SMTPD_DENY:
-    ap_rprintf(r, "%d %s\r\n", 521, in_data->msg ? in_data->msg : "");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 521, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 521, "Command Denied");
+    }
     return 521;
   case SMTPD_DONE:
     return 1;
   case SMTPD_DONE_DISCONNECT:
     return 0;
   default:
-    ap_rprintf(r, "%d %s\r\n", 500,
-	       "Syntax error, command unrecognized");
+    smtpd_respond_oneline(scr, 500, "Syntax error, command unrecognized");
     return 500;
   }
 
@@ -130,23 +138,31 @@
 
 HANDLER_DECLARE(ehlo) {
   int i = 0;
-  smtpd_request_rec *sr = smtpd_get_request_rec(r);
+  smtpd_trans_rec *str = scr->transaction;
 
   if (buffer[0] == '\0') {
-    ap_rprintf(r, "%d %s\r\n", 501, "Syntax: EHLO hostname");
+    smtpd_respond_oneline(scr, 501, "Syntax: EHLO hostname");
     return 501;
   }
   
-  switch(smtpd_run_ehlo(r, in_data, buffer)) {
+  switch(smtpd_run_ehlo(scr, in_data, buffer)) {
   case SMTPD_DONE:
     return 1;
   case SMTPD_DONE_DISCONNECT:
     return 0;
   case SMTPD_DENY:
-    ap_rprintf(r, "%d %s\r\n", 550, in_data->msg ? in_data->msg :  "");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 550, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 550, "");
+    }
     return 550;
   case SMTPD_DENYSOFT:
-    ap_rprintf(r, "%d %s\r\n", 450, in_data->msg ? in_data->msg :  "");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 450, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 450, "");
+    }
     return 450;
   default:
     break;
@@ -156,46 +172,57 @@
 
   /* RFC 2821 states that when ehlo or helo is received, reset */
   /* state */
-  smtpd_reset_transaction(r);
+  smtpd_reset_transaction(scr);
 
-  sr->helo = apr_pstrdup(sr->p, buffer);
-  sr->state_vector = SMTPD_STATE_GOT_HELO;
-  sr->extended = SMTPD_PROTOCOL_ESMTP;
-
-  if (sr->extensions->nelts) {
-    ap_rprintf(r, "%d-%s\r\n", 250, sr->helo);
-  
-    for (i = 0; i < sr->extensions->nelts - 1; ++i) {
-      ap_rprintf(r, "%d-%s\r\n", 250, ((char **)sr->extensions->elts)[i]);
+  str->helo = apr_pstrdup(str->p, buffer);
+  str->state_vector = SMTPD_STATE_GOT_HELO;
+  str->extended = SMTPD_PROTOCOL_ESMTP;
+
+  if (scr->extensions->nelts) {
+    char **messages = apr_palloc(in_data->p, sizeof(char *) *
+				 (scr->extensions->nelts + 2));
+    
+    messages[0] = str->helo;
+    for (i = 0; i < scr->extensions->nelts; ++i) {
+      messages[i+1] = ((char **)scr->extensions->elts)[i];
     }
-    ap_rprintf(r, "%d %s\r\n", 250, ((char **)sr->extensions->elts)[i]);
+    messages[i] = 0;
 
+    smtpd_respond_multiline(scr, 250, messages);
   } else {
-    ap_rprintf(r, "%d %s\r\n", 250, sr->helo);
+    smtpd_respond_oneline(scr, 250, str->helo);
   }
 
   return 250;
 }
 
 HANDLER_DECLARE(helo) {
-  smtpd_request_rec *sr = smtpd_get_request_rec(r);
+  smtpd_trans_rec *str = scr->transaction;
 
   /* bad syntax */
   if (buffer[0] == '\0') {
-    ap_rprintf(r, "%d %s\r\n", 501, "Syntax: HELO hostname");
+    smtpd_respond_oneline(scr, 501, "Syntax: HELO hostname");
     return 501;
   }
 
-  switch(smtpd_run_helo(r, in_data, buffer)) {
+  switch(smtpd_run_helo(scr, in_data, buffer)) {
   case SMTPD_DONE:
     return 1;
   case SMTPD_DONE_DISCONNECT:
     return 0;
   case SMTPD_DENY:
-    ap_rprintf(r, "%d %s\r\n", 550, in_data->msg ? in_data->msg :  "");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 550, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 550, "");
+    }
     return 550;
   case SMTPD_DENYSOFT:
-    ap_rprintf(r, "%d %s\r\n", 450, in_data->msg ? in_data->msg :  "");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 450, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 450, "");
+    }
     return 450;
   default:
     break;
@@ -203,81 +230,81 @@
 
   /* RFC 2821 states that when ehlo or helo is received, reset */
   /* state */
-  smtpd_reset_transaction(r);
+  smtpd_reset_transaction(scr);
   
-  sr->helo = apr_pstrdup(sr->p, buffer);
-  sr->state_vector = SMTPD_STATE_GOT_HELO;
-  ap_rprintf(r, "%d %s\r\n", 250, sr->helo);
+  str->helo = apr_pstrdup(str->p, buffer);
+  str->state_vector = SMTPD_STATE_GOT_HELO;
+  smtpd_respond_oneline(scr, 250, str->helo);
 
   return 250;
 }
 
 HANDLER_DECLARE(mail) {
-  smtpd_request_rec *sr = smtpd_get_request_rec(r);
+  smtpd_trans_rec *str = scr->transaction;
   char *loc;
 
   /* already got mail */
-  if (sr->state_vector == SMTPD_STATE_GOT_MAIL) {
-    ap_rprintf(r, "%d %s\r\n", 503, "Error: Nested MAIL command");
+  if (str->state_vector == SMTPD_STATE_GOT_MAIL) {
+    smtpd_respond_oneline(scr, 503, "Error: Nested MAIL command");
     return 503;
   }
 
   /* bad syntax */
   if ((loc = ap_strcasestr(buffer, "from:")) == NULL) {
-    ap_rprintf(r, "%d %s\r\n", 501, "Syntax: MAIL FROM:<address>");
+    smtpd_respond_oneline(scr, 501,  "Syntax: MAIL FROM:<address>");
     return 501;
   }
 
   loc += sizeof("from:") - 1;
 
-  ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, r->server,
+  ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, scr->s,
 	       "full from_parameter: %s", loc);  
 
-  switch(smtpd_run_mail(r, in_data, loc)) {
+  switch(smtpd_run_mail(scr, in_data, loc)) {
   case SMTPD_DONE:
     return 1;
   case SMTPD_DONE_DISCONNECT:
     /* zero to disconnect */
     return 0;
   case SMTPD_DENY:
-    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, r->server,
+    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, scr->s,
 		 "deny mail from %s (%s)", loc,
-		 in_data->msg ? in_data->msg : "");  
-    if (in_data->msg) {
-      ap_rprintf(r, "%d %s\r\n", 550, in_data->msg);
+		 in_data->msgs ? in_data->msgs[0] : "");  
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 550, in_data->msgs);
     } else {
-      ap_rprintf(r, "%d %s, denied\r\n", 550, loc);
+      smtpd_respond_oneline(scr, 550, "denied");
     }
     return 550;
   case SMTPD_DENYSOFT:
-    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, r->server,
+    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, scr->s,
 		 "denysoft mail from %s (%s)", loc,
-		 in_data->msg ? in_data->msg : "");  
-    if (in_data->msg) {
-      ap_rprintf(r, "%d %s\r\n", 450, in_data->msg);
+		 in_data->msgs ? in_data->msgs[0] : "");  
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 450, in_data->msgs);
     } else {
-      ap_rprintf(r, "%d %s, temporarily denied\r\n", 450, loc);
+      smtpd_respond_oneline(scr, 450, "temporarily denied");
     }
     return 450;
   case SMTPD_DENY_DISCONNECT:
-    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, r->server,
+    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, scr->s,
 		 "deny mail from %s (%s)", loc,
-		 in_data->msg ? in_data->msg : "");  
-    if (in_data->msg) {
-      ap_rprintf(r, "%d %s\r\n", 550, in_data->msg);
+		 in_data->msgs ? in_data->msgs[0] : "");  
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 550, in_data->msgs);
     } else {
-      ap_rprintf(r, "%d %s, denied\r\n", 550, loc);
+      smtpd_respond_oneline(scr, 550, "denied");
     }
     /* zero to disconnect */
     return 0;
   case SMTPD_DENYSOFT_DISCONNECT:
-    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, r->server,
+    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, scr->s,
 		 "denysoft mail from %s (%s)", loc,
-		 in_data->msg ? in_data->msg : "");  
-    if (in_data->msg) {
-      ap_rprintf(r, "%d %s\r\n", 450, in_data->msg);
+		 in_data->msgs[0] ? in_data->msgs[0] : "");  
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 450, in_data->msgs);
     } else {
-      ap_rprintf(r, "%d %s, temporarily denied\r\n", 450, loc);
+      smtpd_respond_oneline(scr, 450, "temporarily denied");
     }
     /* zero to disconnect */
     return 0;
@@ -286,110 +313,132 @@
   }
 
   /* default handling */
-  sr->mail_from = apr_pstrdup(sr->p, loc);
-  sr->state_vector = SMTPD_STATE_GOT_MAIL;
-  ap_rprintf(r, "%d %s\r\n", 250, "Ok");
+  str->mail_from = apr_pstrdup(str->p, loc);
+  str->state_vector = SMTPD_STATE_GOT_MAIL;
+  smtpd_respond_oneline(scr, 250, "Ok");
 
   return 250;
 }
 
 HANDLER_DECLARE(rcpt) {
-  smtpd_request_rec *sr = smtpd_get_request_rec(r);
+  smtpd_trans_rec *str = scr->transaction;
   char *loc;
 
   /* need mail first */
-  if ((sr->state_vector != SMTPD_STATE_GOT_MAIL) &&
-      (sr->state_vector != SMTPD_STATE_GOT_RCPT)) {
-    ap_rprintf(r, "%d %s\r\n", 503, "Error: need MAIL command");
+  if ((str->state_vector != SMTPD_STATE_GOT_MAIL) &&
+      (str->state_vector != SMTPD_STATE_GOT_RCPT)) {
+    smtpd_respond_oneline(scr, 503, "Error: need MAIL command");
     return 503;
   }
 
   /* bad syntax */
   if ((loc = ap_strcasestr(buffer, "to:")) == NULL) {
-    ap_rprintf(r, "%d %s\r\n", 501, "Syntax: RCPT TO:<address>");
+    smtpd_respond_oneline(scr, 501, "Syntax: RCPT TO:<address>");
     return 501;
   }
 
   loc += sizeof("to:") - 1;
 
-  switch(smtpd_run_rcpt(r, in_data, loc)) {
+  switch(smtpd_run_rcpt(scr, in_data, loc)) {
   case SMTPD_DONE:
     return 1;
   case SMTPD_DONE_DISCONNECT:
     return 0;
   case SMTPD_DENY:
-    ap_rprintf(r, "%d %s\r\n", 550, in_data->msg ? in_data->msg :
-	       "relaying denied");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 550, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 550, "relaying denied");
+    }
     return 550;
   case SMTPD_DENYSOFT:
-    if (in_data->msg) {
-      ap_rprintf(r, "%d %s\r\n", 450, in_data->msg);
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 450, in_data->msgs);
     } else {
-      ap_rprintf(r, "%d %s, relaying temporarily denied\r\n", 450, loc);
+      smtpd_respond_oneline(scr, 450, "relaying temporarily denied");
     }
     return 450;
   case SMTPD_DENY_DISCONNECT:
-    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, r->server,
+    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, scr->s,
 		 "delivery denied (%s)",
-		 in_data->msg ? in_data->msg : "");
-    ap_rprintf(r, "%d %s\r\n", 550, in_data->msg ? in_data->msg :
-	       "delivery denied");
+		 in_data->msgs ? in_data->msgs[0] : "");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 550, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 550, "relaying denied");
+    }
     return 0;
   case SMTPD_DENYSOFT_DISCONNECT:
-    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, r->server,
+    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, scr->s,
 		 "delivery denied (%s)",
-		 in_data->msg ? in_data->msg : "");
-    ap_rprintf(r, "%d %s\r\n", 450, in_data->msg ? in_data->msg :
-	       "relaying temporarily denied");
+		 in_data->msgs ? in_data->msgs[0] : "");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 450, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 450, "relaying temporarily denied");
+    }
     return 0;
   case SMTPD_OK: /* recipient is okay */
     break;
   default:
-    ap_rprintf(r, "%d %s\r\n", 450, "No plugin decided if relaying is "
-	       "allowed");
+    smtpd_respond_oneline(scr, 450,
+			  "No plugin decided if relaying is allowed");
     return 450;
   }
 
   /* add a recipient */
-  (*((char **)apr_array_push(sr->rcpt_to))) = apr_pstrdup(sr->p, loc);
-  sr->state_vector = SMTPD_STATE_GOT_RCPT;
-  ap_rprintf(r, "%d %s\r\n", 250, "Ok");
+  (*((char **)apr_array_push(str->rcpt_to))) = apr_pstrdup(str->p, loc);
+  str->state_vector = SMTPD_STATE_GOT_RCPT;
+  smtpd_respond_oneline(scr, 250, "Ok");
 
   return 250;
 }
 
 
 inline static int
-smtpd_queue(request_rec *r, smtpd_return_data *in_data) {
-    switch(smtpd_run_queue(r, in_data)) {
+smtpd_queue(smtpd_conn_rec *scr, smtpd_return_data *in_data) {
+    switch(smtpd_run_queue(scr, in_data)) {
     case SMTPD_DONE:
       return 1;
     case SMTPD_DONE_DISCONNECT:
       return 0;
     case SMTPD_OK:
-      ap_rprintf(r, "%d %s\r\n", 250, in_data->msg ? in_data->msg :
-		 "Queued");
+      if (in_data->msgs) {
+	smtpd_respond_multiline(scr, 250, in_data->msgs);
+      } else {
+	smtpd_respond_oneline(scr, 250, "Queued");
+      }
       return 250;
     case SMTPD_DENY:
-      ap_rprintf(r, "%d %s\r\n", 552, in_data->msg ? in_data->msg :
-		 "Message denied");
+      if (in_data->msgs) {
+	smtpd_respond_multiline(scr, 552, in_data->msgs);
+      } else {
+	smtpd_respond_oneline(scr, 552, "Message denied");
+      }
       return 552;
     case SMTPD_DENYSOFT:
-      ap_rprintf(r, "%d %s\r\n", 452, in_data->msg ? in_data->msg :
-		 "Message denied temporarily");
+      if (in_data->msgs) {
+	smtpd_respond_multiline(scr, 452, in_data->msgs);
+      } else {
+	smtpd_respond_oneline(scr, 452, "Message denied temporarily");
+      }
       return 452;
     default:
-      ap_rprintf(r, "%d %s\r\n", 451, in_data->msg ? in_data->msg :
-		 "Queuing declined or disabled; try again later");
+      if (in_data->msgs) {
+	smtpd_respond_multiline(scr, 451, in_data->msgs);
+      } else {
+	smtpd_respond_oneline(scr, 451,
+			      "Queuing declined or disabled; try again later");
+      }
       return 451;
     }
 }
 
 
 HANDLER_DECLARE(data) {
-  smtpd_request_rec *sr = smtpd_get_request_rec(r);
+  smtpd_trans_rec *str = scr->transaction;
   smtpd_svr_config_rec *pConfig =
-    ap_get_module_config(r->server->module_config,
+    ap_get_module_config(scr->s->module_config,
 			 &smtpd_module);
   int rv, retval = 0;
   char *tempfile;
@@ -397,7 +446,7 @@
   apr_file_t *tfp;
   apr_size_t len, total_data = 0;
 
-  switch(smtpd_run_data(r, in_data)) {
+  switch(smtpd_run_data(scr, in_data)) {
   case SMTPD_DONE:
     return 1;
   case SMTPD_DONE_DISCONNECT:
@@ -405,44 +454,55 @@
   case SMTPD_DENY:
     /* REVIEW: should we reset state here? */
     /* smtpd_clear_request_rec(sr); */
-    ap_rprintf(r, "%d %s\r\n", 554, in_data->msg ? in_data->msg :
-	       "Message denied");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 554, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 554, "Message denied");
+    }
     return 554;
   case SMTPD_DENYSOFT:
     /* REVIEW: should we reset state here? */
     /* smtpd_clear_request_rec(sr); */
-    ap_rprintf(r, "%d %s\r\n", 451, in_data->msg ? in_data->msg :
-	       "Message denied temporarily");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 451, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 451, "Message denied temporarily");
+    }
     return 451;
   case SMTPD_DENY_DISCONNECT:
-    ap_rprintf(r, "%d %s\r\n", 554, in_data->msg ? in_data->msg :
-	       "Message denied");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 554, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 554, "Message denied");
+    }
     return 0;
   case SMTPD_DENYSOFT_DISCONNECT:
-    ap_rprintf(r, "%d %s\r\n", 451, in_data->msg ? in_data->msg :
-	       "Message denied temporarily");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 451, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 451, "Message denied temporarily");
+    }
     return 0;
   default:
     break;
   }
 
-  if (sr->state_vector != SMTPD_STATE_GOT_RCPT) {
-    ap_rprintf(r, "%d %s\r\n", 503, "Error: need RCPT command");
+  if (str->state_vector != SMTPD_STATE_GOT_RCPT) {
+    smtpd_respond_oneline(scr, 503, "Error: need RCPT command");
     return 503;
   }
-  
-  ap_rprintf(r, "%d %s\r\n", 354, "End data with <CR><LF>.<CR><LF>");
-  ap_rflush(r);
 
-  bb = apr_brigade_create(sr->p, r->connection->bucket_alloc);
+  smtpd_respond_oneline(scr, 354, "End data with <CR><LF>.<CR><LF>");
+  
+  bb = apr_brigade_create(scr->p, scr->c->bucket_alloc);
 
-  tempfile = apr_pstrdup(sr->p, "/tmp/tmp.XXXXXX");
+  tempfile = apr_pstrdup(str->p, "/tmp/tmp.XXXXXX");
   rv = apr_file_mktemp(&tfp, tempfile,
 		       APR_CREATE | APR_WRITE | APR_READ |
-		       APR_DELONCLOSE, r->pool);
+		       APR_DELONCLOSE, scr->p);
 
   if (rv != APR_SUCCESS) {
-    ap_rprintf(r, "%d %s\r\n", 421, "Error: Internal");
+    smtpd_respond_oneline(scr, 421, "Error: Internal");
     /* file error close connection */
     return 0;
   }
@@ -450,8 +510,7 @@
   /* just wait until we get the line with a dot. */
   /* or until we can't read anymore. */
   /* or until we have too much data */
-  while ((ap_rgetline(&buffer, BUFFER_STR_LEN,
-		      &len, r, 0, bb) == APR_SUCCESS) &&
+  while ((smtpd_getline(scr, buffer, BUFFER_STR_LEN) == APR_SUCCESS) &&
 	 (strcmp(buffer, ".")) &&
 	 (total_data < pConfig->max_data)) {
     apr_file_write(tfp, buffer, &len);
@@ -460,9 +519,9 @@
     total_data += len;
   }
 
-  sr->tfp = tfp;
+  str->tfp = tfp;
   
-  switch(smtpd_run_data_post(r, in_data)) {
+  switch(smtpd_run_data_post(scr, in_data)) {
   case SMTPD_DONE:
     retval = 1;
     goto cleanup;
@@ -471,19 +530,25 @@
     goto cleanup;
   case SMTPD_DENY:
     retval = 552;
-    ap_rprintf(r, "%d %s\r\n", 552, in_data->msg ? in_data->msg :
-	       "Message denied");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 552, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 552, "Message denied");
+    }
     break;
   case SMTPD_DENYSOFT:
     retval = 452;
-    ap_rprintf(r, "%d %s\r\n", 452, in_data->msg ? in_data->msg :
-	       "Message denied temporarily");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 452, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 452, "Message denied temporarily");
+    }
     break;
   default:
-    retval = smtpd_queue(r, in_data);
+    retval = smtpd_queue(scr, in_data);
   }
 
-  smtpd_reset_transaction(r);
+  smtpd_reset_transaction(scr);
 
  cleanup:
   apr_file_close(tfp);
@@ -491,22 +556,22 @@
 }
 
 HANDLER_DECLARE(rset) {
-  smtpd_reset_transaction(r);
+  smtpd_reset_transaction(scr);
 
-  ap_rprintf(r, "%d %s\r\n", 250, "Ok");
+  smtpd_respond_oneline(scr, 250, "Ok");
 
   return 250;
 }
 
 HANDLER_DECLARE(noop) {
-  ap_rprintf(r, "%d %s\r\n", 250, "Ok");
+  smtpd_respond_oneline(scr, 250, "Ok");
 
   return 250;
 }
 
 HANDLER_DECLARE(quit) {
-  if (smtpd_run_quit(r, in_data) != SMTPD_DONE) {
-    ap_rprintf(r, "%d %s\r\n", 221, "Bye");
+  if (smtpd_run_quit(scr, in_data) != SMTPD_DONE) {
+    smtpd_respond_oneline(scr, 221, "Bye");
   }
   /* zero to disconnect */
   return 0;
@@ -514,22 +579,32 @@
 
 HANDLER_DECLARE(vrfy) {
  
-  switch(smtpd_run_vrfy(r, in_data, buffer)) {
+  switch(smtpd_run_vrfy(scr, in_data, buffer)) {
   case SMTPD_DONE:
     return 1;
   case SMTPD_DONE_DISCONNECT:
     return 0;
   case SMTPD_DENY:
-    ap_rprintf(r, "%d %s\r\n", 554, in_data->msg ? in_data->msg :
-	       "Address denied");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 554, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 554, "Address denied");
+    }
     return 554;
   case SMTPD_OK: /* user is okay */
-    ap_rprintf(r, "%d %s\r\n", 250, in_data->msg ? in_data->msg :
-	       "Address okay");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 250, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 250, "Address okay");
+    }
     return 250;
   default:
-    ap_rprintf(r, "%d %s\r\n", 252, in_data->msg ? in_data->msg :
-	       "Address seems fine, but we might not accept it.");
+    if (in_data->msgs) {
+      smtpd_respond_multiline(scr, 252, in_data->msgs);
+    } else {
+      smtpd_respond_oneline(scr, 252,
+			    "Address seems fine, but we might not accept it.");
+    }
     return 252;
   }
 }



Re: svn commit: r235759 - in /httpd/mod_smtpd/trunk: mod_smtpd.h smtp.h smtp_core.c smtp_protocol.c

Posted by Rian Hunter <ri...@mit.edu>.
On Mon, 2005-08-22 at 23:37, Garrett Rooney wrote:
> > Modified: httpd/mod_smtpd/trunk/mod_smtpd.h
> > URL: http://svn.apache.org/viewcvs/httpd/mod_smtpd/trunk/mod_smtpd.h?rev=235759&r1=235758&r2=235759&view=diff
> > ==============================================================================
> > --- httpd/mod_smtpd/trunk/mod_smtpd.h (original)
> > +++ httpd/mod_smtpd/trunk/mod_smtpd.h Mon Aug 22 10:22:27 2005
> > @@ -63,7 +63,7 @@
> >      SMTPD_STATE_GOT_HELO,
> >      SMTPD_STATE_GOT_MAIL,
> >      SMTPD_STATE_GOT_RCPT
> > -  } smtpd_request_state;
> > +  } smtpd_trans_state;
> >  
> >    typedef enum {
> >      SMTPD_PROTOCOL_SMTP,
> > @@ -72,78 +72,90 @@
> >  
> >    typedef struct smtpd_return_data {
> >      apr_pool_t *p;
> > -    char *msg;
> > +    /* list of messages
> > +       null terminated */
> > +    char **msgs;
> 
> Why is this a char **?  It seems kind of error prone to require the 
> module programmer to allocate and manage that array correctly, this 
> feels like another case where an APR array would potentially simplify 
> things.

Good point. I'll do this. Originally there was a reason why I didn't
want it to be a APR array, but that reason doesn't apply anymore.

> >    } smtpd_return_data;
> >  
> > -  typedef struct smtpd_request_rec {
> > +  typedef struct smtpd_trans_rec {
> >      apr_pool_t *p;
> > -    conn_rec *c;
> > -    server_rec *s;
> > -
> > -    /* spooled data file pointer */
> > -    apr_file_t *tfp;
> >  
> >      /* where are we in the current transaction */
> > -    smtpd_request_state state_vector;
> > +    smtpd_trans_state state_vector;
> 
> Out of curiosity, why is this called a "state_vector", there's only one 
> element, so it doesn't feel much like a vector to me (at least not in 
> the std::vector sense of the word from c++).

Well this is another thing from old code. Originally the state was a bit
vector, but now it's just incremental values so the name should
certainly change.

> > +apr_status_t
> > +smtpd_getline(smtpd_conn_rec *scr, char *data, apr_size_t dlen)
> > +{
> > +  apr_status_t rc;
> > +  apr_bucket *e;
> > +  const char *str;
> > +  char *pos, *last_char = data;
> > +  apr_size_t len, bytes_handled = 0;
> > +
> > +  while (1) {
> > +    rc = ap_get_brigade(scr->c->input_filters, scr->bb_in, AP_MODE_GETLINE,
> > +			APR_BLOCK_READ, 0);
> > +    if (rc != APR_SUCCESS) return rc;
> 
> Putting the body of the if statement on the same line as the if is kind 
> of sketchy IMO.  In particular it means that when you're single stepping 
> through the code in a debugger it's impossible to easily tell if the 
> statement is true or not based on the fact that you stepped onto that line.
> 
> Also, APR_SUCCESS is defined to be zero, so you can write that in less 
> characters by saying:
> 
> if (rc)
>    return rc;
> 
> Although that's just a style thing.

I'll make a seperate change for this.

> > +    while(!APR_BRIGADE_EMPTY(scr->bb_in)) {
> > +      e = APR_BRIGADE_FIRST(scr->bb_in);
> > +      
> > +      rc = apr_bucket_read(e, &str, &len, APR_BLOCK_READ);
> > +      if (rc != APR_SUCCESS) return rc;      
> > +      apr_bucket_delete(e);
> > +           
> > +      if (len == 0) continue;
> > + 
> > +      /* Would this overrun our buffer?  If so, we'll die. */
> > +      if (dlen < bytes_handled + len) {
> > +	if (data) {
> > +	  /* ensure this string is NUL terminated */
> > +	  if (bytes_handled > 0) {
> > +	    data[bytes_handled-1] = '\0';
> > +	  }
> > +	  else {
> > +	    data[0] = '\0';
> > +	  }
> > +	}
> > +	return APR_ENOSPC;
> > +      }
> 
> The indentation in this section seems kind of screwed up.  Speaking of 
> indentation, none of the mod_smtpd code follows the httpd style 
> guidelines, if that's going to eventually change it might be good to get 
> it out of the way sooner rather than later, as those kind of changes 
> tend to obscure the revision history of the code.  See 
> http://httpd.apache.org/dev/styleguide.html for details about the style 
> guidelines.

I'll make a completely independent commit for style problems.

> 
> > -  apr_socket_t *csd=((core_net_rec *)r->input_filters->ctx)->client_socket;
> > +  //  r->request_config  = ap_create_request_config(r->pool);
> 
> C++ style comment.  This will break some C compilers.  Plus, it's an 
> example of leaving old code commented out in a function, which is a bad 
> habit to get into.  If the code has a reason to be there, at least leave 
> a comment as to why, if not, just kill it.
> 
> > -  r = smtpd_create_request(c);
> > -  ap_update_child_status(r->connection->sbh, SERVER_BUSY_KEEPALIVE, r);
> > +  scr = smtpd_create_conn_rec(c);
> > +  //  ap_update_child_status(scr->c->sbh, SERVER_BUSY_KEEPALIVE, r);
> 
> Another case of that...

I left that code in there just in case others knew how to change emulate
the same behavior. It was more like a note, but I should have definitely
commented on why it was there.

> 
> -garrett

Thanks a lot! In the future I'll be a lot more careful about code I
commit.
-rian


Re: svn commit: r235759 - in /httpd/mod_smtpd/trunk: mod_smtpd.h smtp.h smtp_core.c smtp_protocol.c

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
soc-rian@apache.org wrote:
> Author: soc-rian
> Date: Mon Aug 22 10:22:27 2005
> New Revision: 235759
> 
> URL: http://svn.apache.org/viewcvs?rev=235759&view=rev
> Log:
> mod_smtpd overhaul:
> 1. new structs: smtpd_conn_rec, smtpd_trans_rec
> 2. different i/o API: smtpd_getline, smtpd_respond_oneline,
>    smtpd_respond_multiline.

A quick review.  Note that all of this stuff would be much easier to 
review and understand if there were some examples of how to write 
modules that plug into it and make it do something.  As I don't have 
access to that, all of my comments are kind of theoretical, but 
hopefully they can still be helpful.

-garrett

> Modified:
>     httpd/mod_smtpd/trunk/mod_smtpd.h
>     httpd/mod_smtpd/trunk/smtp.h
>     httpd/mod_smtpd/trunk/smtp_core.c
>     httpd/mod_smtpd/trunk/smtp_protocol.c
> 
> Modified: httpd/mod_smtpd/trunk/mod_smtpd.h
> URL: http://svn.apache.org/viewcvs/httpd/mod_smtpd/trunk/mod_smtpd.h?rev=235759&r1=235758&r2=235759&view=diff
> ==============================================================================
> --- httpd/mod_smtpd/trunk/mod_smtpd.h (original)
> +++ httpd/mod_smtpd/trunk/mod_smtpd.h Mon Aug 22 10:22:27 2005
> @@ -63,7 +63,7 @@
>      SMTPD_STATE_GOT_HELO,
>      SMTPD_STATE_GOT_MAIL,
>      SMTPD_STATE_GOT_RCPT
> -  } smtpd_request_state;
> +  } smtpd_trans_state;
>  
>    typedef enum {
>      SMTPD_PROTOCOL_SMTP,
> @@ -72,78 +72,90 @@
>  
>    typedef struct smtpd_return_data {
>      apr_pool_t *p;
> -    char *msg;
> +    /* list of messages
> +       null terminated */
> +    char **msgs;

Why is this a char **?  It seems kind of error prone to require the 
module programmer to allocate and manage that array correctly, this 
feels like another case where an APR array would potentially simplify 
things.

>    } smtpd_return_data;
>  
> -  typedef struct smtpd_request_rec {
> +  typedef struct smtpd_trans_rec {
>      apr_pool_t *p;
> -    conn_rec *c;
> -    server_rec *s;
> -
> -    /* spooled data file pointer */
> -    apr_file_t *tfp;
>  
>      /* where are we in the current transaction */
> -    smtpd_request_state state_vector;
> +    smtpd_trans_state state_vector;

Out of curiosity, why is this called a "state_vector", there's only one 
element, so it doesn't feel much like a vector to me (at least not in 
the std::vector sense of the word from c++).


> +apr_status_t
> +smtpd_getline(smtpd_conn_rec *scr, char *data, apr_size_t dlen)
> +{
> +  apr_status_t rc;
> +  apr_bucket *e;
> +  const char *str;
> +  char *pos, *last_char = data;
> +  apr_size_t len, bytes_handled = 0;
> +
> +  while (1) {
> +    rc = ap_get_brigade(scr->c->input_filters, scr->bb_in, AP_MODE_GETLINE,
> +			APR_BLOCK_READ, 0);
> +    if (rc != APR_SUCCESS) return rc;

Putting the body of the if statement on the same line as the if is kind 
of sketchy IMO.  In particular it means that when you're single stepping 
through the code in a debugger it's impossible to easily tell if the 
statement is true or not based on the fact that you stepped onto that line.

Also, APR_SUCCESS is defined to be zero, so you can write that in less 
characters by saying:

if (rc)
   return rc;

Although that's just a style thing.

> +    while(!APR_BRIGADE_EMPTY(scr->bb_in)) {
> +      e = APR_BRIGADE_FIRST(scr->bb_in);
> +      
> +      rc = apr_bucket_read(e, &str, &len, APR_BLOCK_READ);
> +      if (rc != APR_SUCCESS) return rc;      
> +      apr_bucket_delete(e);
> +           
> +      if (len == 0) continue;
> + 
> +      /* Would this overrun our buffer?  If so, we'll die. */
> +      if (dlen < bytes_handled + len) {
> +	if (data) {
> +	  /* ensure this string is NUL terminated */
> +	  if (bytes_handled > 0) {
> +	    data[bytes_handled-1] = '\0';
> +	  }
> +	  else {
> +	    data[0] = '\0';
> +	  }
> +	}
> +	return APR_ENOSPC;
> +      }

The indentation in this section seems kind of screwed up.  Speaking of 
indentation, none of the mod_smtpd code follows the httpd style 
guidelines, if that's going to eventually change it might be good to get 
it out of the way sooner rather than later, as those kind of changes 
tend to obscure the revision history of the code.  See 
http://httpd.apache.org/dev/styleguide.html for details about the style 
guidelines.


> -  apr_socket_t *csd=((core_net_rec *)r->input_filters->ctx)->client_socket;
> +  //  r->request_config  = ap_create_request_config(r->pool);

C++ style comment.  This will break some C compilers.  Plus, it's an 
example of leaving old code commented out in a function, which is a bad 
habit to get into.  If the code has a reason to be there, at least leave 
a comment as to why, if not, just kill it.

> -  r = smtpd_create_request(c);
> -  ap_update_child_status(r->connection->sbh, SERVER_BUSY_KEEPALIVE, r);
> +  scr = smtpd_create_conn_rec(c);
> +  //  ap_update_child_status(scr->c->sbh, SERVER_BUSY_KEEPALIVE, r);

Another case of that...

-garrett