r17610 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r17609‎ | r17610 | r17611 >
Date:11:43, 13 November 2006
Author:river
Status:old
Tags:
Comment:
when a header is too longer for the fixed buffer, allocate another buffer using a pt_allocator. preserves the efficiency increase while allowing uncommonly long headers to work correctly
Modified paths:
  • /trunk/willow/src/include/whttp_header.h (modified) (history)
  • /trunk/willow/src/willow/whttp_header.cc (modified) (history)

Diff [purge]

Index: trunk/willow/src/include/whttp_header.h
——@@ -29,8 +29,7 @@
3030 #define REQTYPE_OPTIONS 4
3131 #define REQTYPE_INVALID -1
3232
33 -#define MAX_HDRNAM 30
34 -#define MAX_HDRVAL 128
 33+#define HDR_BUFSZ 256
3534
3635 extern struct request_type {
3736 const char *name;
——@@ -38,28 +37,100 @@
3938 int type;
4039 } supported_reqtypes[];
4140
 41+/*
 42+ * A single header. Usually stored inside a header_list. For speed,
 43+ * we assume the length of one header line is no more than HDX_BUFSZ-1
 44+ * characters, and pre-allocate a buffer of that size. In case the
 45+ * header is longer, hr_buffer is ignored, and a new buffer is allocated
 46+ * from the pt_allocator. hr_free==true indicated that the buffer should
 47+ * be freed on destruction.
 48+ */
4249 struct header : freelist_allocator<header> {
43 - header(char const *, size_t, char const *, size_t);
44 - ~header() {}
 50+ /*
 51+ * Construct a new header from the given name and value, which need
 52+ * not be null terminated.
 53+ */
 54+ header( char const *name, size_t namelen,
 55+ char const *value, size_t valuelen);
 56+
 57+ ~header();
4558 header(header const &);
 59+
 60+ /*
 61+ * Assign new values to this header.
 62+ */
 63+ void assign( char const *name, size_t namelen,
 64+ char const *value, size_t valuelen);
4665
47 - char hr_name[MAX_HDRNAM];
48 - char hr_value[MAX_HDRVAL];
 66+ /*
 67+ * Destructively assign the contents of other to *this. The state
 68+ * of other is undefined until assign() or ~header is called.
 69+ */
 70+ void move (header &other);
 71+
 72+ /* return the header's name */
 73+ char const *name (void) const { return hr_name; }
 74+ /* return the header's value */
 75+ char const *value (void) const { return hr_value; }
 76+
 77+private:
 78+ friend struct header_list;
 79+
 80+ char hr_buffer[HDR_BUFSZ];
 81+ char *hr_name; /* name, == hr_buffer or else alloc'd data */
 82+ char *hr_value; /* value */
 83+ size_t hr_allocd; /* size of buffer alloced if any */
 84+ static pt_allocator<char> alloc;
4985 };
5086
 87+/*
 88+ * A list of headers found in a request.
 89+ */
5190 struct header_list {
 91+ /* Construct an empty header list. */
5292 header_list();
5393 ~header_list() {};
5494
55 - void add (char const *, char const *);
56 - void add (char const *, size_t, char const *, size_t);
57 - void append_last (const char *, size_t);
 95+ /*
 96+ * Add a new (header,value) pair to the list. name and value must be
 97+ * nul-terminated.
 98+ */
 99+ void add (char const *name, char const *value);
 100+
 101+ /*
 102+ * As above, but nul terminated is not required.
 103+ */
 104+ void add (char const *name, size_t namelen,
 105+ char const *value, size_t valuelen);
 106+
 107+ /*
 108+ * Append ", app" to the end of the previous header. app need not
 109+ * be terminated.
 110+ */
 111+ void append_last (const char *app, size_t applen);
 112+
 113+ /*
 114+ * Remove the named header from the list.
 115+ */
 116+ void remove (const char *);
 117+
 118+ /*
 119+ * Return a string (allocated with new char[]) containing the request
 120+ * headers in a form suitable for use in an HTTP request or response.
 121+ * The caller is expected to delete the returned value.
 122+ */
58123 char *build (void);
59 - void remove (const char *);
60 - void dump (int);
61 - int undump (int, off_t *);
 124+
 125+ /*
 126+ * Find a specific header in the list. Returns NULL if no such header
 127+ * exists.
 128+ */
62129 struct header *find (const char *name);
63130
 131+private:
 132+ friend struct header_spigot;
 133+ friend struct header_parser;
 134+
64135 header *hl_last;
65136 vector<header, pt_allocator<header> > hl_hdrs;
66137 int hl_len;
Index: trunk/willow/src/willow/whttp_header.cc
——@@ -58,19 +58,77 @@
5959 return REQTYPE_INVALID;
6060 }
6161
 62+pt_allocator<char> header::alloc;
 63+
6264 header::header(char const *n, size_t nlen, char const *v, size_t vlen)
 65+ : hr_allocd(0)
