[Libevent-users] thread-safety bug report with patch

[Tani Hosokawa]

These are four broken functions that didn't make it into the latest 
release -- I noticed that one of them is even marked as not being 
thread-safe.  Maybe I'm unclear as to why that wasn't just changed to a 
stack variable but it seems to me that since it's just used as a 
temporary space, and evhttp_add_header copies the value using strdup(), 
this should be fine.  Without these modifications, under any real load 
you will start seeing really damaged HTTP requests, as chunks of headers 
will be swapped around randomly between threads.

In addition to these, which can be fixed without changing the interface, 
the documentation should probably note that evhttp_hostportfile() is not 
thread-safe.  It's easy enough to fix, but requires a different function 
prototype and is not backwards compatible.

--- http.c.old  2008-02-26 10:03:04.000000000 -0800
+++ http.c      2008-02-26 10:12:06.000000000 -0800
@@ -188,10 +188,8 @@
 #endif

 static const char *
-html_replace(char ch)
+html_replace(char ch,char *buf)
 {
-       static char buf[2];
-
        switch (ch) {
        case '<':
                return "&lt;";
@@ -224,17 +222,19 @@
 char *
 evhttp_htmlescape(const char *html)
 {
+        char scratch_space[2];
+
        int i, new_size = 0, old_size = strlen(html);
        char *escaped_html, *p;

        for (i = 0; i < old_size; ++i)
-               new_size += strlen(html_replace(html[i]));
+               new_size += strlen(html_replace(html[i],scratch_space));

        p = escaped_html = malloc(new_size + 1);
        if (escaped_html == NULL)
                event_err(1, "%s: malloc(%d)", __func__, new_size + 1);
        for (i = 0; i < old_size; ++i) {
-               const char *replaced = html_replace(html[i]);
+               const char *replaced = html_replace(html[i],scratch_space);
                /* this is length checked */
                strcpy(p, replaced);
                p += strlen(replaced);
@@ -308,7 +308,7 @@
 evhttp_make_header_request(struct evhttp_connection *evcon,
     struct evhttp_request *req)
 {
-       static char line[1024];
+       char line[1024];
        const char *method;

        evhttp_remove_header(req->output_headers, "Accept-Encoding");
@@ -380,7 +380,7 @@
 {
        if (evhttp_find_header(headers, "Transfer-Encoding") == NULL &&
            evhttp_find_header(headers, "Content-Length") == NULL) {
-               static char len[12]; /* XXX: not thread-safe */
+               char len[12];
                snprintf(len, sizeof(len), "%ld", content_length);
                evhttp_add_header(headers, "Content-Length", len);
        }
@@ -394,7 +394,7 @@
 evhttp_make_header_response(struct evhttp_connection *evcon,
     struct evhttp_request *req)
 {
-       static char line[1024];
+       char line[1024];
        snprintf(line, sizeof(line), "HTTP/%d.%d %d %s\r\n",
            req->major, req->minor, req->response_code,
            req->response_code_line);
@@ -433,7 +433,7 @@
 void
 evhttp_make_header(struct evhttp_connection *evcon, struct 
evhttp_request *req)
 {
-       static char line[1024];
+       char line[1024];
        struct evkeyval *header;

        /*