Browse Source

Fix crash if a read event occur and the request args have been deleted

This could happen with the upload module if for a reason the upload
is aborted. In this case an internal redirect is done, but the request
still contains our read even handler.
Unfortunately the request args are nullified before calling this read
even handler. So we were crashing because we didn't check for id==null.
This patch fixes this issue.

Signed-off-by: Brice Figureau <brice@daysofwonder.com>
tags/v0.7
Brice Figureau 16 years ago
parent
commit
ddca3a78af
1 changed files with 27 additions and 16 deletions
  1. 27
    16
      ngx_http_uploadprogress_module.c

+ 27
- 16
ngx_http_uploadprogress_module.c View File

@@ -131,7 +131,7 @@ get_tracking_id(ngx_http_request_t * r)
131 131
     ngx_uint_t                       i;
132 132
     ngx_list_part_t                 *part;
133 133
     ngx_table_elt_t                 *header;
134
-    ngx_str_t                       *ret;
134
+    ngx_str_t                       *ret, args;
135 135
 
136 136
     ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "upload-progress: get_tracking_id");
137 137
 
@@ -165,14 +165,19 @@ get_tracking_id(ngx_http_request_t * r)
165 165
     ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, 
166 166
                     "upload-progress: get_tracking_id no header found");
167 167
 
168
-    /* not found, check as a reaquest arg */
169
-    if (r->args.len) {
168
+    /* not found, check as a request arg */
169
+    /* it is possible the request args have not been yet created (or already released) */
170
+    /* so let's try harder first from the request line */
171
+    args.len =  r->args.len;
172
+    args.data = r->args.data;
173
+    
174
+    if (args.len && args.data) {
170 175
         ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, 
171
-                       "upload-progress: get_tracking_id no header found but args present");
176
+                       "upload-progress: get_tracking_id no header found, args found");
172 177
         i = 0;
173
-        p = r->args.data;
178
+        p = args.data;
174 179
         do {
175
-            ngx_uint_t len = r->args.len - (p - r->args.data);
180
+            ngx_uint_t len = args.len - (p - args.data);
176 181
             if (len >= 14 && ngx_strncasecmp(p, (u_char*)"X-Progress-ID=", 14) == 0) {
177 182
               ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, 
178 183
                              "upload-progress: get_tracking_id found args: %s",p);
@@ -186,7 +191,7 @@ get_tracking_id(ngx_http_request_t * r)
186 191
 
187 192
         if (i) {
188 193
             start_p = p += 14;
189
-            while (p < r->args.data + r->args.len) {
194
+            while (p < args.data + args.len) {
190 195
                 if (*p++ != '&') {
191 196
                     continue;
192 197
                 }
@@ -321,12 +326,17 @@ static void ngx_http_uploadprogress_event_handler(ngx_http_request_t *r)
321 326
 
322 327
     /* find node, update rest */
323 328
     oldid = id = get_tracking_id(r);
324
-    
329
+
330
+    if (id == NULL) {
331
+        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0,
332
+                       "upload-progress: read_event_handler cant find id");
333
+        return;
334
+    }
335
+
325 336
     /* perform a deep copy of id */
326 337
     id = ngx_http_uploadprogress_strdup(id, r->connection->log);
327
-    
328 338
     ngx_free(oldid);
329
-		
339
+
330 340
     ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
331 341
                    "upload-progress: read_event_handler found id: %V", id);
332 342
     upcf = ngx_http_get_module_loc_conf(r, ngx_http_uploadprogress_module);
@@ -334,18 +344,19 @@ static void ngx_http_uploadprogress_event_handler(ngx_http_request_t *r)
334 344
     
335 345
     /* call the original read event handler */
336 346
     module_ctx = ngx_http_get_module_ctx(r, ngx_http_uploadprogress_module);
337
-    module_ctx->read_event_handler(r);
347
+    if (module_ctx != NULL ) {
348
+        module_ctx->read_event_handler(r);
349
+    }
338 350
 
339 351
     /* at this stage, r is not anymore safe to use */
340 352
     /* the request could have been closed/freed behind our back */
341 353
     /* and thats the same issue with any other material that was allocated in the request pool */
342 354
     /* that's why we duplicate id afterward */
343 355
 
344
-    if (id == NULL) {
345
-        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0,
346
-                       "upload-progress: read_event_handler cant find id");
347
-        return;
348
-    }
356
+    /* it's also possible that the id was null if we got a spurious (like abort) read */
357
+    /* event. In this case we still have called the original read event handler */
358
+    /* but we have to bail out, because we won't ever be able to find our upload node */
359
+
349 360
 
350 361
     if (shm_zone == NULL) {
351 362
         ngx_http_uploadprogress_strdupfree(id);

Loading…
Cancel
Save