Le 02/03/2023 à 16:10, ylavic@apache.org a écrit :
> Author: ylavic
> Date: Thu Mar 2 15:10:30 2023
> New Revision: 1907980
>
> URL: http://svn.apache.org/viewvc?rev=1907980&view=rev
> Log:
> mod_proxy_uwsgi: Stricter backend HTTP response parsing/validation
>
>
> Added:
> httpd/httpd/trunk/changes-entries/proxy_uwsgi_response_validation.txt
> Modified:
> httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c
>
> Added: httpd/httpd/trunk/changes-entries/proxy_uwsgi_response_validation.txt
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/changes-entries/proxy_uwsgi_response_validation.txt?rev=1907980&view=auto
> ==============================================================================
> --- httpd/httpd/trunk/changes-entries/proxy_uwsgi_response_validation.txt (added)
> +++ httpd/httpd/trunk/changes-entries/proxy_uwsgi_response_validation.txt Thu Mar 2 15:10:30 2023
> @@ -0,0 +1,2 @@
> + *) mod_proxy_uwsgi: Stricter backend HTTP response parsing/validation.
> + [Yann Ylavic]
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c?rev=1907980&r1=1907979&r2=1907980&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c Thu Mar 2 15:10:30 2023
> @@ -313,18 +313,16 @@ static int uwsgi_response(request_rec *r
> pass_bb = apr_brigade_create(r->pool, c->bucket_alloc);
>
> len = ap_getline(buffer, sizeof(buffer), rp, 1);
> -
> if (len <= 0) {
> - /* oops */
> + /* invalid or empty */
> return HTTP_INTERNAL_SERVER_ERROR;
> }
> -
> backend->worker->s->read += len;
> -
> - if (len >= sizeof(buffer) - 1) {
> - /* oops */
> + if ((apr_size_t)len >= sizeof(buffer)) {
Hi Yann,
Why removing the -1?
My understading is that it is there in case of:
uwsgi_response()
ap_getline()
ap_rgetline()
ap_fgetline_core()
code around cleanup:
In this path, IIUC, sizeof(buffer) - 1 is returned.
Can this happen?
CJ
> + /* too long */
> return HTTP_INTERNAL_SERVER_ERROR;
> }
> +
[...]
> Author: ylavic
> Date: Thu Mar 2 15:10:30 2023
> New Revision: 1907980
>
> URL: http://svn.apache.org/viewvc?rev=1907980&view=rev
> Log:
> mod_proxy_uwsgi: Stricter backend HTTP response parsing/validation
>
>
> Added:
> httpd/httpd/trunk/changes-entries/proxy_uwsgi_response_validation.txt
> Modified:
> httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c
>
> Added: httpd/httpd/trunk/changes-entries/proxy_uwsgi_response_validation.txt
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/changes-entries/proxy_uwsgi_response_validation.txt?rev=1907980&view=auto
> ==============================================================================
> --- httpd/httpd/trunk/changes-entries/proxy_uwsgi_response_validation.txt (added)
> +++ httpd/httpd/trunk/changes-entries/proxy_uwsgi_response_validation.txt Thu Mar 2 15:10:30 2023
> @@ -0,0 +1,2 @@
> + *) mod_proxy_uwsgi: Stricter backend HTTP response parsing/validation.
> + [Yann Ylavic]
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c?rev=1907980&r1=1907979&r2=1907980&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_uwsgi.c Thu Mar 2 15:10:30 2023
> @@ -313,18 +313,16 @@ static int uwsgi_response(request_rec *r
> pass_bb = apr_brigade_create(r->pool, c->bucket_alloc);
>
> len = ap_getline(buffer, sizeof(buffer), rp, 1);
> -
> if (len <= 0) {
> - /* oops */
> + /* invalid or empty */
> return HTTP_INTERNAL_SERVER_ERROR;
> }
> -
> backend->worker->s->read += len;
> -
> - if (len >= sizeof(buffer) - 1) {
> - /* oops */
> + if ((apr_size_t)len >= sizeof(buffer)) {
Hi Yann,
Why removing the -1?
My understading is that it is there in case of:
uwsgi_response()
ap_getline()
ap_rgetline()
ap_fgetline_core()
code around cleanup:
In this path, IIUC, sizeof(buffer) - 1 is returned.
Can this happen?
CJ
> + /* too long */
> return HTTP_INTERNAL_SERVER_ERROR;
> }
> +
[...]