[Asterisk-code-review] res_ari.c: Prefer exact handler match over wildcard (...asterisk[13])

[Friendly Automation]

Friendly Automation has submitted this change and it was merged. ( https://gerrit.asterisk.org/c/asterisk/+/12751 )

Change subject: res_ari.c:  Prefer exact handler match over wildcard
......................................................................

res_ari.c:  Prefer exact handler match over wildcard

Given the following request path and 2 handler paths...
Request: /channels/externalMedia
Handler: /channels/{channelId}      "wildcard"
Handler: /channels/externalmedia    "non-wildcard"

...if /channels/externalMedia was registered as a handler after
/channels/{channelId} as shown above, the request would automatically
match the wildcard handler and attempt to parse "externalMedia" into
the channelId variable which isn't what was intended.  It'd work
if the non-wildard entry was defined in rest-api/api-docs/channels.json
before the wildcard entry but that makes the json files
order-dependent which isn't a good thing.

To combat this issue, the search loop saves any wildcard match but
continues looking for exact matches at the same level.  If it finds
one, it's used.  If it hasn't found an exact match at the end of
the current level, the wildcard is used.  Regardless, after
searching the current level, the wildcard is cleared so it won't
accidentally match for a different object or a higher level.

BTW, it's currently not possible for more than 1 wildcard entry
to be defined for a level.  For instance, there couldn't be:
Handler: /channels/{channelId}
Handler: /channels/{channelName}
We wouldn't know which one to match.

Change-Id: I574aa3cbe4249c92c30f74b9b40e750e9002f925
---
M res/res_ari.c
1 file changed, 18 insertions(+), 5 deletions(-)

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



diff --git a/res/res_ari.c b/res/res_ari.c
index 0554587..2664800 100644
--- a/res/res_ari.c
+++ b/res/res_ari.c
@@ -496,6 +496,7 @@
 {
 	RAII_VAR(struct stasis_rest_handlers *, root, NULL, ao2_cleanup);
 	struct stasis_rest_handlers *handler;
+	struct stasis_rest_handlers *wildcard_handler = NULL;
 	RAII_VAR(struct ast_variable *, path_vars, NULL, ast_variables_destroy);
 	char *path = ast_strdupa(uri);
 	char *path_segment;
@@ -504,37 +505,49 @@
 	root = handler = get_root_handler();
 	ast_assert(root != NULL);
 
+	ast_debug(3, "Finding handler for %s\n", path);
+
 	while ((path_segment = strsep(&path, "/")) && (strlen(path_segment) > 0)) {
 		struct stasis_rest_handlers *found_handler = NULL;
 		int i;
 
 		ast_uri_decode(path_segment, ast_uri_http_legacy);
-		ast_debug(3, "Finding handler for %s\n", path_segment);
+		ast_debug(3, "  Finding handler for %s\n", path_segment);
 
 		for (i = 0; found_handler == NULL && i < handler->num_children; ++i) {
 			struct stasis_rest_handlers *child = handler->children[i];
 
-			ast_debug(3, "  Checking %s\n", child->path_segment);
 			if (child->is_wildcard) {
 				/* Record the path variable */
 				struct ast_variable *path_var = ast_variable_new(child->path_segment, path_segment, __FILE__);
 				path_var->next = path_vars;
 				path_vars = path_var;
-				found_handler = child;
+				wildcard_handler = child;
+				ast_debug(3, "        Checking %s %s:  Matched wildcard.\n", handler->path_segment, child->path_segment);
+
 			} else if (strcmp(child->path_segment, path_segment) == 0) {
 				found_handler = child;
+				ast_debug(3, "        Checking %s %s:  Explicit match with %s\n", handler->path_segment, child->path_segment, path_segment);
+			} else {
+				ast_debug(3, "        Checking %s %s:  Didn't match %s\n", handler->path_segment, child->path_segment, path_segment);
 			}
 		}
 
+		if (!found_handler && wildcard_handler) {
+			ast_debug(3, "  No explicit handler found for %s.  Using wildcard %s.\n",
+				path_segment, wildcard_handler->path_segment);
+			found_handler = wildcard_handler;
+			wildcard_handler = NULL;
+		}
+
 		if (found_handler == NULL) {
 			/* resource not found */
-			ast_debug(3, "  Handler not found\n");
+			ast_debug(3, "  Handler not found for %s\n", path_segment);
 			ast_ari_response_error(
 				response, 404, "Not Found",
 				"Resource not found");
 			return;
 		} else {
-			ast_debug(3, "  Got it!\n");
 			handler = found_handler;
 		}
 	}

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I574aa3cbe4249c92c30f74b9b40e750e9002f925
Gerrit-Change-Number: 12751
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.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/20190821/8dfb3ea1/attachment-0001.html>