[PATCH] Dav: dav_request_buffering directive

Tommy Lindgren tommy.lindgren at gmail.com
Fri Jun 2 03:43:38 UTC 2017


Hi,

Currently the webdav module writes the request body to a temporary file
and moves it to destination filepath once upload is complete.

The patch below introduces dav_request_buffering. Default is "on". If
"off", the request body will be written directly to destination filepath
(unless destination file already exists, in which case 409 is returned).

Rationale:

We work with a system where http clients can upload large files (>100
GB, think raw uncompressed HD footage). We want to use nginx + the dav
module for this. However, other clients want to access the uploaded
files as soon as possible (even if the upload is still running). Thus we
implemented "dav_request_buffering off".

To make sure a client can't overwrite arbitrary files we protect the
location with the secure_link module, i.e. the system has previously
handed out a secure upload url (with unique filename to avoid
collisions) to the client.

Implementation details:

The name "dav_request_buffering" is supposed to be analogous with
"proxy_request_buffering" which hopefully make sense even though the
implementation is very different.

We extend ngx_http_request_s with request_body_temp_name, which the dav
module sets when dav_request_buffering is off, and then
ngx_create_temp_file writes directly to this filepath.
request_body_in_clean_file is not set in this case. In other words, we
instruct nginx core to write the "temporary" file to the final
destination directly.

Any chance seeing this patch/feature merged?

Thanks,
Tommy

# HG changeset patch
# User Tommy Lindgren <tli at vizrt.com>
# Date 1496048257 -7200
#      Mon May 29 10:57:37 2017 +0200
# Node ID 4f4a483ca56229b7690f5d305bf056027005c7f2
# Parent  ed1101bbf19f1edf300665b6398cd66d45b71577
Dav: introduced dav_request_buffering directive

By default the dav module writes the request body to a temporary file
and moves it to destination filepath when upload is complete.

If dav_request_buffering is "off", the request body will be written
directly to destination filepath (unless destination file already
exists, in which case 409 is returned).

diff --git a/src/core/ngx_file.c b/src/core/ngx_file.c
--- a/src/core/ngx_file.c
+++ b/src/core/ngx_file.c
@@ -150,6 +150,22 @@ ngx_create_temp_file(ngx_file_t *file, n
     ngx_pool_cleanup_t       *cln;
     ngx_pool_cleanup_file_t  *clnf;
 
+    if (file->name.data != NULL) {
+        file->fd = ngx_open_tempfile(file->name.data, 1, access);
+
+        ngx_log_debug1(NGX_LOG_DEBUG_CORE, file->log, 0,
+                       "forced temp fd:%d", file->fd);
+
+        if (file->fd == NGX_INVALID_FILE) {
+            ngx_log_error(NGX_LOG_ERR, file->log, ngx_errno,
+                          ngx_open_tempfile_n " \"%s\" failed",
+                          file->name.data);
+            return NGX_ERROR;
+        }
+
+        return NGX_OK;
+    }
+
     if (file->name.len) {
         name = file->name;
         levels = 0;
diff --git a/src/http/modules/ngx_http_dav_module.c b/src/http/modules/ngx_http_dav_module.c
--- a/src/http/modules/ngx_http_dav_module.c
+++ b/src/http/modules/ngx_http_dav_module.c
@@ -23,6 +23,7 @@ typedef struct {
     ngx_uint_t  access;
     ngx_uint_t  min_delete_depth;
     ngx_flag_t  create_full_put_path;
+    ngx_flag_t  dav_request_buffering;
 } ngx_http_dav_loc_conf_t;
 
 
@@ -104,6 +105,13 @@ static ngx_command_t  ngx_http_dav_comma
       offsetof(ngx_http_dav_loc_conf_t, access),
       NULL },
 
+    { ngx_string("dav_request_buffering"),
+      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
+      ngx_conf_set_flag_slot,
+      NGX_HTTP_LOC_CONF_OFFSET,
+      offsetof(ngx_http_dav_loc_conf_t, dav_request_buffering),
+      NULL },
+
       ngx_null_command
 };
 
