socket.c: Return -1 in poll_read if read returns 0.

Currently, bbs_poll_read and bbs_node_poll_read both
return the return value from poll and read unmodified.
However, this can lead to callers misinterpreting the
return value when read returns 0, and this is treated
as a poll timeout, since these cannot be differentiated.

We could expand the return values to differentiate better
(e.g. return -2 for poll failure), but most existing usage
of these APIs already assume a return value of 0 is from
poll and treats negative return values as failure, so just
return -1 if read returns 0 to make everything "just work".
In particular, this now ensures that the SMTP server does not
try to send a "451 timeout" message when a client disconnects.

bbs/socket.c

@@ -1672,16 +1672,26 @@ ssize_t bbs_node_poll_read(struct bbs_node *node, int ms, char *buf, size_t len)
 		return res;
 	}
 	res = bbs_node_read(node, buf, len);
+	if (res <= 0) {
+		/* If read returns 0, return -1, since the connection is dead anyways,
+		 * to reserve a return value of 0 for poll timeout. */
+		return -1;
+	}
 	return res;
 }
 
 ssize_t bbs_poll_read(int fd, int ms, char *buf, size_t len)
 {
 	ssize_t res = bbs_poll(fd, ms);
 	if (res <= 0) {
-		return res;
+		return res - 1;
 	}
 	res = read(fd, buf, len);
+	if (res <= 0) {
+		/* If read returns 0, return -1, since the connection is dead anyways,
+		 * to reserve a return value of 0 for poll timeout. */
+		return -1;
+	}
 	return res;
 }
 

include/node.h

@@ -426,7 +426,7 @@ ssize_t bbs_node_read(struct bbs_node *node, char *buf, size_t len);
  * \param ms for poll
  * \param buf Buffer for data
  * \param len Size of buf
- * \retval Same as poll() and read()
+ * \retval Same as poll() and read(), except -1 is returned if read returns 0
  */
 ssize_t bbs_node_poll_read(struct bbs_node *node, int ms, char *buf, size_t len);
 
@@ -436,7 +436,7 @@ ssize_t bbs_node_poll_read(struct bbs_node *node, int ms, char *buf, size_t len)
  * \param ms for poll
  * \param buf Buffer for data
  * \param len Size of buf
- * \retval Same as poll() and read()
+ * \retval Same as poll() and read(), except -1 is returned if read returns 0
  */
 ssize_t bbs_poll_read(int fd, int ms, char *buf, size_t len);
 

nets/net_smtp.c

@@ -2854,10 +2854,13 @@ static void handle_client(struct smtp_session *smtp, SSL **sslptr)
 			res += 1; /* Convert the res back to a normal one. */
 			if (res == 0) {
 				/* Timeout occured. */
-				/* XXX This also happens if a noncompliant SMTP client sends us more than 1,000 bytes in a single line
-				 * and exhausts our buffer. In this case, bbs_readline returns -1.
-				 * We should probably send a more appropriate error in this event. */
-				smtp_reply(smtp, 451, 4.4.2, "Timeout - closing connection"); /* XXX Should do only if poll returns 0, not if read returns 0 */
+				smtp_reply(smtp, 451, 4.4.2, "Timeout - closing connection");
+			} else if (res == -2) {
+				/* bbs_readline returns -3 on buffer exhaustion (which we've incremented by 1).
+				 * Since our connection state is messed up at this point, the only sane thing
+				 * we can do is print an error and disconnect. */
+				smtp_reply_nostatus(smtp, 550, "Maximum line length exceeded");
+				smtp->failures += 3; /* Semantically, this is a bad client; however, we're just going to disconnect now anyways */
 			}
 			break;
 		}