kristapsdz / kcgi

minimal CGI and FastCGI library for C/C++
https://kristaps.bsd.lv/kcgi
ISC License
275 stars 40 forks source link

Parameters in Content-Type header aren't always ignored #109

Closed hyjial closed 2 weeks ago

hyjial commented 7 months ago

Trigger

The HTML document below makes Firefox ESR 115.7.0 from OpenBSD ports send a POST request with Content-Type = application/x-www-form-urlencoded;charset=UTF-8.

<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8" />
</head>
<body>
<script type="application/javascript">
function onLoad(e) {
        const curr_url = new URL(document.URL);
        if (!curr_url.protocol.startsWith('http'))
                return;
        const url = new URL('/events', curr_url.origin);
        const body = new URLSearchParams([
                ['name', 'page_viewed'],
                ['value', 'index'],
        ]);
        const options = {
                body: body,
                method: 'POST',
        };
        var p = fetch(url, options);
        p.then(() => {}, () => {});
}

window.addEventListener('load', onLoad);
</script>
</body>
</html>

Problem

Kcgi doesn't parse the request body correctly because here, it compares the full value of the Content-Type header to application/x-www-form-urlencoded and doesn't ignore the possible parameters in the header value, like it does in the multipart/form-data case. As a result the request body is presented to the application as a single pair with the empty string as key.

Fix

The patch below solves the issue and makes the body appear as two pairs with key name and value.

--- kcgi-VERSION_0_13_3/child.c Sat Dec  2 20:50:03 2023
+++ kcgi/child.c    Thu Feb  8 11:30:13 2024
@@ -1466,7 +1466,7 @@
    }

    if (cp != NULL) {
-       if (strcasecmp(cp, "application/x-www-form-urlencoded") == 0)
+       if (strncasecmp(cp, "application/x-www-form-urlencoded", 33) == 0)
            parse_pairs_urlenc(pp, b);
        else if (strncasecmp(cp, "multipart/form-data", 19) == 0) 
            parse_multi(pp, cp + 19, b, bsz);

Probably worth applying the same fix to the text/plain case.

N.B.: I tested both problem and fix with kcgi 0.13.3 from OpenBSD ports, but this part of the code hasn't changed since the release.

kristapsdz commented 2 weeks ago

Very good catch, thank you! I've just fixed this and included some regression tests for it.