ngx_str_t::to_str should not unwrap.

In general, we should avoid exposing methods that panic when its
reasonable to return a Result instead. This particular method was used,
likely without considering that it may panic or validating that the
contents are utf-8, in ngx::http::NgxListIterator's Iterator impl,
which made it very easy to panic based on untrusted input.

Author: pchickey

examples/async.rs

@@ -9,11 +9,11 @@ use ngx::ffi::{
     ngx_array_push, ngx_command_t, ngx_conf_t, ngx_connection_t, ngx_event_t, ngx_http_handler_pt,
     ngx_http_module_t, ngx_http_phases_NGX_HTTP_ACCESS_PHASE, ngx_int_t, ngx_module_t,
     ngx_post_event, ngx_posted_events, ngx_posted_next_events, ngx_str_t, ngx_uint_t,
-    NGX_CONF_TAKE1, NGX_HTTP_LOC_CONF, NGX_HTTP_LOC_CONF_OFFSET, NGX_HTTP_MODULE,
+    NGX_CONF_TAKE1, NGX_HTTP_LOC_CONF, NGX_HTTP_LOC_CONF_OFFSET, NGX_HTTP_MODULE, NGX_LOG_EMERG,
 };
 use ngx::http::{self, HttpModule, MergeConfigError};
 use ngx::http::{HttpModuleLocationConf, HttpModuleMainConf, NgxHttpCoreModule};
-use ngx::{http_request_handler, ngx_log_debug_http, ngx_string};
+use ngx::{http_request_handler, ngx_conf_log_error, ngx_log_debug_http, ngx_string};
 use tokio::runtime::Runtime;
 
 struct Module;
@@ -205,7 +205,13 @@ extern "C" fn ngx_http_async_commands_set_enable(
     unsafe {
         let conf = &mut *(conf as *mut ModuleConfig);
         let args: &[ngx_str_t] = (*(*cf).args).as_slice();
-        let val = args[1].to_str();
+        let val = match args[1].to_str() {
+            Ok(s) => s,
+            Err(_) => {
+                ngx_conf_log_error!(NGX_LOG_EMERG, cf, "`async` argument is not utf-8 encoded");
+                return ngx::core::NGX_CONF_ERROR;
+            }
+        };
 
         // set default value optionally
         conf.enable = false;

examples/awssig.rs

@@ -6,10 +6,10 @@ use ngx::ffi::{
     ngx_array_push, ngx_command_t, ngx_conf_t, ngx_http_handler_pt, ngx_http_module_t,
     ngx_http_phases_NGX_HTTP_PRECONTENT_PHASE, ngx_int_t, ngx_module_t, ngx_str_t, ngx_uint_t,
     NGX_CONF_TAKE1, NGX_HTTP_LOC_CONF, NGX_HTTP_LOC_CONF_OFFSET, NGX_HTTP_MODULE,
-    NGX_HTTP_SRV_CONF,
+    NGX_HTTP_SRV_CONF, NGX_LOG_EMERG,
 };
 use ngx::http::*;
-use ngx::{http_request_handler, ngx_log_debug_http, ngx_string};
+use ngx::{http_request_handler, ngx_conf_log_error, ngx_log_debug_http, ngx_string};
 
 struct Module;
 
@@ -176,7 +176,17 @@ extern "C" fn ngx_http_awssigv4_commands_set_enable(
     unsafe {
         let conf = &mut *(conf as *mut ModuleConfig);
         let args: &[ngx_str_t] = (*(*cf).args).as_slice();
-        let val = args[1].to_str();
+        let val = match args[1].to_str() {
+            Ok(s) => s,
+            Err(_) => {
+                ngx_conf_log_error!(
+                    NGX_LOG_EMERG,
+                    cf,
+                    "`awssigv4` argument is not utf-8 encoded"
+                );
+                return ngx::core::NGX_CONF_ERROR;
+            }
+        };
 
         // set default value optionally
         conf.enable = false;

examples/curl.rs

@@ -4,11 +4,11 @@ use ngx::core;
 use ngx::ffi::{
     ngx_array_push, ngx_command_t, ngx_conf_t, ngx_http_handler_pt, ngx_http_module_t,
     ngx_http_phases_NGX_HTTP_ACCESS_PHASE, ngx_int_t, ngx_module_t, ngx_str_t, ngx_uint_t,
-    NGX_CONF_TAKE1, NGX_HTTP_LOC_CONF, NGX_HTTP_LOC_CONF_OFFSET, NGX_HTTP_MODULE,
+    NGX_CONF_TAKE1, NGX_HTTP_LOC_CONF, NGX_HTTP_LOC_CONF_OFFSET, NGX_HTTP_MODULE, NGX_LOG_EMERG,
 };
 use ngx::http::{self, HttpModule, MergeConfigError};
 use ngx::http::{HttpModuleLocationConf, HttpModuleMainConf, NgxHttpCoreModule};
-use ngx::{http_request_handler, ngx_log_debug_http, ngx_string};
+use ngx::{http_request_handler, ngx_conf_log_error, ngx_log_debug_http, ngx_string};
 
 struct Module;
 
@@ -119,7 +119,13 @@ extern "C" fn ngx_http_curl_commands_set_enable(
         let conf = &mut *(conf as *mut ModuleConfig);
         let args: &[ngx_str_t] = (*(*cf).args).as_slice();
 
-        let val = args[1].to_str();
+        let val = match args[1].to_str() {
+            Ok(s) => s,
+            Err(_) => {
+                ngx_conf_log_error!(NGX_LOG_EMERG, cf, "`curl` argument is not utf-8 encoded");
+                return ngx::core::NGX_CONF_ERROR;
+            }
+        };
 
         // set default value optionally
         conf.enable = false;

nginx-sys/src/string.rs

@@ -40,15 +40,14 @@ impl ngx_str_t {
         self.len == 0
     }
 
-    /// Convert the nginx string to a string slice (`&str`).
-    ///
-    /// # Panics
-    /// This function panics if the `ngx_str_t` is not valid UTF-8.
+    /// Returns the contents of this `ngx_str_t` a string slice (`&str`) if
+    /// the contents are utf-8 encoded.
     ///
     /// # Returns
-    /// A string slice (`&str`) representing the nginx string.
-    pub fn to_str(&self) -> &str {
-        str::from_utf8(self.as_bytes()).unwrap()
+    /// A string slice (`&str`) representing the nginx string, or else a
+    /// Utf8Error.
+    pub fn to_str(&self) -> Result<&str, str::Utf8Error> {
+        str::from_utf8(self.as_bytes())
     }
 
     /// Creates an empty `ngx_str_t` instance.