refactor socket EOF handling

handling EOF already at the socket level isn't a very good idea - it
breaks the abstraction, and makes implementing sane semantics hard.

src/drv_imap.c

@@ -114,6 +114,8 @@ typedef struct imap_store {
 
 	/* Used during sequential operations like connect */
 	enum { GreetingPending = 0, GreetingBad, GreetingOk, GreetingPreauth } greeting;
+	int expectBYE; /* LOGOUT is in progress */
+	int expectEOF; /* received LOGOUT's OK or unsolicited BYE */
 	int canceling; /* imap_cancel() is in progress */
 	union {
 		void (*imap_open)( store_t *srv, void *aux );
@@ -674,7 +676,7 @@ parse_imap_list( imap_store_t *ctx, char **sp, parse_list_state_t *sts )
 {
 	list_t *cur, **curp;
 	char *s = *sp, *d, *p;
-	int bytes;
+	int n, bytes;
 	char c;
 
 	assert( sts );
@@ -724,7 +726,13 @@ parse_imap_list( imap_store_t *ctx, char **sp, parse_list_state_t *sts )
 			s[cur->len] = 0;
 
 		  getbytes:
-			bytes -= socket_read( &ctx->conn, s, bytes );
+			n = socket_read( &ctx->conn, s, bytes );
+			if (n < 0) {
+			  badeof:
+				error( "IMAP error: unexpected EOF from %s\n", ctx->conn.name );
+				goto bail;
+			}
+			bytes -= n;
 			if (bytes > 0)
 				goto postpone;
 
@@ -738,6 +746,8 @@ parse_imap_list( imap_store_t *ctx, char **sp, parse_list_state_t *sts )
 		  getline:
 			if (!(s = socket_read_line( &ctx->conn )))
 				goto postpone;
+			if (s == (void *)~0)
+				goto badeof;
 			if (DFlags & VERBOSE) {
 				printf( "%s%s\n", ctx->label, s );
 				fflush( stdout );
@@ -1218,6 +1228,12 @@ imap_socket_read( void *aux )
 		}
 		if (!(cmd = socket_read_line( &ctx->conn )))
 			return;
+		if (cmd == (void *)~0) {
+			if (!ctx->expectEOF)
+				error( "IMAP error: unexpected EOF from %s\n", ctx->conn.name );
+			/* A clean shutdown sequence ends with bad_callback as well (see imap_cleanup()). */
+			break;
+		}
 		if (DFlags & VERBOSE) {
 			printf( "%s%s\n", ctx->label, cmd );
 			fflush( stdout );
@@ -1250,12 +1266,14 @@ imap_socket_read( void *aux )
 					goto dogreet;
 				}
 			} else if (!strcmp( "BYE", arg )) {
-				if (ctx->conn.state != SCK_CLOSING) {
+				if (!ctx->expectBYE) {
-					ctx->conn.state = SCK_CLOSING;
 					ctx->greeting = GreetingBad;
 					error( "IMAP error: unexpected BYE response: %s\n", cmd );
-				}
 					/* We just wait for the server to close the connection now. */
+					ctx->expectEOF = 1;
+				} else {
+					/* We still need to wait for the LOGOUT's tagged OK. */
+				}
 			} else if (ctx->greeting == GreetingPending) {
 				error( "IMAP error: bogus greeting response %s\n", arg );
 				break;
@@ -1470,7 +1488,7 @@ imap_cleanup( void )
 	for (ctx = unowned; ctx; ctx = nctx) {
 		nctx = ctx->next;
 		set_bad_callback( ctx, (void (*)(void *))imap_cancel_store, ctx );
-		((imap_store_t *)ctx)->conn.state = SCK_CLOSING;
+		((imap_store_t *)ctx)->expectBYE = 1;
 		imap_exec( (imap_store_t *)ctx, 0, imap_cleanup_p2, "LOGOUT" );
 	}
 }
@@ -1481,6 +1499,8 @@ imap_cleanup_p2( imap_store_t *ctx,
 {
 	if (response == RESP_NO)
 		imap_cancel_store( &ctx->gen );
+	else if (response == RESP_OK)
+		ctx->expectEOF = 1;
 }
 
 /******************* imap_open_store *******************/

src/socket.c

@@ -43,6 +43,15 @@
 # include <openssl/x509v3.h>
 #endif
 
+enum {
+	SCK_CONNECTING,
+#ifdef HAVE_LIBSSL
+	SCK_STARTTLS,
+#endif
+	SCK_READY,
+	SCK_EOF
+};
+
 static void
 socket_fail( conn_t *conn )
 {
@@ -67,11 +76,12 @@ ssl_return( const char *func, conn_t *conn, int ret )
 	case SSL_ERROR_SSL:
 		if (!(err = ERR_get_error())) {
 			if (ret == 0) {
-				if (conn->state != SCK_CLOSING)
+				/* Callers take the short path out, so signal higher layers from here. */
-					error( "Socket error: secure %s %s: unexpected EOF\n", func, conn->name );
+				conn->state = SCK_EOF;
-			} else {
+				conn->read_callback( conn->callback_aux );
-				sys_error( "Socket error: secure %s %s", func, conn->name );
+				return 0;
 			}
+			sys_error( "Socket error: secure %s %s", func, conn->name );
 		} else {
 			error( "Socket error: secure %s %s: %s\n", func, conn->name, ERR_error_string( err, 0 ) );
 		}
@@ -582,10 +592,9 @@ do_read( conn_t *sock, char *buf, int len )
 			sys_error( "Socket error: read from %s", sock->name );
 			socket_fail( sock );
 		} else if (!n) {
-			if (sock->state != SCK_CLOSING)
+			/* EOF. Callers take the short path out, so signal higher layers from here. */
-				error( "Socket error: read from %s: unexpected EOF\n", sock->name );
+			sock->state = SCK_EOF;
-			socket_fail( sock );
+			sock->read_callback( sock->callback_aux );
-			return -1;
 		}
 	}
 
@@ -656,6 +665,8 @@ int
 socket_read( conn_t *conn, char *buf, int len )
 {
 	int n = conn->bytes;
+	if (!n && conn->state == SCK_EOF)
+		return -1;
 	if (n > len)
 		n = len;
 	memcpy( buf, conn->buf + conn->offset, n );
@@ -680,6 +691,8 @@ socket_read_line( conn_t *b )
 			memmove( b->buf, b->buf + b->offset, b->bytes );
 			b->offset = 0;
 		}
+		if (b->state == SCK_EOF)
+			return (void *)~0;
 		return 0;
 	}
 	n = p + 1 - s;

src/socket.h

@@ -29,15 +29,6 @@
 #include <zlib.h>
 #endif
 
-enum {
-	SCK_CONNECTING,
-#ifdef HAVE_LIBSSL
-	SCK_STARTTLS,
-#endif
-	SCK_READY,
-	SCK_CLOSING
-};
-
 #ifdef HAVE_LIBSSL
 typedef struct ssl_st SSL;
 typedef struct ssl_ctx_st SSL_CTX;