[prev in list] [next in list] [prev in thread] [next in thread] 

List:       inn-workers
Subject:    Re: count single \r or \n as \r\n while checking line length against
From:       Julien_ÉLIE <julien () trigofacile ! com>
Date:       2010-10-24 14:57:13
Message-ID: F50FBC2559954E22B5305846C0D7E4FA () Iulius
[Download RAW message or body]

Hi Florian,

> please find below the complete patch. I have been running with it on our
> feeder for a month now without issues.

OK, thanks a lot!



> Also, I looked into adding a test for nnrpd similar to artparse.t for
> innd, but it's not so simple; ARTpost() in nnrpd/post.c probably needs
> to be rewritten / refactored for it to be feasible. Have you ever
> thought about that?

There is a lot of work to do for the test suite...  We should probably
first update it to use the last version of C TAP Harness:
    http://www.eyrie.org/~eagle/software/c-tap-harness/
but it takes time (everything needs being updated...).
And then add more tests; there are many tests that could be added!



> @@ -164,12 +164,21 @@ TrimSpaces(char *p)
>  static char *
>  NextHeader(char *p)
>  {
> -  for ( ; (p = strchr(p, '\n')) != NULL; p++) {
> +  char *q;
> +
> +  for (q = p; (p = strchr(p, '\n')) != NULL; p++) {
> +    if (p - q > MAXARTLINELENGTH) {
> +      strlcpy(Error, "Header line too long",
> +              sizeof(Error));
> +      return NULL;
> +    }
>      if (ISWHITE(p[1]))
>        continue;
>      *p = '\0';
>      return p + 1;
>    }
> +  strlcpy(Error, "Article has no body -- just headers",
> +          sizeof(Error));
>    return NULL;
>  }

That's not exact:
 * "p - q" is not the lenght of a line;
 * q is not updated when there is a continuation line.


Suggestion:

--- nnrpd/post.c        (révision 9130)
+++ nnrpd/post.c        (copie de travail)
@@ -168,12 +168,25 @@
 static char *
 NextHeader(char *p)
 {
-    for ( ; (p = strchr(p, '\n')) != NULL; p++) {
-       if (ISWHITE(p[1]))
-           continue;
+    char *q;
+    for (q = p; (p = strchr(p, '\n')) != NULL; p++) {
+        /* Note that '\r\n' has temporarily been internally replaced by '\n'.
+         * Therefore, the count takes it into account. */
+        if (p - q + 2 > MAXARTLINELENGTH) {
+            strlcpy(Error, "Header line too long",
+                    sizeof(Error));
+            return NULL;
+        }
+        /* Check if there is another line for the header. */
+        if (ISWHITE(p[1])) {
+            q = p + 1;
+            continue;
+        }
        *p = '\0';
        return p + 1;
     }
+    strlcpy(Error, "Article has no body -- just headers",
+            sizeof(Error));
     return NULL;
 }



Is it OK for you?
The patch looks fine with that correction and I would be happy
to commit it.  Thanks!

-- 
Julien ÉLIE

« Ad iura renunciata, non datur regressus. » 

_______________________________________________
inn-workers mailing list
inn-workers@lists.isc.org
https://lists.isc.org/mailman/listinfo/inn-workers

[prev in list] [next in list] [prev in thread] [next in thread]