Return-Path: X-Original-To: apmail-httpd-dev-archive@www.apache.org Delivered-To: apmail-httpd-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 44637187F8 for ; Wed, 16 Sep 2015 14:51:06 +0000 (UTC) Received: (qmail 39815 invoked by uid 500); 16 Sep 2015 14:51:05 -0000 Delivered-To: apmail-httpd-dev-archive@httpd.apache.org Received: (qmail 39753 invoked by uid 500); 16 Sep 2015 14:51:05 -0000 Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm Precedence: bulk Reply-To: dev@httpd.apache.org list-help: list-unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@httpd.apache.org Received: (qmail 39743 invoked by uid 99); 16 Sep 2015 14:51:05 -0000 Received: from Unknown (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 16 Sep 2015 14:51:05 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 5A254F32F2 for ; Wed, 16 Sep 2015 14:51:05 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.898 X-Spam-Level: ** X-Spam-Status: No, score=2.898 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=3, RCVD_IN_MSPIKE_H2=-0.001, SPF_PASS=-0.001] autolearn=disabled Authentication-Results: spamd1-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mx1-eu-west.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id WvIjN4KI2G7i for ; Wed, 16 Sep 2015 14:51:04 +0000 (UTC) Received: from mail-vk0-f48.google.com (mail-vk0-f48.google.com [209.85.213.48]) by mx1-eu-west.apache.org (ASF Mail Server at mx1-eu-west.apache.org) with ESMTPS id 8AED723123 for ; Wed, 16 Sep 2015 14:51:03 +0000 (UTC) Received: by vkfp126 with SMTP id p126so102450588vkf.3 for ; Wed, 16 Sep 2015 07:50:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=nlevix6p118tOrZwcYzjpUvswRTfAwqKz7MvO7soLLk=; b=nWEZ5WTbERRIYIXXyXxpeGuepOCgsVv1f5W+W3amNVcyKmNRVH3KtD4S+0rD5C2KRz +nhz9nmBKlxTm9fr0WjXSKMfUP7Q+12FTNhk9nhLaDt4NeZ9PYrlMMYwajx5u7j+GuNL XzKDty+aDSBXQhZOmOD6paS4Gm3eFqKabA0bcAXfPiqiZhv+XQS10ogbz3kn8bgERWAl 0XjrvQJgYdqs7zF0ODU6mrj9pZvnFmY2Z3S0kf6VaoDaJZoMD4dPrm/6B0TENPcjcxS7 bYi87+qbiVDLNKfrDRRwy6v/4PKBJoRheiOGETKRlH8gpVJvqYHXI9ByYr0DhRjrLPhC TC9g== MIME-Version: 1.0 X-Received: by 10.31.152.74 with SMTP id a71mr30187608vke.116.1442415055849; Wed, 16 Sep 2015 07:50:55 -0700 (PDT) Received: by 10.31.180.134 with HTTP; Wed, 16 Sep 2015 07:50:55 -0700 (PDT) In-Reply-To: <20150916141850.82DDAAC022D@hades.apache.org> References: <20150916141850.82DDAAC022D@hades.apache.org> Date: Wed, 16 Sep 2015 09:50:55 -0500 Message-ID: Subject: Re: svn commit: r1703415 - in /httpd/test/framework/trunk/c-modules: authany/mod_authany.c test_session/mod_test_session.c From: Greg Stein To: dev@httpd.apache.org Content-Type: multipart/alternative; boundary=001a1141ecb005cda3051fde6dcb --001a1141ecb005cda3051fde6dcb Content-Type: text/plain; charset=UTF-8 On Wed, Sep 16, 2015 at 9:18 AM, wrote: > Author: jim > Date: Wed Sep 16 14:18:49 2015 > New Revision: 1703415 > >... > Modified: > httpd/test/framework/trunk/c-modules/test_session/mod_test_session.c > URL: > http://svn.apache.org/viewvc/httpd/test/framework/trunk/c-modules/test_session/mod_test_session.c?rev=1703415&r1=1703414&r2=1703415&view=diff > > ============================================================================== > --- httpd/test/framework/trunk/c-modules/test_session/mod_test_session.c > (original) > +++ httpd/test/framework/trunk/c-modules/test_session/mod_test_session.c > Wed Sep 16 14:18:49 2015 > @@ -149,7 +149,6 @@ static apr_status_t test_session_encode( > > static apr_status_t test_session_decode(request_rec * r, session_rec * z) > { > - apr_status_t result = OK; > const size_t prefix_len = strlen(TEST_SESSION_ENCODING_PREFIX); > test_session_dcfg_t *dconf = ap_get_module_config(r->per_dir_config, > > &test_session_module); > @@ -203,7 +202,7 @@ static int test_session_handler(request_ > return DECLINED; > > /* Copy the header for SessionHeader from the request to the > response. */ > - if (overrides = apr_table_get(r->headers_in, TEST_SESSION_HEADER)) > + if ((overrides = apr_table_get(r->headers_in, TEST_SESSION_HEADER))) > apr_table_setn(r->headers_out, TEST_SESSION_HEADER, overrides); > > /* Additional commands to test the session API via POST. */ > @@ -240,15 +239,15 @@ static int test_session_handler(request_ > } > else if (!strcmp(pair->name, "name")) { > apr_size_t len; > - apr_brigade_length(pair->value, 1, &len); > + apr_brigade_length(pair->value, 1, (apr_off_t *)&len); > This seems really dangerous. Aren't there cases where sizeof(apr_size_t) != sizeof(apr_off_t) ?? > fieldName = apr_pcalloc(r->pool, sizeof(char) * len + 1); > - result = apr_brigade_flatten(pair->value, fieldName, > &len); > + result = apr_brigade_flatten(pair->value, fieldName, > (apr_size_t *)&len); > This seems unnecessary. &len should already be apr_size_t * > } > else if (!strcmp(pair->name, "value")) { > apr_size_t len; > - apr_brigade_length(pair->value, 1, &len); > + apr_brigade_length(pair->value, 1, (apr_off_t *)&len); > fieldValue = apr_pcalloc(r->pool, sizeof(char) * len + 1); > - result = apr_brigade_flatten(pair->value, fieldValue, > &len); > + result = apr_brigade_flatten(pair->value, fieldValue, > (apr_size_t *)&len); > These two, same as above. Heh. I haven't reviewed httpd code for years. But I saw a change to mod_authany.c and thought "what is that? isn't that module crazy stable?" ... curiosity :-P Cheers, -g --001a1141ecb005cda3051fde6dcb Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On W= ed, Sep 16, 2015 at 9:18 AM, <jim@apache.org> wrote:
Author: jim
Date: Wed Sep 16 14:18:49 2015
New Revision: 1703415
>...=C2=A0
Modified: httpd/test/framework/trunk/c-modules/test_sessi= on/mod_test_session.c
URL: http://sv= n.apache.org/viewvc/httpd/test/framework/trunk/c-modules/test_session/mod_t= est_session.c?rev=3D1703415&r1=3D1703414&r2=3D1703415&view=3Ddi= ff
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D
--- httpd/test/framework/trunk/c-modules/test_session/mod_test_session.c (o= riginal)
+++ httpd/test/framework/trunk/c-modules/test_session/mod_test_session.c We= d Sep 16 14:18:49 2015
@@ -149,7 +149,6 @@ static apr_status_t test_session_encode(

=C2=A0static apr_status_t test_session_decode(request_rec * r, session_rec = * z)
=C2=A0{
-=C2=A0 =C2=A0 apr_status_t result =3D OK;
=C2=A0 =C2=A0 =C2=A0const size_t prefix_len =3D strlen(TEST_SESSION_ENCODIN= G_PREFIX);
=C2=A0 =C2=A0 =C2=A0test_session_dcfg_t *dconf =3D ap_get_module_config(r-&= gt;per_dir_config,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0&test_session_module);<= br> @@ -203,7 +202,7 @@ static int test_session_handler(request_
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return DECLINED;

=C2=A0 =C2=A0 =C2=A0/* Copy the header for SessionHeader from the request t= o the response. */
-=C2=A0 =C2=A0 if (overrides =3D apr_table_get(r->headers_in, TEST_SESSI= ON_HEADER))
+=C2=A0 =C2=A0 if ((overrides =3D apr_table_get(r->headers_in, TEST_SESS= ION_HEADER)))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0apr_table_setn(r->headers_out, TEST_SE= SSION_HEADER, overrides);

=C2=A0 =C2=A0 =C2=A0/* Additional commands to test the session API via POST= . */
@@ -240,15 +239,15 @@ static int test_session_handler(request_
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else if (!strcmp(pair->n= ame, "name")) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0apr_size_t le= n;
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 apr_brigade_length= (pair->value, 1, &len);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 apr_brigade_length= (pair->value, 1, (apr_off_t *)&len);

=
This seems really dangerous. Aren't there cases where sizeof(apr_s= ize_t) !=3D sizeof(apr_off_t) ??
=C2=A0
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fieldName =3D= apr_pcalloc(r->pool, sizeof(char) * len + 1);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 result =3D apr_bri= gade_flatten(pair->value, fieldName, &len);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 result =3D apr_bri= gade_flatten(pair->value, fieldName, (apr_size_t *)&len);

This seems unnecessary. &len should already b= e apr_size_t *
=C2=A0
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else if (!strcmp(pair->n= ame, "value")) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0apr_size_t le= n;
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 apr_brigade_length= (pair->value, 1, &len);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 apr_brigade_length= (pair->value, 1, (apr_off_t *)&len);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fieldValue = =3D apr_pcalloc(r->pool, sizeof(char) * len + 1);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 result =3D apr_bri= gade_flatten(pair->value, fieldValue, &len);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 result =3D apr_bri= gade_flatten(pair->value, fieldValue, (apr_size_t *)&len);

These two, same as above.

Heh. I haven't reviewed httpd code for years. But I saw a change to = mod_authany.c and thought "what is that? isn't that module crazy s= table?" ... curiosity :-P

Cheers,
-= g

--001a1141ecb005cda3051fde6dcb--