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

Unexpected percent encoding of POST data may yield wrong Content-Length #40

Open
avbentem opened this issue Dec 21, 2020 · 0 comments
Open

Comments

@avbentem
Copy link

avbentem commented Dec 21, 2020

It seems that $_POST and http_build_query are not symmetrical. Currently, $_POST is tried first when reading the original POST request:

} elseif ('POST' == $request_method) {
    $request_params = $_POST;
    if (empty($request_params)) {
        $data = file_get_contents('php://input');
        if (!empty($data)) {
            $request_params = $data;
        }
    }
} elseif ('PUT' == $request_method || 'DELETE' == $request_method) {
    $request_params = file_get_contents('php://input');
} else {
    $request_params = null;
}

And if $_POST gets some content above, then http_build_query is used when passing the request to the remote server:

// add data for POST, PUT or DELETE requests
if ('POST' == $request_method) {
    $post_data = is_array($request_params) ? http_build_query($request_params) : $request_params;
    curl_setopt($ch, CURLOPT_POST, true);
    curl_setopt($ch, CURLOPT_POSTFIELDS,  $post_data);
} elseif ('PUT' == $request_method || 'DELETE' == $request_method) {
    curl_setopt($ch, CURLOPT_CUSTOMREQUEST, $request_method);
    curl_setopt($ch, CURLOPT_POSTFIELDS, $request_params);
}

Note that $_POST percent decodes if needed, but http_build_query percent encodes whenever possible.

For example, consider Content-Type: application/x-www-form-urlencoded with a POST body including an @ that (erroneously) is not percent-encoded:

[email protected]&password=sEcr3t

Here, $_POST['user'] will simply return the original value as decoding is not needed: [email protected]. However, http_build_query will encode the @ to read %40. And the original Content-Length header will not be increased to make up for the two extra characters. Hence, the remote server may get a truncated body.

I'm not using PHP a lot, but I'd be tempted to avoid any parsing that $_POST does, and always use php://input. However:

php://input is not available with enctype="multipart/form-data"

So maybe something like the following would be better?

} elseif ('POST' == $request_method) {
    $data = file_get_contents('php://input');
    if (!empty($data)) {
        $request_params = $data;
    }
    else {
        $request_params = $_POST;
    }

(This works for me, but I have not tested with enctype="multipart/form-data".)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant