Hi all,
I would appreciate some other eyes on the patch below.
I guess that that the fix is correct, but I don't know the possible
implication of the fix.
As said in the commit description, -1 seems to be a valid length, but I
don't know if such buckets can happen here.
So this patch can change the behavior of the code and trigger a path
that was never executed before.
Comments from s.o. with deeper understanding of filters are welcomed.
Should the commit description be tweaked, please do so.
CJ
Le 10/08/2019 à 11:52, jaillet...@apache.org a écrit :
Author: jailletc36
Date: Sat Aug 10 09:52:34 2019
New Revision: 1864868
URL: http://svn.apache.org/viewvc?rev=1864868&view=rev
Log:
Fix a signed/unsigned comparison that can never match.
-1 is a valid length value (for socket, pipe and cgi buckets for example)
All path I've checked cast the -1 to (apr_size_t) in order for the comparison
to work. So do it as well here.
This has been like that in trunk since r708144, about 11 years ago, so I assume
that it is not really an issue.
Spotted by gcc 9.1 and -Wextra
Modified:
httpd/httpd/trunk/server/core_filters.c
Modified: httpd/httpd/trunk/server/core_filters.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core_filters.c?rev=1864868&r1=1864867&r2=1864868&view=diff
==============================================================================
--- httpd/httpd/trunk/server/core_filters.c (original)
+++ httpd/httpd/trunk/server/core_filters.c Sat Aug 10 09:52:34 2019
@@ -277,7 +277,7 @@ apr_status_t ap_core_input_filter(ap_fil
while ((len < readbytes) && (rv == APR_SUCCESS)
&& (e != APR_BRIGADE_SENTINEL(ctx->bb))) {
/* Check for the availability of buckets with known length */
- if (e->length != -1) {
+ if (e->length != (apr_size_t)-1) {
len += e->length;
e = APR_BUCKET_NEXT(e);
}
