search

skeeto / emacs-web-server

Extensible Emacs HTTP 1.1 server
The Unlicense
240 stars 31 forks source link

httpd-parse-uri should not unhex the script path #15

Closed Fuco1 closed 6 years ago

Fuco1 commented 6 years ago

If we use "nice URI"s with hex-encoded content we can get back data which do not correspond to the request. For example an API endpoint such as /buffers/some%2Fname/more-data would return /buffers/some/name/more-data but this makes the router quite confused because now it has three inputs instead of two.

The entire point of hex encoding is to prevent such a scenario, no? I think it only makes sense for the query string (where we already know that the content between = and & is the value, but not for the path.

Fuco1 commented 6 years ago

This relates to https://github.com/netguy204/imp.el/issues/33 and https://github.com/netguy204/imp.el/issues/30 which I've fixed by disabling the unhexing in httpd and insread unhexing the buffer name in imp.el.

skeeto commented 6 years ago

Yup, you're exactly right. It looks like the W3C recommends this as well (Example 2):

The URIs

http://www.w3.org/albert/bertram/marie-claude

and

http://www.w3.org/albert/bertram%2Fmarie-claude

are NOT identical, as in the second case the encoded slash does not have hierarchical significance.

Fuco1 commented 6 years ago

Do you want me to make a PR? The patch is trivial, I think all we need to change is the one line in httpd-parse-uri.

skeeto commented 6 years ago

I just went ahead and fixed it myself. It really was trivial but that was in part due to another bug. Not only did we need to remove the httpd-unhex, but I also wanted to make sure defservlet* still decoded the value before passing it to the body of the servlet (which is why I didn't just give you the go ahead). In short, I still wanted to decode the path components, just later in the pipeline. Turns out the macro was already running httpd-unhex a second time, which shouldn't have been happening. So removing the pre-routing unhex fixes this bug, too.

Thanks!

Fuco1 commented 6 years ago

Great, thanks! What you say makes total sense.