Commit 6aef05cf7f75a53bab2b4107b5ba06a16e834275
1 parent
4a075de8
fix rather nasty reentrance bug
as i first wrote the parser i used static valiables to hold the state of the currently parsed request. If a request would spread of multiple reads this would lead to one reqeust messing up the state of another. not those states are part of the parser object itself where they belong.
Showing
3 changed files
with
32 additions
and
24 deletions
1 | +2012-02-10 06:22:39 +0100 Georg Hopp | ||
2 | + | ||
3 | + * fix rather nasty reentrance bug (HEAD, master) | ||
4 | + | ||
1 | 2012-02-10 05:52:50 +0100 Georg Hopp | 5 | 2012-02-10 05:52:50 +0100 Georg Hopp |
2 | 6 | ||
3 | - * fix bug that arose in rewrite of header get and results in an ugly memory leak, as well as no headers would be found any more (HEAD, master) | 7 | + * fix bug that arose in rewrite of header get and results in an ugly memory leak, as well as no headers would be found any more |
4 | 8 | ||
5 | 2012-02-10 00:27:51 +0100 Georg Hopp | 9 | 2012-02-10 00:27:51 +0100 Georg Hopp |
6 | 10 |
@@ -17,10 +17,14 @@ typedef enum e_HttpRequestState { | @@ -17,10 +17,14 @@ typedef enum e_HttpRequestState { | ||
17 | 17 | ||
18 | CLASS(HttpRequestParser) { | 18 | CLASS(HttpRequestParser) { |
19 | char * buffer; | 19 | char * buffer; |
20 | + char * cur_data; | ||
21 | + | ||
20 | size_t buffer_used; | 22 | size_t buffer_used; |
21 | size_t buffer_size; | 23 | size_t buffer_size; |
22 | 24 | ||
23 | HttpRequestQueue request_queue; | 25 | HttpRequestQueue request_queue; |
26 | + HttpRequest cur_request; | ||
27 | + | ||
24 | HttpRequestState state; | 28 | HttpRequestState state; |
25 | }; | 29 | }; |
26 | 30 |
@@ -7,8 +7,8 @@ | @@ -7,8 +7,8 @@ | ||
7 | #include "interface/class.h" | 7 | #include "interface/class.h" |
8 | 8 | ||
9 | 9 | ||
10 | -#define REMAINS(pars,done) \ | ||
11 | - ((pars)->buffer_used - ((done) - (pars)->buffer)) | 10 | +#define REMAINS(pars) \ |
11 | + ((pars)->buffer_used - ((pars)->cur_data - (pars)->buffer)) | ||
12 | 12 | ||
13 | 13 | ||
14 | static | 14 | static |
@@ -43,34 +43,32 @@ void httpRequestParserGetHeader(HttpRequest, char *); | @@ -43,34 +43,32 @@ void httpRequestParserGetHeader(HttpRequest, char *); | ||
43 | void | 43 | void |
44 | httpRequestParserParse(HttpRequestParser this) | 44 | httpRequestParserParse(HttpRequestParser this) |
45 | { | 45 | { |
46 | - static HttpRequest request = NULL; | ||
47 | - static char * data; // static pointer to unprocessed data | ||
48 | char * line; | 46 | char * line; |
49 | int cont = 1; | 47 | int cont = 1; |
50 | 48 | ||
51 | while(cont) { | 49 | while(cont) { |
52 | switch(this->state) { | 50 | switch(this->state) { |
53 | case HTTP_REQUEST_GARBAGE: | 51 | case HTTP_REQUEST_GARBAGE: |
54 | - data = this->buffer; // initialize static pointer | ||
55 | - httpRequestSkip(&data); | ||
56 | - request = new(HttpRequest); | 52 | + this->cur_data = this->buffer; // initialize static pointer |
53 | + httpRequestSkip(&(this->cur_data)); | ||
54 | + this->cur_request = new(HttpRequest); | ||
57 | 55 | ||
58 | this->state = HTTP_REQUEST_START; | 56 | this->state = HTTP_REQUEST_START; |
59 | break; | 57 | break; |
60 | 58 | ||
61 | case HTTP_REQUEST_START: | 59 | case HTTP_REQUEST_START: |
62 | - if (NULL == (line = httpRequestParserGetLine(&data))) { | 60 | + if (NULL == (line = httpRequestParserGetLine(&(this->cur_data)))) { |
63 | cont = 0; | 61 | cont = 0; |
64 | break; | 62 | break; |
65 | } | 63 | } |
66 | 64 | ||
67 | - httpRequestParserGetRequestLine(request, line); | 65 | + httpRequestParserGetRequestLine(this->cur_request, line); |
68 | 66 | ||
69 | this->state = HTTP_REQUEST_REQUEST_LINE_DONE; | 67 | this->state = HTTP_REQUEST_REQUEST_LINE_DONE; |
70 | break; | 68 | break; |
71 | 69 | ||
72 | case HTTP_REQUEST_REQUEST_LINE_DONE: | 70 | case HTTP_REQUEST_REQUEST_LINE_DONE: |
73 | - if (NULL == (line = httpRequestParserGetLine(&data))) { | 71 | + if (NULL == (line = httpRequestParserGetLine(&(this->cur_data)))) { |
74 | cont = 0; | 72 | cont = 0; |
75 | break; | 73 | break; |
76 | } | 74 | } |
@@ -80,19 +78,19 @@ httpRequestParserParse(HttpRequestParser this) | @@ -80,19 +78,19 @@ httpRequestParserParse(HttpRequestParser this) | ||
80 | break; | 78 | break; |
81 | } | 79 | } |
82 | 80 | ||
83 | - httpRequestParserGetHeader(request, line); | 81 | + httpRequestParserGetHeader(this->cur_request, line); |
84 | break; | 82 | break; |
85 | 83 | ||
86 | case HTTP_REQUEST_HEADERS_DONE: | 84 | case HTTP_REQUEST_HEADERS_DONE: |
87 | - httpHeaderSort(request->header, request->nheader); | 85 | + httpHeaderSort(this->cur_request->header, this->cur_request->nheader); |
88 | 86 | ||
89 | { | 87 | { |
90 | char * nbody; | 88 | char * nbody; |
91 | 89 | ||
92 | - if (0 == request->nbody) { | 90 | + if (0 == this->cur_request->nbody) { |
93 | nbody = httpHeaderGet( | 91 | nbody = httpHeaderGet( |
94 | - request->header, | ||
95 | - request->nheader, | 92 | + this->cur_request->header, |
93 | + this->cur_request->nheader, | ||
96 | "Content-Length"); | 94 | "Content-Length"); |
97 | 95 | ||
98 | if (NULL == nbody) { | 96 | if (NULL == nbody) { |
@@ -100,14 +98,16 @@ httpRequestParserParse(HttpRequestParser this) | @@ -100,14 +98,16 @@ httpRequestParserParse(HttpRequestParser this) | ||
100 | break; | 98 | break; |
101 | } | 99 | } |
102 | else { | 100 | else { |
103 | - request->nbody = atoi(nbody); | 101 | + this->cur_request->nbody = atoi(nbody); |
104 | } | 102 | } |
105 | } | 103 | } |
106 | 104 | ||
107 | - if (REMAINS(this, data) >= request->nbody) { | ||
108 | - request->body = calloc(1, request->nbody + 1); | ||
109 | - memcpy(request->body, data, request->nbody); | ||
110 | - data += request->nbody; | 105 | + if (REMAINS(this) >= this->cur_request->nbody) { |
106 | + this->cur_request->body = calloc(1, this->cur_request->nbody + 1); | ||
107 | + memcpy(this->cur_request->body, | ||
108 | + this->cur_data, | ||
109 | + this->cur_request->nbody); | ||
110 | + this->cur_data += this->cur_request->nbody; | ||
111 | this->state = HTTP_REQUEST_DONE; | 111 | this->state = HTTP_REQUEST_DONE; |
112 | } | 112 | } |
113 | } | 113 | } |
@@ -119,14 +119,14 @@ httpRequestParserParse(HttpRequestParser this) | @@ -119,14 +119,14 @@ httpRequestParserParse(HttpRequestParser this) | ||
119 | * enqueue current request | 119 | * enqueue current request |
120 | */ | 120 | */ |
121 | this->request_queue->requests[(this->request_queue->nrequests)++] = | 121 | this->request_queue->requests[(this->request_queue->nrequests)++] = |
122 | - request; | 122 | + this->cur_request; |
123 | 123 | ||
124 | /** | 124 | /** |
125 | * remove processed stuff from input buffer. | 125 | * remove processed stuff from input buffer. |
126 | */ | 126 | */ |
127 | - memmove(this->buffer, data, REMAINS(this, data)); | 127 | + memmove(this->buffer, this->cur_data, REMAINS(this)); |
128 | 128 | ||
129 | - this->buffer_used -= data - this->buffer; | 129 | + this->buffer_used -= this->cur_data - this->buffer; |
130 | 130 | ||
131 | /** | 131 | /** |
132 | * dont continue loop if input buffer is empty | 132 | * dont continue loop if input buffer is empty |
Please
register
or
login
to post a comment