@@ -142,8 +150,12 @@ ngx_module_t  ngx_http_dav_module = {
 static ngx_int_t
 ngx_http_dav_handler(ngx_http_request_t *r)
 {
+    size_t                    root;
+    ngx_str_t                 path; 
     ngx_int_t                 rc;
     ngx_http_dav_loc_conf_t  *dlcf;
+    ngx_file_info_t           fi;
+    ngx_err_t         err;
 
     dlcf = ngx_http_get_module_loc_conf(r, ngx_http_dav_module);
 
@@ -169,10 +181,39 @@ ngx_http_dav_handler(ngx_http_request_t 
 
         r->request_body_in_file_only = 1;
         r->request_body_in_persistent_file = 1;
-        r->request_body_in_clean_file = 1;
         r->request_body_file_group_access = 1;
         r->request_body_file_log_level = 0;
 
+        if (dlcf->dav_request_buffering) {
+            r->request_body_in_clean_file = 1;
+        } else {
+            if (ngx_http_map_uri_to_path(r, &path, &root, 0) == NULL) {
+                return NGX_HTTP_INTERNAL_SERVER_ERROR;
+            }
+
+            if (ngx_file_info(path.data, &fi) != NGX_FILE_ERROR) {
+                return NGX_HTTP_CONFLICT;
+            }
+
+            if (dlcf->create_full_put_path) {
+                err = ngx_create_full_path(path.data, ngx_dir_access(dlcf->access));
+                if (err) {
+                    ngx_log_error(NGX_LOG_CRIT, r->connection->log, err,
+                            ngx_create_dir_n " \"%s\" failed", path.data);
+                    return NGX_HTTP_INTERNAL_SERVER_ERROR;
+                }
+            }
+
+            r->request_body_temp_name.len = path.len;
+            r->request_body_temp_name.data = ngx_pnalloc(r->pool, path.len + 1);
+
+            if (r->request_body_temp_name.data == NULL) {
+                return NGX_HTTP_INTERNAL_SERVER_ERROR;
+            }
+
+            ngx_memcpy(r->request_body_temp_name.data, path.data, path.len);
+        }
+
         rc = ngx_http_read_client_request_body(r, ngx_http_dav_put_handler);
 
         if (rc >= NGX_HTTP_SPECIAL_RESPONSE) {
@@ -213,6 +254,16 @@ ngx_http_dav_put_handler(ngx_http_reques
     ngx_ext_rename_file_t     ext;
     ngx_http_dav_loc_conf_t  *dlcf;
 
+    dlcf = ngx_http_get_module_loc_conf(r, ngx_http_dav_module);
+
+    if (!dlcf->dav_request_buffering) {
+        r->headers_out.content_length_n = 0;
+        r->headers_out.status = NGX_HTTP_CREATED;
+        r->header_only = 1;
+        ngx_http_finalize_request(r, ngx_http_send_header(r));
+        return;
+    }
+
     if (r->request_body == NULL || r->request_body->temp_file == NULL) {
         ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
         return;
@@ -251,8 +302,6 @@ ngx_http_dav_put_handler(ngx_http_reques
         }
     }
 
-    dlcf = ngx_http_get_module_loc_conf(r, ngx_http_dav_module);
-
     ext.access = dlcf->access;
     ext.path_access = dlcf->access;
     ext.time = -1;
@@ -1115,6 +1164,7 @@ ngx_http_dav_create_loc_conf(ngx_conf_t 
     conf->min_delete_depth = NGX_CONF_UNSET_UINT;
     conf->access = NGX_CONF_UNSET_UINT;
     conf->create_full_put_path = NGX_CONF_UNSET;
+    conf->dav_request_buffering = NGX_CONF_UNSET;
 
     return conf;
 }
@@ -1137,6 +1187,9 @@ ngx_http_dav_merge_loc_conf(ngx_conf_t *
     ngx_conf_merge_value(conf->create_full_put_path,
                          prev->create_full_put_path, 0);
 
+    ngx_conf_merge_value(conf->dav_request_buffering,
+                         prev->dav_request_buffering, 1);
+
     return NGX_CONF_OK;
 }
 
diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
--- a/src/http/ngx_http_request.h
+++ b/src/http/ngx_http_request.h
@@ -481,6 +481,7 @@ struct ngx_http_request_s {
     unsigned                          request_body_file_group_access:1;
     unsigned                          request_body_file_log_level:3;
     unsigned                          request_body_no_buffering:1;
+    ngx_str_t                         request_body_temp_name;
 
     unsigned                          subrequest_in_memory:1;
     unsigned                          waited:1;
diff --git a/src/http/ngx_http_request_body.c b/src/http/ngx_http_request_body.c
--- a/src/http/ngx_http_request_body.c
+++ b/src/http/ngx_http_request_body.c
@@ -450,6 +450,7 @@ ngx_http_write_request_body(ngx_http_req
         tf->file.fd = NGX_INVALID_FILE;
         tf->file.log = r->connection->log;
         tf->path = clcf->client_body_temp_path;
+        tf->file.name = r->request_body_temp_name;
         tf->pool = r->pool;
         tf->warn = "a client request body is buffered to a temporary file";
         tf->log_level = r->request_body_file_log_level;


More information about the nginx-devel mailing list