INN commit: trunk/nnrpd (article.c group.c)

INN Commit Russ_Allbery at isc.org
Sun Aug 31 15:56:50 UTC 2008


    Date: Sunday, August 31, 2008 @ 08:56:50
  Author: iulius
Revision: 7995

Create a separate function CMDisrange() to check whether a given
string is a valid range.
Change the order of the checks on arguments in order to answer 501
and not 4xx when the syntax is wrong.

Modified:
  trunk/nnrpd/article.c
  trunk/nnrpd/group.c

-----------+
 article.c |   51 +++++++++++++++++++++++++++++++++++----------------
 group.c   |   17 ++++++++++++-----
 2 files changed, 47 insertions(+), 21 deletions(-)

Modified: article.c
===================================================================
--- article.c	2008-08-31 15:05:31 UTC (rev 7994)
+++ article.c	2008-08-31 15:56:50 UTC (rev 7995)
@@ -61,6 +61,7 @@
 static int		queued_iov = 0;
 
 bool CMDgetrange(int ac, char *av[], ARTRANGE *rp, bool *DidReply);
+bool CMDisrange(char *string);
 
 static void
 PushIOvHelper(struct iovec* vec, int* countp)
@@ -648,6 +649,12 @@
 	return;
     }
 
+    /* Check whether the second argument (a number) is valid. */
+    if (ac == 2 && strspn(av[1], "0123456789") != strlen(av[1])) {
+        Reply("%d Syntax error in article number\r\n", NNTP_ERR_SYNTAX);
+        return;
+    }
+
     /* Trying to read. */
     if (GRPcount == 0) {
 	Reply("%s\r\n", ARTnotingroup);
@@ -664,10 +671,7 @@
 	tart=ARTnumber;
     }
     else {
-	if (strspn(av[1], "0123456789") != strlen(av[1])) {
-	    Reply("%d Syntax error in article number\r\n", NNTP_ERR_SYNTAX);
-	    return;
-	}
+        /* We have already checked that the article number is an integer. */
 	strlcpy(buff, av[1], sizeof(buff));
 	tart=(ARTNUM)atol(buff);
     }
@@ -772,7 +776,6 @@
 CMDgetrange(int ac, char *av[], ARTRANGE *rp, bool *DidReply)
 {
     char		*p;
-    bool                dashfound = false;
 
     *DidReply = false;
 
@@ -787,17 +790,11 @@
         return true;
     }
 
-    /* Check the syntax:  only allow digits and *one* "-". */
-    for (p = av[1]; *p; p++) {
-        if (*p == '-' && !dashfound) {
-            dashfound = true;
-            continue;
-        }
-        if (!CTYPE(isdigit, *p)) {
-            Reply("%d Syntax error\r\n", NNTP_ERR_SYNTAX);
-            *DidReply = true;
-            return false;
-        }
+    /* Check the syntax. */
+    if (!CMDisrange(av[1])) {
+        Reply("%d Syntax error\r\n", NNTP_ERR_SYNTAX);
+        *DidReply = true;
+        return false;
     }
 
     /* Got just a single number? */
@@ -830,6 +827,28 @@
 
 
 /*
+**  Return true if the provided string is a valid range.
+*/
+bool
+CMDisrange(char *string)
+{
+    bool dashfound = false;
+
+    /* Check the syntax:  only allow digits and *one* "-". */
+    for (; *string; string++) {
+        if (*string == '-' && !dashfound) {
+            dashfound = true;
+            continue;
+        }
+        if (!CTYPE(isdigit, *string))
+            return false;
+    }
+    
+    return true;
+}
+
+
+/*
 **  Apply virtual hosting to an Xref field.
 */
 static char *

Modified: group.c
===================================================================
--- group.c	2008-08-31 15:05:31 UTC (rev 7994)
+++ group.c	2008-08-31 15:56:50 UTC (rev 7995)
@@ -11,6 +11,7 @@
 #include "inn/ov.h"
 
 extern bool CMDgetrange(int ac, char *av[], ARTRANGE *rp, bool *DidReply);
+extern bool CMDisrange(char *string);
 
 /*
 **  Change to or list the specified newsgroup.  If invalid, stay in the old
@@ -54,11 +55,17 @@
 	group = xstrdup(av[1]);
     }
 
+    /* Check whether the second argument is valid. */
+    if (ac == 3 && !CMDisrange(av[2])) {
+        Reply("%d Syntax error\r\n", NNTP_ERR_SYNTAX);
+        return;
+    }
+
     /* FIXME: Temporarily work around broken API. */
     if (!OVgroupstats(group, &low, &high, &count, NULL)) {
-	Reply("%s %s\r\n", NOSUCHGROUP, group);
-	free(group);
-	return;
+        Reply("%s %s\r\n", NOSUCHGROUP, group);
+        free(group);
+        return;
     }
     ARTlow = low;
     ARThigh = high;
@@ -169,9 +176,9 @@
         /* Parse the range. */
         if (ac == 3) {
             /* CMDgetrange() expects av[1] to contain the range.
-             * It is av[2] for LISTGROUP. */
+             * It is av[2] for LISTGROUP.
+             * We already know that GRPcur exists. */
             if (!CMDgetrange(ac, av + 1, &range, &DidReply)) {
-                /* It cannot happen because GRPcur exists. */
                 if (DidReply) {
                     free(group);
                     return;



More information about the inn-committers mailing list