6366 {
 67+ assign(n, nlen, v, vlen);
 68+}
 69+
 70+header::header(header const &other)
 71+ : hr_allocd(0)
 72+{
 73+ assign(other.hr_name, strlen(other.hr_name),
 74+ other.hr_value, strlen(other.hr_value));
 75+}
 76+
 77+void
 78+header::assign(char const *n, size_t nlen, char const *v, size_t vlen)
 79+{
 80+char *buf = hr_buffer;
 81+ if (hr_allocd) {
 82+ alloc.deallocate(hr_name, hr_allocd);
 83+ hr_allocd = false;
 84+ }
 85+
 86+ if ((nlen + vlen + 2) >= HDR_BUFSZ) {
 87+ buf = alloc.allocate(nlen + vlen + 2);
 88+ hr_allocd = nlen + vlen + 2;
 89+ }
 90+
 91+ hr_name = buf;
 92+ hr_value = buf + nlen + 1;
 93+
6494 memcpy(hr_name, n, nlen);
6595 memcpy(hr_value, v, vlen);
6696 hr_name[nlen] = hr_value[vlen] = '\0';
6797 }
6898
69 -header::header(header const &other)
 99+header::~header(void)
70100 {
71 - strcpy(hr_name, other.hr_name);
72 - strcpy(hr_value, other.hr_value);
 101+ if (hr_allocd)
 102+ alloc.deallocate(hr_name, hr_allocd);
73103 }
74104
 105+void
 106+header::move(header &other)
 107+{
 108+ /*
 109+ * The other header is static, just copy the string.
 110+ */
 111+ if (!other.hr_allocd) {
 112+ if (hr_allocd) {
 113+ alloc.deallocate(hr_name, hr_allocd);
 114+ hr_allocd = 0;
 115+ }
 116+
 117+ hr_name = hr_buffer;
 118+ strcpy(hr_name, other.hr_name);
 119+ hr_value = hr_buffer + strlen(hr_name) + 1;
 120+ strcpy(hr_value, other.hr_value);
 121+ return;
 122+ }
 123+
 124+ /*
 125+ * The other header is allocd, steal its buffer.
 126+ */
 127+ hr_allocd = other.hr_allocd;
 128+ hr_name = other.hr_name;
 129+ hr_value = other.hr_value;
 130+ other.hr_allocd = 0;
 131+}
 132+
75133 header_list::header_list()
76134 : hl_len(0)
77135 {
——@@ -96,10 +154,43 @@
97155 {
98156 char const *tmp;
99157 char *n;
100 - assert(hl_last);
101 - strlcat(hl_last->hr_value, ", ", sizeof(hl_last->hr_value));
102 - strncat(hl_last->hr_value, append, min(len, MAX_HDRVAL - strlen(hl_last->hr_value) - 1));
 158+int curnlen, curvlen;
 159+ curnlen = strlen(hl_last->hr_name);
 160+ curvlen = strlen(hl_last->hr_value);
 161+
103162 hl_len += len + 2;
 163+
 164+size_t nbufsz = curnlen + curvlen + 4 + len;
 165+ /*
 166+ * Simple case: not allocated, and new header fits in static buf.
 167+ */
 168+ if (!hl_last->hr_allocd && nbufsz < HDR_BUFSZ) {
 169+ strcat(hl_last->hr_value, ", ");
 170+ strncat(hl_last->hr_value, append, len);
 171+ return;
 172+ }
 173+
 174+ /*
 175+ * New header is too long.
 176+ */
 177+ if ((!hl_last->hr_allocd && nbufsz >= HDR_BUFSZ) ||
 178+ (hl_last->hr_allocd && nbufsz >= hl_last->hr_allocd)) {
 179+ char *nbuf = hl_last->alloc.allocate(nbufsz);
 180+ strcpy(nbuf, hl_last->hr_name);
 181+ sprintf(nbuf + curnlen + 1, "%s, %.*s",
 182+ hl_last->hr_value, len, append);
 183+
 184+ if (hl_last->hr_allocd)
 185+ hl_last->alloc.deallocate(hl_last->hr_name, hl_last->hr_allocd);
 186+
 187+ hl_last->hr_name = nbuf;
 188+ hl_last->hr_value = nbuf + curnlen + 1;
 189+ hl_last->hr_allocd = nbufsz;
 190+ return;
 191+ }
 192+
 193+ /* should not get here */
 194+ abort();
104195 }
105196
106197 void
——@@ -110,8 +201,7 @@
111202 if (strcasecmp(it->hr_name, name))
112203 continue;
113204 hl_len -= strlen(it->hr_name) + strlen(it->hr_value) + 4;
114 - strcpy(it->hr_name, hl_hdrs.rbegin()->hr_name);
115 - strcpy(it->hr_value, hl_hdrs.rbegin()->hr_value);
 205+ it->move(*hl_hdrs.rbegin());
116206 hl_hdrs.pop_back();
117207 return;
118208 }
——@@ -224,11 +314,6 @@
225315 value++;
226316 vlen = rn - value;
227317
228 - if (nlen > MAX_HDRNAM || vlen > MAX_HDRVAL) {
229 - _sink_spigot->sp_cork();
230 - return io::sink_result_error;
231 - }
232 -
233318 if (!strncasecmp(name, "Transfer-Encoding", nlen) && !strncasecmp(value, "chunked", vlen))
234319 _flags.f_chunked = 1;
235320 else if (!strncasecmp(name, "Content-Length", nlen))