Re: Tin & Synchronet a Bug ?

On Thu, Nov 17, 2005 at 11:03:36AM +0100, Urs Janßen wrote:
> > Nov 16 17:12:33 ice synchronet: srvc 0011 NNTP cmd: MODE READER
> > Nov 16 17:12:33 ice synchronet: srvc 0011 NNTP rsp: 200 Hello, you can post
> > Nov 16 17:12:33 ice synchronet: srvc 0011 NNTP cmd: XOVER
> > Nov 16 17:12:33 ice synchronet: srvc 0011 NNTP rsp: 224 Overview 
> > information follows
> 
> this is the real bug, the RFC says that the server should respond with 412
> if XOVER is requested without beeing in a newsgroup (tin does this to see if
> the server knows the XOVER command). instead of returning a _single_-line
> error-message the server gives a multiline response when this is not
> expected by the client. as tin only read the first response line and leaves
> all other lines on the input queqe, they will be fetched with the response to
> the next command -> total confusion. 
> 
> again, the bug is not inside tin, but inside the synchronet server, RFC 2980
> clearly says:
> 
> | A news group must have been selected earlier, else a 412
> | error response is returned.  If no articles are in the range
> | specified, a 420 error response is returned by the server.  A 502
> | response will be returned if the client only has permission to
> | transfer articles.
> 

below is a patch for tin to handle Synchronets broken XOVER response.
the nntp-part of the server seems to have a lot of other bugs or at least
missing features (e.g. missing extended LIST commands (LIST NEWSGROUPS)),
...

--- nntplib.c.o	2005-11-17 13:44:04.835572717 +0100
+++ nntplib.c	2005-11-17 13:52:35.904775699 +0100
@@ -1401,10 +1401,28 @@
 	 * We have to check that we _don't_ get an ERR_COMMAND
 	 */
 	if (nntp_caps.type == NO) {
-		for (i = 0; i < 2; i++) {
-			if (!nntp_command(&xover_cmds[i], ERR_COMMAND, NULL, 0)) {
-				nntp_caps.over_cmd = &xover_cmds[i];
-				break;
+		int j = 0;
+
+		for (i = 0; i < 2 && j >= 0; i++) {
+			j = new_nntp_command(&xover_cmds[i], ERR_NCING, line, sizeof(line));
+			switch (j) {
+				case ERR_COMMAND:
+					break;
+
+				case 224:	/* unexpected multiline ok, e.g.: Synchronet 3.13b-Linux NNTP Service 1.92 */
+					nntp_caps.over_cmd = &xover_cmds[i];
+#	ifdef DEBUG
+					debug_nntp(&xover_cmds[i], "skipping data");
+#	endif /* DEBUG */
+					while ((linep = tin_fgets(FAKE_NNTP_FP, FALSE)) != NULL)
+						;
+					j = -1;
+					break;
+
+				default:
+					nntp_caps.over_cmd = &xover_cmds[i];
+					j = -1;
+					break;
 			}
 		}
 	} else {
@@ -1413,8 +1431,25 @@
 			 * CAPABILITIES/LIST EXTENSIONS didn't mention OVER or XOVER, try
 			 * XOVER
 			 */
-			if (!nntp_command(xover_cmds, ERR_COMMAND, NULL, 0))
-				nntp_caps.over_cmd = xover_cmds;
+			i = new_nntp_command(xover_cmds, ERR_NCING, line, sizeof(line));
+
+			switch (i) {
+				case ERR_COMMAND:
+					break;
+
+				case 215:	/* unexpected multiline ok, e.g.: Synchronet 3.13b-Linux NNTP Service 1.92 */
+					nntp_caps.over_cmd = xover_cmds;
+#	ifdef DEBUG
+					debug_nntp(xover_cmds, "skipping data");
+#	endif /* DEBUG */
+					while ((linep = tin_fgets(FAKE_NNTP_FP, FALSE)) != NULL)
+						;
+					break;
+
+				default:
+					nntp_caps.over_cmd = xover_cmds;
+					break;
+			}
 		}
 #	if 0 /* unused */
 		if (!nntp_caps.hdr_cmd) {