Skip to content

Commit a6b8db0

Browse files
covenercovener
committed
improve h2 header error handling
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1927038 13f79535-47bb-0310-9956-ffa450edef68
1 parent 63f48bb commit a6b8db0

File tree

8 files changed

+130
-59
lines changed

8 files changed

+130
-59
lines changed

‎modules/http2/h2_request.c‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,20 @@ typedef struct {
6464
apr_table_t *headers;
6565
apr_pool_t *pool;
6666
apr_status_t status;
67+
h2_hd_scratch *scratch;
6768
} h1_ctx;
6869

6970
static int set_h1_header(void *ctx, const char *key, const char *value)
7071
{
7172
h1_ctx *x = ctx;
7273
int was_added;
73-
h2_req_add_header(x->headers, x->pool, key, strlen(key), value, strlen(value), 0, &was_added);
74+
h2_req_add_header(x->headers, x->pool, key, strlen(key),
75+
value, strlen(value), x->scratch, &was_added);
7476
return 1;
7577
}
7678

7779
apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool,
78-
request_rec *r)
80+
request_rec *r, h2_hd_scratch *scratch)
7981
{
8082
h2_request *req;
8183
const char *scheme, *authority, *path;
@@ -125,6 +127,7 @@ apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool,
125127
x.pool = pool;
126128
x.headers = req->headers;
127129
x.status = APR_SUCCESS;
130+
x.scratch = scratch;
128131
apr_table_do(set_h1_header, &x, r->headers_in, NULL);
129132

130133
*preq = req;
@@ -134,7 +137,8 @@ apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool,
134137
apr_status_t h2_request_add_header(h2_request *req, apr_pool_t *pool,
135138
const char *name, size_t nlen,
136139
const char *value, size_t vlen,
137-
size_t max_field_len, int *pwas_added)
140+
struct h2_hd_scratch *scratch,
141+
int *pwas_added)
138142
{
139143
apr_status_t status = APR_SUCCESS;
140144

@@ -185,7 +189,7 @@ apr_status_t h2_request_add_header(h2_request *req, apr_pool_t *pool,
185189
else {
186190
/* non-pseudo header, add to table */
187191
status = h2_req_add_header(req->headers, pool, name, nlen, value, vlen,
188-
max_field_len, pwas_added);
192+
scratch, pwas_added);
189193
}
190194

191195
return status;

‎modules/http2/h2_request.h‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,21 @@
1919

2020
#include "h2.h"
2121

22+
struct h2_hd_scratch;
23+
2224
h2_request *h2_request_create(int id, apr_pool_t *pool, const char *method,
2325
const char *scheme, const char *authority,
2426
const char *path, apr_table_t *header);
2527

2628
apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool,
27-
request_rec *r);
29+
request_rec *r,
30+
struct h2_hd_scratch *scratch);
2831

2932
apr_status_t h2_request_add_header(h2_request *req, apr_pool_t *pool,
3033
const char *name, size_t nlen,
3134
const char *value, size_t vlen,
32-
size_t max_field_len, int *pwas_added);
35+
struct h2_hd_scratch *scratch,
36+
int *pwas_added);
3337

3438
apr_status_t h2_request_add_trailer(h2_request *req, apr_pool_t *pool,
3539
const char *name, size_t nlen,

‎modules/http2/h2_session.c‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,29 @@ static void cleanup_unprocessed_streams(h2_session *session)
109109
h2_mplx_c1_streams_do(session->mplx, rst_unprocessed_stream, session);
110110
}
111111

