Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP URL Parsing Fails when CONFIG_HTTP_SERVER_RESOURCE_WILDCARD is enabled and wildcard contains "?" #84198

Open
EricNRS opened this issue Jan 19, 2025 · 1 comment · May be fixed by #84226
Assignees
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug

Comments

@EricNRS
Copy link
Contributor

EricNRS commented Jan 19, 2025

Describe the bug
If CONFIG_HTTP_SERVER_RESOURCE_WILDCARD is enabled and a wildcard containing "?" is used, then compare_strings() will match the "?" in the wildcard and think that it is the parameter list and it will incorrectly match a shorter URL.

For example, if you have the following three resources, then "/logs/" will match resource three instead of the correct resource 2.

HTTP_RESOURCE_DEFINE(log_list, web_service, "/logs", &log_list_resource_detail);              // resource 1, list the log files
HTTP_RESOURCE_DEFINE(log_list_slash, web_service, "/logs/", &log_list_slash_resource_detail); // resource 2, list the log files
HTTP_RESOURCE_DEFINE(log_file_resource, web_service, "/logs/?*", &log_file_resource_detail);  // resource 3, the actual log file

if (compare_strings(path, resource->resource) == 0) {
NET_DBG("Got match for %s", resource->resource);
*path_len = strlen(resource->resource);
return resource->detail;
}

To Reproduce
Enable CONFIG_HTTP_SERVER_RESOURCE_WILDCARD and add the two resources above and then browse to "/logs/".

Expected behavior
Resource handler 2 is called.

Impact
Wrong resource handler is called.

Fix
I have done this fix which works for the use case here, but there may be ramifications that I don't understand, so I will wait to open a PR until some discussion has been made.

EricNRS@a1e97a6

commit a1e97a69a84c6e3ffdf4c8eccd9f927f0220f0ea (HEAD -> 2025-01-20-http1-fnmatch-fail, ericnrs/2025-01-20-http1-fnmatch-fail)
Author: Eric Holmberg <[email protected]>
Date:   Mon Jan 20 00:48:38 2025 +1300

    net: lib: http: fix URL matching when fnmatch wildcards are used
    
    If CONFIG_HTTP_SERVER_RESOURCE_WILDCARD is enabled and a wildcard
    containing "?" is used, then `compare_strings()` will match the "?"
    in the wildcard and think that it is the parameter list and it will
    incorrectly match a shorter URL.
    
    Fixes: 84198
    
    Signed-off-by: Eric Holmberg <[email protected]>

diff --git a/subsys/net/lib/http/http_server_core.c b/subsys/net/lib/http/http_server_core.c
index d5ef4999c77..0bbfed39bf5 100644
--- a/subsys/net/lib/http/http_server_core.c
+++ b/subsys/net/lib/http/http_server_core.c
@@ -751,9 +751,7 @@ struct http_resource_detail *get_resource_detail(const struct http_service_desc
                                *path_len = strlen(resource->resource);
                                return resource->detail;
                        }
-               }
-
-               if (compare_strings(path, resource->resource) == 0) {
+                       } else if (compare_strings(path, resource->resource) == 0) {
                        NET_DBG("Got match for %s", resource->resource);
 
                        *path_len = strlen(resource->resource);
@EricNRS EricNRS added the bug The issue is a bug, or the PR is fixing a bug label Jan 19, 2025
@mrodgers-witekio
Copy link
Collaborator

I think this can be fixed for the more general case by changing the compare_strings function so that a ?character is only treated as the end of the string coming from the HTTP request. The string from the resource->resource can only ever be terminated by a NULL character.

I should be able to put together a quick PR later today

mrodgers-witekio added a commit to mrodgers-witekio/zephyr that referenced this issue Jan 20, 2025
Fixes zephyrproject-rtos#84198.

If a '?' character is used as part of a wildcard resource, do not treat
this as the end of the string when comparing with a path from the HTTP
request. Only the path from the HTTP request may be terminated by '?'
(in the case of a request with query parameters).

Signed-off-by: Matt Rodgers <[email protected]>
mrodgers-witekio added a commit to mrodgers-witekio/zephyr that referenced this issue Jan 20, 2025
Fixes zephyrproject-rtos#84198.

If a '?' character is used as part of a wildcard resource, do not treat
this as the end of the string when comparing with a path from the HTTP
request. Only the path from the HTTP request may be terminated by '?'
(in the case of a request with query parameters).

Signed-off-by: Matt Rodgers <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants