From f956ef92e9d15a87cf3eae511e73d48ad6cb3c61 Mon Sep 17 00:00:00 2001 From: Jeff Kaufman Date: Tue, 22 Oct 2013 16:47:49 -0400 Subject: [PATCH] paths: don't require existence Create paths for FileCachePath and LogDir if they don't exist yet and give them proper permissions. --- README.md | 2 -- src/ngx_pagespeed.cc | 55 +++++++++++++++++++++++++++++++++++++++ test/nginx_system_test.sh | 10 +++---- 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 6395317fe..a9249798a 100644 --- a/README.md +++ b/README.md @@ -72,8 +72,6 @@ In your `nginx.conf`, add to the main or server block: ```nginx pagespeed on; - -# needs to exist and be writable by nginx pagespeed FileCachePath /var/ngx_pagespeed_cache; ``` diff --git a/src/ngx_pagespeed.cc b/src/ngx_pagespeed.cc index b319bf617..cadc77145 100644 --- a/src/ngx_pagespeed.cc +++ b/src/ngx_pagespeed.cc @@ -58,6 +58,7 @@ #include "net/instaweb/util/public/google_url.h" #include "net/instaweb/util/public/gzip_inflater.h" #include "pthread_shared_mem.h" +#include "net/instaweb/util/public/null_message_handler.h" #include "net/instaweb/util/public/query_params.h" #include "net/instaweb/util/public/statistics_logger.h" #include "net/instaweb/util/public/stdio_file_system.h" @@ -563,6 +564,45 @@ bool ps_is_global_only_option(const StringPiece& option_name) { return false; } +char* ps_init_dir(const StringPiece& directive, + const StringPiece& path, + ngx_conf_t* cf) { + if (path.size() == 0 || path[0] != '/') { + return string_piece_to_pool_string( + cf->pool, net_instaweb::StrCat(directive, " ", path, + " must start with a slash")); + } + + net_instaweb::StdioFileSystem file_system; + net_instaweb::NullMessageHandler message_handler; + GoogleString gs_path; + path.CopyToString(&gs_path); + if (!file_system.IsDir(gs_path.c_str(), &message_handler).is_true()) { + if (!file_system.RecursivelyMakeDir(path, &message_handler)) { + return string_piece_to_pool_string( + cf->pool, net_instaweb::StrCat( + directive, " path ", path, + " does not exist and could not be created.")); + } + // Directory created, but may not be readable by the worker processes. + } + + if (geteuid() != 0) { + return NULL; // We're not root, so we're staying whoever we are. + } + + ngx_core_conf_t* ccf = + (ngx_core_conf_t*)(ngx_get_conf(cf->cycle->conf_ctx, ngx_core_module)); + CHECK(ccf != NULL); + + if (chown(gs_path.c_str(), ccf->user, ccf->group) != 0) { + return string_piece_to_pool_string( + cf->pool, net_instaweb::StrCat( + directive, " ", path, " unable to set permissions")); + } + return NULL; +} + #define NGX_PAGESPEED_MAX_ARGS 10 char* ps_configure(ngx_conf_t* cf, net_instaweb::NgxRewriteOptions** options, @@ -595,6 +635,21 @@ char* ps_configure(ngx_conf_t* cf, } } + // Some options require the worker process to be able to read and write to + // a specific directory. Generally the master process is root while the + // worker is nobody, so we need to change permissions and create the directory + // if necessary. + if (n_args == 2 && + (net_instaweb::StringCaseEqual("LogDir", args[0]) || + net_instaweb::StringCaseEqual("FileCachePath", args[0]))) { + char* error_message = ps_init_dir(args[0], args[1], cf); + if (error_message != NULL) { + return error_message; + } + // The directory has been prepared, but we haven't actually parsed the + // directive yet. That happens below in ParseAndSetOptions(). + } + ps_main_conf_t* cfg_m = static_cast( ngx_http_cycle_get_module_main_conf(cf->cycle, ngx_pagespeed)); if (*options == NULL) { diff --git a/test/nginx_system_test.sh b/test/nginx_system_test.sh index e4522f715..334074b5e 100755 --- a/test/nginx_system_test.sh +++ b/test/nginx_system_test.sh @@ -120,14 +120,14 @@ PROXY_CACHE="$TEST_TMP/proxycache" TMP_PROXY_CACHE="$TEST_TMP/tmpproxycache" ERROR_LOG="$TEST_TMP/error.log" ACCESS_LOG="$TEST_TMP/access.log" -check_simple mkdir "$PROXY_CACHE" -check_simple mkdir "$TMP_PROXY_CACHE" + +# Check that we do ok with directories that already exist. FILE_CACHE="$TEST_TMP/file-cache" check_simple mkdir "$FILE_CACHE" + +# And directories that don't. SECONDARY_CACHE="$TEST_TMP/file-cache/secondary/" -check_simple mkdir "$SECONDARY_CACHE" -SHM_CACHE="$TEST_TMP/file-cache/with_shm/" -check_simple mkdir "$SHM_CACHE" +SHM_CACHE="$TEST_TMP/file-cache/intermediate/directories/with_shm/" VALGRIND_OPTIONS=""