Mailing List Archive

On 9/27/22 12:53 PM, icing@apache.org wrote:
> Author: icing
> Date: Tue Sep 27 10:53:51 2022
> New Revision: 1904297
>
> URL: http://svn.apache.org/viewvc?rev=1904297&view=rev
> Log:
> *) mod_http2: type adjustments and castings for int/apr_uint32_t/apr_size_t/apr_off_t.
>
>
> Modified:
> httpd/httpd/trunk/modules/http2/h2_c2.c
> httpd/httpd/trunk/modules/http2/h2_mplx.c
> httpd/httpd/trunk/modules/http2/h2_mplx.h
> httpd/httpd/trunk/modules/http2/h2_proxy_session.c
> httpd/httpd/trunk/modules/http2/h2_proxy_util.c
> httpd/httpd/trunk/modules/http2/h2_push.c
> httpd/httpd/trunk/modules/http2/h2_session.c
> httpd/httpd/trunk/modules/http2/h2_session.h
> httpd/httpd/trunk/modules/http2/h2_stream.c
> httpd/httpd/trunk/modules/http2/h2_util.c
> httpd/httpd/trunk/modules/http2/h2_util.h
> httpd/httpd/trunk/modules/http2/h2_workers.c
> httpd/httpd/trunk/modules/http2/h2_workers.h
> httpd/httpd/trunk/modules/http2/mod_http2.c
>

> Modified: httpd/httpd/trunk/modules/http2/h2_stream.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_stream.c?rev=1904297&r1=1904296&r2=1904297&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http2/h2_stream.c (original)
> +++ httpd/httpd/trunk/modules/http2/h2_stream.c Tue Sep 27 10:53:51 2022
> @@ -147,7 +147,7 @@ static int on_frame(h2_stream_state_t st
> {
> ap_assert(frame_type >= 0);
> ap_assert(state >= 0);
> - if (frame_type >= maxlen) {
> + if (frame_type < 0 || (apr_size_t)frame_type >= maxlen) {

Do we need to check for frame_type < 0 with ap_assert(frame_type >= 0); two lines above?

> return state; /* NOP, ignore unknown frame types */
> }
> return on_map(state, frame_map[frame_type]);

> Modified: httpd/httpd/trunk/modules/http2/h2_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_util.c?rev=1904297&r1=1904296&r2=1904297&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http2/h2_util.c (original)
> +++ httpd/httpd/trunk/modules/http2/h2_util.c Tue Sep 27 10:53:51 2022

> @@ -1169,56 +1169,24 @@ apr_size_t h2_util_table_bytes(apr_table
> * h2_util for bucket brigades
> ******************************************************************************/
>
> -static apr_status_t last_not_included(apr_bucket_brigade *bb,
> - apr_off_t maxlen,
> - apr_bucket **pend)
> +static void fit_bucket_into(apr_bucket *b, apr_off_t *plen)
> {
> - apr_bucket *b;
> - apr_status_t status = APR_SUCCESS;
> -
> - if (maxlen >= 0) {
> - /* Find the bucket, up to which we reach maxlen/mem bytes */
> - for (b = APR_BRIGADE_FIRST(bb);
> - (b != APR_BRIGADE_SENTINEL(bb));
> - b = APR_BUCKET_NEXT(b)) {
> -
> - if (APR_BUCKET_IS_METADATA(b)) {
> - /* included */
> - }
> - else {
> - if (b->length == ((apr_size_t)-1)) {
> - const char *ign;
> - apr_size_t ilen;
> - status = apr_bucket_read(b, &ign, &ilen, APR_BLOCK_READ);
> - if (status != APR_SUCCESS) {
> - return status;
> - }
> - }
> -
> - if (maxlen == 0 && b->length > 0) {
> - *pend = b;
> - return status;
> - }
> -
> - if (APR_BUCKET_IS_FILE(b)
> -#if APR_HAS_MMAP
> - || APR_BUCKET_IS_MMAP(b)
> -#endif
> - ) {
> - /* we like to move it, always */
> - }
> - else if (maxlen < (apr_off_t)b->length) {
> - apr_bucket_split(b, (apr_size_t)maxlen);
> - maxlen = 0;
> - }
> - else {
> - maxlen -= b->length;
> - }
> - }
> - }
> + /* signed apr_off_t is at least as large as unsigned apr_size_t.
> + * Propblems may arise when they are both the same size. Then

Problems instead of Propblems :-)

> + * the bucket length *may* be larger than a value we can hold
> + * in apr_off_t. Before casting b->length to apr_off_t we must
> + * check the limitations.
> + * After we resized the bucket, it is safe to cast and substract.
> + */
> + if ((sizeof(apr_off_t) == sizeof(apr_int64_t)
> + && b->length > APR_INT64_MAX)
> + || (sizeof(apr_off_t) == sizeof(apr_int32_t)
> + && b->length > APR_INT32_MAX)
> + || *plen < (apr_off_t)b->length) {
> + /* bucket is longer the *plen */
> + apr_bucket_split(b, *plen);
> }
> - *pend = APR_BRIGADE_SENTINEL(bb);
> - return status;
> + *plen -= (apr_off_t)b->length;
> }
>


Regards

Rüdiger