[Asterisk-code-review] http: Add ability to disable /httpstatus URI (asterisk[13])

Friendly Automation asteriskteam at digium.com
Thu Jan 23 08:43:32 CST 2020


Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13669 )

Change subject: http: Add ability to disable /httpstatus URI
......................................................................

http: Add ability to disable /httpstatus URI

Add a new configuration option 'enable_status' which allows the
/httpstatus URI handler to be administratively disabled.

We also no longer unconditionally register the /static and /httpstatus
URI handlers, but instead do it based upon configuration.

Behavior change: If enable_static was turned off, the URI handler was
still installed but returned a 403 when it was accessed. Because we
now register/unregister the URI handlers as appropriate, if the
/static URI is disabled we will return a 404 instead.

Additionally:

* Change 'enablestatic' to 'enable_static' but keep the former for
  backwards compatibility.
* Improve some internal variable names

ASTERISK-28710 #close

Change-Id: I647510f796473793b1d3ce1beb32659813be69e1
---
M configs/samples/http.conf.sample
A doc/CHANGES-staging/http.txt
M main/http.c
3 files changed, 53 insertions(+), 17 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit



diff --git a/configs/samples/http.conf.sample b/configs/samples/http.conf.sample
index 342dff4..62ed642 100644
--- a/configs/samples/http.conf.sample
+++ b/configs/samples/http.conf.sample
@@ -1,5 +1,5 @@
 ;
-; Asterisk Builtin mini-HTTP server
+; Asterisk Built-in mini-HTTP server
 ;
 ;
 ; Note about Asterisk documentation:
@@ -65,7 +65,13 @@
 ; Whether Asterisk should serve static content from static-http
 ; Default is no.
 ;
-;enablestatic=yes
+;enable_static=yes
+;
+; Whether Asterisk should serve a status page showing the running
+; configuration of this built-in HTTP server.
+; Default is yes.
+;
+;enable_status=no
 ;
 ; Redirect one URI to another.  This is how you would set a
 ; default page.
diff --git a/doc/CHANGES-staging/http.txt b/doc/CHANGES-staging/http.txt
new file mode 100644
index 0000000..ad778ec
--- /dev/null
+++ b/doc/CHANGES-staging/http.txt
@@ -0,0 +1,4 @@
+Subject: http
+
+You can now disable the /httpstatus page served by Asterisk's built-in
+HTTP server by setting 'enable_status' to 'no' in http.conf.
diff --git a/main/http.c b/main/http.c
index 136c916..0600bfa 100644
--- a/main/http.c
+++ b/main/http.c
@@ -134,7 +134,8 @@
 
 /* all valid URIs must be prepended by the string in prefix. */
 static char prefix[MAX_PREFIX];
-static int enablestatic;
+static int static_uri_enabled;
+static int status_uri_enabled;
 
 /*! \brief Limit the kinds of files we're willing to serve up */
 static struct {
@@ -255,9 +256,13 @@
 		return 0;
 	}
 
-	/* Yuck.  I'm not really sold on this, but if you don't deliver static content it makes your configuration
-	   substantially more challenging, but this seems like a rather irritating feature creep on Asterisk. */
-	if (!enablestatic || ast_strlen_zero(uri)) {
+	/* Yuck.  I'm not really sold on this, but if you don't deliver static content it
+	 * makes your configuration substantially more challenging, but this seems like a
+	 * rather irritating feature creep on Asterisk.
+	 *
+	 * XXX: It is not clear to me what this comment means or if it is any longer
+	 *      relevant. */
+	if (ast_strlen_zero(uri)) {
 		goto out403;
 	}
 
@@ -408,7 +413,7 @@
 	return 0;
 }
 
-static struct ast_http_uri statusuri = {
+static struct ast_http_uri status_uri = {
 	.callback = httpstatus_callback,
 	.description = "Asterisk HTTP General Status",
 	.uri = "httpstatus",
@@ -417,7 +422,7 @@
 	.key = __FILE__,
 };
 
-static struct ast_http_uri staticuri = {
+static struct ast_http_uri static_uri = {
 	.callback = static_callback,
 	.description = "Asterisk HTTP Static Delivery",
 	.uri = "static",
@@ -2095,8 +2100,9 @@
 {
 	struct ast_config *cfg;
 	struct ast_variable *v;
-	int enabled=0;
-	int newenablestatic=0;
+	int enabled = 0;
+	int new_static_uri_enabled = 0;
+	int new_status_uri_enabled = 1; /* Default to enabled for BC */
 	char newprefix[MAX_PREFIX] = "";
 	char server_name[MAX_SERVER_NAME_LENGTH];
 	struct http_uri_redirect *redirect;
@@ -2174,8 +2180,10 @@
 			}
 		} else if (!strcasecmp(v->name, "enabled")) {
 			enabled = ast_true(v->value);
-		} else if (!strcasecmp(v->name, "enablestatic")) {
-			newenablestatic = ast_true(v->value);
+		} else if (!strcasecmp(v->name, "enablestatic") || !strcasecmp(v->name, "enable_static")) {
+			new_static_uri_enabled = ast_true(v->value);
+		} else if (!strcasecmp(v->name, "enable_status")) {
+			new_status_uri_enabled = ast_true(v->value);
 		} else if (!strcasecmp(v->name, "bindport")) {
 			if (ast_parse_arg(v->value, PARSE_UINT32 | PARSE_IN_RANGE | PARSE_DEFAULT,
 				&bindport, DEFAULT_PORT, 0, 65535)) {
@@ -2226,7 +2234,6 @@
 	}
 
 	ast_copy_string(http_server_name, server_name, sizeof(http_server_name));
-	enablestatic = newenablestatic;
 
 	if (num_addrs && enabled) {
 		int i;
@@ -2272,6 +2279,22 @@
 		}
 	}
 
+	if (static_uri_enabled && !new_static_uri_enabled) {
+		ast_http_uri_unlink(&static_uri);
+	} else if (!static_uri_enabled && new_static_uri_enabled) {
+		ast_http_uri_link(&static_uri);
+	}
+
+	static_uri_enabled = new_static_uri_enabled;
+
+	if (status_uri_enabled && !new_status_uri_enabled) {
+		ast_http_uri_unlink(&status_uri);
+	} else if (!status_uri_enabled && new_status_uri_enabled) {
+		ast_http_uri_link(&status_uri);
+	}
+
+	status_uri_enabled = new_status_uri_enabled;
+
 	return 0;
 }
 
@@ -2353,8 +2376,13 @@
 	ast_free(http_tls_cfg.pvtfile);
 	ast_free(http_tls_cfg.cipher);
 
-	ast_http_uri_unlink(&statusuri);
-	ast_http_uri_unlink(&staticuri);
+	if (status_uri_enabled) {
+		ast_http_uri_unlink(&status_uri);
+	}
+
+	if (static_uri_enabled) {
+		ast_http_uri_unlink(&static_uri);
+	}
 
 	AST_RWLIST_WRLOCK(&uri_redirects);
 	while ((redirect = AST_RWLIST_REMOVE_HEAD(&uri_redirects, entry))) {
@@ -2365,8 +2393,6 @@
 
 int ast_http_init(void)
 {
-	ast_http_uri_link(&statusuri);
-	ast_http_uri_link(&staticuri);
 	ast_cli_register_multiple(cli_http, ARRAY_LEN(cli_http));
 	ast_register_cleanup(http_shutdown);
 

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13669
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I647510f796473793b1d3ce1beb32659813be69e1
Gerrit-Change-Number: 13669
Gerrit-PatchSet: 3
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200123/eb604a66/attachment.html>


More information about the asterisk-code-review mailing list