112+
/* APR callback invoked if allocation fails. */
113+
static int abort_on_oom(int retcode)
114+
{
115+
ap_abort_on_oom();
116+
return retcode; /* unreachable, hopefully. */
117+
}
118+
112119
static h2_stream *h2_session_open_stream(h2_session *session, int stream_id,
113120
int initiated_on)
114121
{
115122
h2_stream * stream;
123+
apr_allocator_t *allocator;
116124
apr_pool_t *stream_pool;
125+
apr_status_t rv;
117126

118-
apr_pool_create(&stream_pool, session->pool);
127+
rv = apr_allocator_create(&allocator);
128+
if (rv != APR_SUCCESS)
129+
return NULL;
130+
131+
apr_allocator_max_free_set(allocator, ap_max_mem_free);
132+
apr_pool_create_ex(&stream_pool, session->pool, NULL, allocator);
133+
apr_allocator_owner_set(allocator, stream_pool);
134+
apr_pool_abort_set(abort_on_oom, stream_pool);
119135
apr_pool_tag(stream_pool, "h2_stream");
120136

121137
stream = h2_stream_create(stream_id, stream_pool, session,
@@ -972,6 +988,14 @@ apr_status_t h2_session_create(h2_session **psession, conn_rec *c, request_rec *
972988
}
973989

974990
h2_c1_io_init(&session->io, session);
991+
/* setup request header scratch buffers */
992+
session->hd_scratch.max_len = session->s->limit_req_fieldsize?
993+
session->s->limit_req_fieldsize : 8190;
994+
session->hd_scratch.name =
995+
apr_pcalloc(session->pool, session->hd_scratch.max_len + 1);
996+
session->hd_scratch.value =
997+
apr_pcalloc(session->pool, session->hd_scratch.max_len + 1);
998+
975999
session->padding_max = h2_config_sgeti(s, H2_CONF_PADDING_BITS);
9761000
if (session->padding_max) {
9771001
session->padding_max = (0x01 << session->padding_max) - 1;
@@ -1032,7 +1056,7 @@ apr_status_t h2_session_create(h2_session **psession, conn_rec *c, request_rec *
10321056

10331057
n = h2_config_sgeti(s, H2_CONF_PUSH_DIARY_SIZE);
10341058
session->push_diary = h2_push_diary_create(session->pool, n);
1035-
1059+
10361060
if (APLOGcdebug(c)) {
10371061
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c,
10381062
H2_SSSN_LOG(APLOGNO(03200), session,
@@ -1699,9 +1723,10 @@ static void on_stream_state_enter(void *ctx, h2_stream *stream)
16991723
break;
17001724
case H2_SS_CLEANUP:
17011725
nghttp2_session_set_stream_user_data(session->ngh2, stream->id, NULL);
1726+
update_child_status(session, SERVER_BUSY_WRITE, "done", stream);
17021727
h2_mplx_c1_stream_cleanup(session->mplx, stream, &session->open_streams);
1728+
stream = NULL;
17031729
++session->streams_done;
1704-
update_child_status(session, SERVER_BUSY_WRITE, "done", stream);
17051730
break;
17061731
default:
17071732
break;

‎modules/http2/h2_session.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
*/
3030

3131
#include "h2.h"
32+
#include "h2_util.h"
3233

3334
struct apr_thread_mutext_t;
3435
struct apr_thread_cond_t;
@@ -118,6 +119,8 @@ typedef struct h2_session {
118119
struct h2_iqueue *out_c1_blocked; /* all streams with output blocked on c1 buffer full */
119120
struct h2_iqueue *ready_to_process; /* all streams ready for processing */
120121

122+
h2_hd_scratch hd_scratch;
123+
121124
} h2_session;
122125

123126
const char *h2_session_state_str(h2_session_state state);

‎modules/http2/h2_stream.c‎

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,8 @@ apr_status_t h2_stream_set_request_rec(h2_stream *stream,
659659
if (stream->rst_error) {
660660
return APR_ECONNRESET;
661661
}
662-
status = h2_request_rcreate(&req, stream->pool, r);
662+
status = h2_request_rcreate(&req, stream->pool, r,
663+
&stream->session->hd_scratch);
663664
if (status == APR_SUCCESS) {
664665
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r,
665666
H2_STRM_LOG(APLOGNO(03058), stream,
@@ -691,13 +692,11 @@ static void set_error_response(h2_stream *stream, int http_status)
691692
static apr_status_t add_trailer(h2_stream *stream,
692693
const char *name, size_t nlen,
693694
const char *value, size_t vlen,
694-
size_t max_field_len, int *pwas_added)
695+
h2_hd_scratch *scratch)
695696
{
696697
conn_rec *c = stream->session->c1;
697-
char *hname, *hvalue;
698698
const char *existing;
699699

700-
*pwas_added = 0;
701700
if (nlen == 0 || name[0] == ':') {
702701
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, APR_EINVAL, c,
703702
H2_STRM_LOG(APLOGNO(03060), stream,
@@ -710,20 +709,35 @@ static apr_status_t add_trailer(h2_stream *stream,
710709
if (!stream->trailers_in) {
711710
stream->trailers_in = apr_table_make(stream->pool, 5);
712711
}
713-
hname = apr_pstrndup(stream->pool, name, nlen);
714-
h2_util_camel_case_header(hname, nlen);
715-
existing = apr_table_get(stream->trailers_in, hname);
716-
if (max_field_len
717-
&& ((existing? strlen(existing)+2 : 0) + vlen + nlen + 2 > max_field_len)) {
718-
/* "key: (oldval, )?nval" is too long */
712+
713+
if (((nlen + vlen + 2) > scratch->max_len))
719714
return APR_EINVAL;
715+
716+
/* We need 0-terminated strings to operate on apr_table */
717+
AP_DEBUG_ASSERT(nlen < scratch->max_len);
718+
memcpy(scratch->name, name, nlen);
719+
scratch->name[nlen] = 0;
720+
AP_DEBUG_ASSERT(vlen < scratch->max_len);
721+
memcpy(scratch->value, value, vlen);
722+
scratch->value[vlen] = 0;
723+
724+
existing = apr_table_get(stream->trailers_in, scratch->name);
725+
if(existing) {
726+
if (!vlen) /* not adding a 0-length value to existing */
727+
return APR_SUCCESS;
728+
if ((strlen(existing) + 2 + vlen + nlen + 2 > scratch->max_len)) {
729+
/* "name: existing, value" is too long */
730+
return APR_EINVAL;
731+
}
732+
apr_table_merge(stream->trailers_in, scratch->name, scratch->value);
720733
}
721-
if (!existing) *pwas_added = 1;
722-
hvalue = apr_pstrndup(stream->pool, value, vlen);
723-
apr_table_mergen(stream->trailers_in, hname, hvalue);
724-
ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
725-
H2_STRM_MSG(stream, "added trailer '%s: %s'"), hname, hvalue);
726-
734+
else {
735+
h2_util_camel_case_header(scratch->name, nlen);
736+
apr_table_set(stream->trailers_in, scratch->name, scratch->value);
737+
}
738+
ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
739+
H2_STRM_MSG(stream, "added trailer '%s: %s'"),
740+
scratch->name, scratch->value);
727741
return APR_SUCCESS;
728742
}
729743

@@ -732,7 +746,7 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
732746
const char *value, size_t vlen)
733747
{
734748
h2_session *session = stream->session;
735-
int error = 0, was_added = 0;
749+
int error = 0;
736750
apr_status_t status = APR_SUCCESS;
737751

738752
H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
@@ -760,6 +774,7 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
760774
++stream->request_headers_added;
761775
}
762776
else if (H2_SS_IDLE == stream->state) {
777+
int was_added;
763778
if (!stream->rtmp) {
764779
if (H2_STREAM_CLIENT_INITIATED(stream->id)) {
765780
++stream->session->remote.emitted_count;
@@ -771,16 +786,16 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
771786
}
772787
status = h2_request_add_header(stream->rtmp, stream->pool,
773788
name, nlen, value, vlen,
774-
session->s->limit_req_fieldsize, &was_added);
789+
&session->hd_scratch, &was_added);
775790
ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, session->c1,
776791
H2_STRM_MSG(stream, "add_header: '%.*s: %.*s"),
777792
(int)nlen, name, (int)vlen, value);
778793
if (was_added) ++stream->request_headers_added;
779794
}
780795
else if (H2_SS_OPEN == stream->state) {
781796
status = add_trailer(stream, name, nlen, value, vlen,
782-
session->s->limit_req_fieldsize, &was_added);
783-
if (was_added) ++stream->request_headers_added;
797+
&session->hd_scratch);
798+
if (!status) ++stream->request_headers_added;
784799
}
785800
else {
786801
status = APR_EINVAL;
@@ -789,16 +804,17 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
789804

790805
if (APR_EINVAL == status) {
791806
/* header too long */
792-
if (!h2_stream_is_ready(stream)) {
807+
if (!h2_stream_is_ready(stream) && !stream->request_headers_failed) {
793808
ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, session->c1,
794-
H2_STRM_LOG(APLOGNO(10180), stream,"Request header exceeds "
795-
"LimitRequestFieldSize: %.*s"),
809+
H2_STRM_LOG(APLOGNO(10180), stream,
810+
"Request header exceeds LimitRequestFieldSize(%d): %.*s"),
811+
(int)session->hd_scratch.max_len,
796812
(int)H2MIN(nlen, 80), name);
797813
}
798814
error = HTTP_REQUEST_HEADER_FIELDS_TOO_LARGE;
799815
goto cleanup;
800816
}
801-
817+
802818
if (session->s->limit_req_fields > 0
803819
&& stream->request_headers_added > session->s->limit_req_fields) {
804820
/* too many header lines */
@@ -810,12 +826,13 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
810826
if (!h2_stream_is_ready(stream)) {
811827
ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, session->c1,
812828
H2_STRM_LOG(APLOGNO(10181), stream, "Number of request headers "
813-
"exceeds LimitRequestFields"));
829+
"exceeds LimitRequestFields(%d)"),
830+
(int)session->s->limit_req_fields);
814831
}
815832
error = HTTP_REQUEST_HEADER_FIELDS_TOO_LARGE;
816833
goto cleanup;
817834
}
818-
835+
819836
cleanup:
820837
if (error) {
821838
++stream->request_headers_failed;

‎modules/http2/h2_util.c‎

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,10 +1693,9 @@ int h2_ignore_resp_trailer(const char *name, size_t len)
16931693
}
16941694

16951695
static apr_status_t req_add_header(apr_table_t *headers, apr_pool_t *pool,
1696-
nghttp2_nv *nv, size_t max_field_len,
1696+
nghttp2_nv *nv, h2_hd_scratch *scratch,
16971697
int *pwas_added)
16981698
{
1699-
char *hname, *hvalue;
17001699
const char *existing;
17011700

17021701
*pwas_added = 0;
@@ -1712,15 +1711,14 @@ static apr_status_t req_add_header(apr_table_t *headers, apr_pool_t *pool,
17121711
/* Cookie header come separately in HTTP/2, but need
17131712
* to be merged by "; " (instead of default ", ")
17141713
*/
1715-
if (max_field_len
1716-
&& strlen(existing) + nv->valuelen + nv->namelen + 4
1717-
> max_field_len) {
1714+
if ((strlen(existing) + nv->valuelen + nv->namelen + 4)
1715+
> scratch->max_len) {
17181716
/* "key: oldval, nval" is too long */
17191717
return APR_EINVAL;
17201718
}
1721-
hvalue = apr_pstrndup(pool, (const char*)nv->value, nv->valuelen);
17221719
apr_table_setn(headers, "Cookie",
1723-
apr_psprintf(pool, "%s; %s", existing, hvalue));
1720+
apr_psprintf(pool, "%s; %.*s", existing,
1721+
(int)nv->valuelen, nv->value));
17241722
return APR_SUCCESS;
17251723
}
17261724
}
@@ -1731,35 +1729,48 @@ static apr_status_t req_add_header(apr_table_t *headers, apr_pool_t *pool,
17311729
}
17321730
}
17331731

1734-
hname = apr_pstrndup(pool, (const char*)nv->name, nv->namelen);
1735-
h2_util_camel_case_header(hname, nv->namelen);
1736-
existing = apr_table_get(headers, hname);
1737-
if (max_field_len) {
1738-
if ((existing? strlen(existing)+2 : 0) + nv->valuelen + nv->namelen + 2
1739-
> max_field_len) {
1740-
/* "key: (oldval, )?nval" is too long */
1732+
if (((nv->namelen + nv->valuelen + 2) > scratch->max_len))
1733+
return APR_EINVAL;
1734+
1735+
/* We need 0-terminated strings to operate on apr_table */
1736+
AP_DEBUG_ASSERT(nv->namelen < scratch->max_len);
1737+
memcpy(scratch->name, nv->name, nv->namelen);
1738+
scratch->name[nv->namelen] = 0;
1739+
AP_DEBUG_ASSERT(nv->valuelen < scratch->max_len);
1740+
memcpy(scratch->value, nv->value, nv->valuelen);
1741+
scratch->value[nv->valuelen] = 0;
1742+
1743+
*pwas_added = 1;
1744+
existing = apr_table_get(headers, scratch->name);
1745+
if (existing) {
1746+
if (!nv->valuelen) /* not adding a 0-length value to existing */
1747+
return APR_SUCCESS;
1748+
if ((strlen(existing) + 2 + nv->valuelen + nv->namelen + 2)
1749+
> scratch->max_len) {
1750+
/* "name: existing, value" is too long */
17411751
return APR_EINVAL;
17421752
}
1753+
apr_table_merge(headers, scratch->name, scratch->value);
1754+
}
1755+
else {
1756+
h2_util_camel_case_header(scratch->name, nv->namelen);
1757+
apr_table_set(headers, scratch->name, scratch->value);
17431758
}
1744-
if (!existing) *pwas_added = 1;
1745-
hvalue = apr_pstrndup(pool, (const char*)nv->value, nv->valuelen);
1746-
apr_table_mergen(headers, hname, hvalue);
1747-
17481759
return APR_SUCCESS;
17491760
}
17501761

17511762
apr_status_t h2_req_add_header(apr_table_t *headers, apr_pool_t *pool,
17521763
const char *name, size_t nlen,
17531764
const char *value, size_t vlen,
1754-
size_t max_field_len, int *pwas_added)
1765+
h2_hd_scratch *scratch, int *pwas_added)
17551766
{
17561767
nghttp2_nv nv;
17571768

17581769
nv.name = (uint8_t*)name;
17591770
nv.namelen = nlen;
17601771
nv.value = (uint8_t*)value;
17611772
nv.valuelen = vlen;
1762-
return req_add_header(headers, pool, &nv, max_field_len, pwas_added);
1773+
return req_add_header(headers, pool, &nv, scratch, pwas_added);
17631774
}
17641775

17651776
/*******************************************************************************

0 commit comments

Comments
 (0)