diff --git a/review/pre-2.5-brian.review b/review/pre-2.5-brian.review
index ec326e81..dc9f5802 100644
--- a/review/pre-2.5-brian.review
+++ b/review/pre-2.5-brian.review
@@ -2904,5 +2904,217 @@ static void dummy_free_func(void *data) {}
item.resolution.label.validNeedsfixing
item.status.label.open
+
+
+ 2008-01-09 :: 14:08:14:025 GMT-08:00
+ 2008-01-09 :: 14:08:38:129 GMT-08:00
+
+ brian
+ brian
+ apache2/msc_logging.c
+ item.type.label.suggestion
+ item.severity.label.trivial
+ Spelling.
+ throught
+
+
+ item.resolution.label.validNeedsfixing
+ item.status.label.open
+
+
+
+ 2008-01-09 :: 14:21:32:782 GMT-08:00
+ 2008-01-09 :: 14:22:32:803 GMT-08:00
+
+ brian
+ brian
+ apache2/msc_geo.h
+ item.type.label.suggestion
+ item.severity.label.trivial
+ This value may not be portable based on endianness. The algorithm compares it to the IP address as int in host order.
+ #define GEO_COUNTRY_OFFSET 0xffff00
+
+
+ item.resolution.label.validNeedsfixing
+ item.status.label.open
+
+
+
+ 2008-01-09 :: 14:31:25:720 GMT-08:00
+ 2008-01-09 :: 14:45:20:612 GMT-08:00
+
+ brian
+ brian
+ apache2/msc_geo.c
+ item.type.label.suggestion
+ item.severity.label.trivial
+ Check portability due to endianness. It seems that the DB values are assumed to be in host order. Perhaps the compiled DB is little-endian and we need to compensate?
+
+
+
+ item.resolution.label.validNeedsfixing
+ item.status.label.open
+
+
+
+ 2008-01-09 :: 15:28:14:563 GMT-08:00
+ 2008-01-09 :: 15:28:24:203 GMT-08:00
+
+ brian
+ brian
+ apache2/msc_multipart.c
+ item.type.label.irrelevant
+ item.severity.label.trivial
+ Remove code?
+ #if 0
+static char *multipart_construct_filename(modsec_rec *msr) {
+ char c, *p, *q = msr->mpd->mpp->filename;
+ char *filename;
+
+ /* find the last backward slash and consider the
+ * filename to be only what's right from it
+ */
+ p = strrchr(q, '\\');
+ if (p != NULL) q = p + 1;
+
+ /* do the same for the forward slash */
+ p = strrchr(q, '/');
+ if (p != NULL) q = p + 1;
+
+ /* allow letters, digits and dots, replace
+ * everything else with underscores
+ */
+ p = filename = apr_pstrdup(msr->mp, q);
+ while((c = *p) != 0) {
+ if (!( isalnum(c)||(c == '.') )) *p = '_';
+ p++;
+ }
+
+ return filename;
+}
+#endif
+
+
+ item.resolution.label.validNeedsfixing
+ item.status.label.open
+
+
+
+ 2008-01-09 :: 15:35:50:978 GMT-08:00
+ 2008-01-09 :: 15:49:54:128 GMT-08:00
+
+ brian
+ brian
+ apache2/msc_multipart.c
+ item.type.label.programLogic
+ item.severity.label.trivial
+ The multipart C-D header is case insensitive (rfc 2183), so we should probably use strncasecmp() here.
+ /* accept only what we understand */
+ if (strncmp(c_d_value, "form-data", 9) != 0) {
+ return -1;
+ }
+
+
+ item.resolution.label.validNeedsfixing
+ item.status.label.open
+
+
+
+ 2008-01-09 :: 16:05:05:601 GMT-08:00
+ 2008-01-09 :: 16:07:17:436 GMT-08:00
+
+ brian
+ brian
+ apache2/msc_multipart.c
+ item.type.label.suggestion
+ item.severity.label.trivial
+ This allows for a name of "", so maybe check for name[0] == '\0' and return an error. Although we catch this later on as an unrecognized name and return -10.
+ start = p;
+ while((*p != '\0')&&(*p != '=')&&(*p != '\t')&&(*p != ' ')) p++;
+ if (*p == '\0') return -4;
+
+ name = apr_pstrmemdup(msr->mp, start, (p - start));
+
+
+ item.resolution.label.validNeedsfixing
+ item.status.label.open
+
+
+
+ 2008-01-09 :: 16:14:36:307 GMT-08:00
+ 2008-01-09 :: 17:02:47:324 GMT-08:00
+
+ brian
+ brian
+ apache2/msc_multipart.c
+ item.type.label.suggestion
+ item.severity.label.trivial
+ For a quoted value we just include everything until the end quote. The field values should be US-ASCII 'qtext' or 'quoted-pair' (RFC 822) or escaped using RFC 2047.
+ if (*p == '"') {
+ *t = '\0';
+ break;
+ }
+
+
+ item.resolution.label.validNeedsfixing
+ item.status.label.open
+
+
+
+ 2008-01-09 :: 16:59:12:520 GMT-08:00
+ 2008-01-09 :: 17:00:55:566 GMT-08:00
+
+ brian
+ brian
+ apache2/msc_multipart.c
+ item.type.label.suggestion
+ item.severity.label.trivial
+ I think this is wrong. RFC 822 defines a quoted-string as <"> *(qtext/quoted-pair) <"> and quoted-par being able to quote any CHAR.
+ /* only " and \ can be escaped */
+ if ((*(p + 1) == '"')||(*(p + 1) == '\\')) {
+ p++;
+ }
+ else {
+ /* improper escaping */
+
+ /* We allow for now because IE sends
+ * improperly escaped content and there's
+ * nothing we can do about it.
+ *
+ * return -9;
+ */
+ }
+
+
+ item.resolution.label.validNeedsfixing
+ item.status.label.open
+
+
+
+ 2008-01-09 :: 17:04:30:357 GMT-08:00
+ 2008-01-09 :: 17:05:19:369 GMT-08:00
+
+ brian
+ brian
+ apache2/msc_multipart.c
+ item.type.label.suggestion
+ item.severity.label.trivial
+ RFC 2045 defines an 'attribute' as "ALWAYS case-insensitive", so these should be strncasecmp()
+ if (strcmp(name, "name") == 0) {
+ if (msr->mpd->mpp->name != NULL) return -14;
+ msr->mpd->mpp->name = value;
+
+ if (msr->txcfg->debuglog_level >= 9) {
+ msr_log(msr, 9, "Multipart: Content-Disposition name: %s",
+ log_escape_nq(msr->mp, value));
+ }
+ }
+ else
+ if (strcmp(name, "filename") == 0) {
+
+
+ item.resolution.label.validNeedsfixing
+ item.status.label.open
+