reduced h2_request initialization/copy after review by CJ

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1738563 13f79535-47bb-0310-9956-ffa450edef68

modules/http2/h2_io.c

@@ -43,7 +43,7 @@ h2_io *h2_io_create(int id, apr_pool_t *pool,
         io->id = id;
         io->pool = pool;
         io->bucket_alloc = bucket_alloc;
-        io->request = h2_request_clone(pool, request);
+        io->request = request;
     }
     return io;
 }

modules/http2/h2_request.c

@@ -235,7 +235,8 @@ apr_status_t h2_request_end_headers(h2_request *req, apr_pool_t *pool,
     const char *s;
     
     if (req->eoh) {
-        return APR_EINVAL;
+        /* already done */
+        return APR_SUCCESS;
     }
 
     /* rfc7540, ch. 8.1.2.3:
@@ -337,37 +338,18 @@ apr_status_t h2_request_add_trailer(h2_request *req, apr_pool_t *pool,
     return add_h1_trailer(req, pool, name, nlen, value, vlen);
 }
 
-#define OPT_COPY(p, s)  ((s)? apr_pstrdup(p, s) : NULL)
-
-void h2_request_copy(apr_pool_t *p, h2_request *dst, const h2_request *src)
-{
-    /* keep the dst id */
-    dst->initiated_on   = src->initiated_on;
-    dst->method         = OPT_COPY(p, src->method);
-    dst->scheme         = OPT_COPY(p, src->scheme);
-    dst->authority      = OPT_COPY(p, src->authority);
-    dst->path           = OPT_COPY(p, src->path);
-    dst->headers        = apr_table_clone(p, src->headers);
-    if (src->trailers) {
-        dst->trailers   = apr_table_clone(p, src->trailers);
-    }
-    else {
-        dst->trailers   = NULL;
-    }
-    dst->content_length = src->content_length;
-    dst->chunked        = src->chunked;
-    dst->eoh            = src->eoh;
-    dst->body           = src->body;
-    dst->serialize      = src->serialize;
-    dst->push_policy    = src->push_policy;
-}
-
 h2_request *h2_request_clone(apr_pool_t *p, const h2_request *src)
 {
-    h2_request *nreq = apr_pcalloc(p, sizeof(*nreq));
+    h2_request *dst = apr_pmemdup(p, src, sizeof(*dst));
-    memcpy(nreq, src, sizeof(*nreq));
+    dst->method       = apr_pstrdup(p, src->method);
-    h2_request_copy(p, nreq, src);
+    dst->scheme       = apr_pstrdup(p, src->scheme);
-    return nreq;
+    dst->authority    = apr_pstrdup(p, src->authority);
+    dst->path         = apr_pstrdup(p, src->path);
+    dst->headers      = apr_table_clone(p, src->headers);
+    if (src->trailers) {
+        dst->trailers = apr_table_clone(p, src->trailers);
+    }
+    return dst;
 }
 
 request_rec *h2_request_create_rec(const h2_request *req, conn_rec *c)

modules/http2/h2_request.h

@@ -43,8 +43,6 @@ apr_status_t h2_request_add_trailer(h2_request *req, apr_pool_t *pool,
 apr_status_t h2_request_end_headers(h2_request *req, apr_pool_t *pool, 
                                     int eos, int push);
 
-void h2_request_copy(apr_pool_t *p, h2_request *dst, const h2_request *src);
-
 h2_request *h2_request_clone(apr_pool_t *p, const h2_request *src);
 
 /**

modules/http2/h2_session.c

@@ -121,7 +121,8 @@ static void cleanup_streams(h2_session *session)
     }
 }
 
-h2_stream *h2_session_open_stream(h2_session *session, int stream_id)
+h2_stream *h2_session_open_stream(h2_session *session, int stream_id,
+                                  int initiated_on, const h2_request *req)
 {
     h2_stream * stream;
     apr_pool_t *stream_pool;
@@ -135,7 +136,8 @@ h2_stream *h2_session_open_stream(h2_session *session, int stream_id)
         apr_pool_tag(stream_pool, "h2_stream");
     }
     
-    stream = h2_stream_open(stream_id, stream_pool, session);
+    stream = h2_stream_open(stream_id, stream_pool, session, 
+                            initiated_on, req);
     
     h2_ihash_add(session->streams, stream);
     if (H2_STREAM_CLIENT_INITIATED(stream_id)) {
@@ -360,7 +362,7 @@ static int on_begin_headers_cb(nghttp2_session *ngh2,
         /* nop */
     }
     else {
-        s = h2_session_open_stream((h2_session *)userp, frame->hd.stream_id);
+        s = h2_session_open_stream(userp, frame->hd.stream_id, 0, NULL);
     }
     return s? 0 : NGHTTP2_ERR_START_STREAM_NOT_ALLOWED;
 }
@@ -1030,7 +1032,7 @@ static apr_status_t h2_session_start(h2_session *session, int *rv)
         }
         
         /* Now we need to auto-open stream 1 for the request we got. */
-        stream = h2_session_open_stream(session, 1);
+        stream = h2_session_open_stream(session, 1, 0, NULL);
         if (!stream) {
             status = APR_EGENERAL;
             ap_log_rerror(APLOG_MARK, APLOG_ERR, status, session->r,
@@ -1372,9 +1374,8 @@ struct h2_stream *h2_session_push(h2_session *session, h2_stream *is,
                   session->id, is->id, nid,
                   push->req->method, push->req->path, is->id);
                   
-    stream = h2_session_open_stream(session, nid);
+    stream = h2_session_open_stream(session, nid, is->id, push->req);
     if (stream) {
-        h2_stream_set_h2_request(stream, is->id, push->req);
         status = stream_schedule(session, stream, 1);
         if (status != APR_SUCCESS) {
             ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, session->c,

modules/http2/h2_session.h

@@ -198,9 +198,14 @@ struct h2_stream *h2_session_get_stream(h2_session *session, int stream_id);
  * 
  * @param session the session to register in
  * @param stream_id the new stream identifier
+ * @param initiated_on the stream id this one is initiated on or 0
+ * @param req the request for this stream or NULL if not known yet
  * @return the new stream
  */
-struct h2_stream *h2_session_open_stream(h2_session *session, int stream_id);
+struct h2_stream *h2_session_open_stream(h2_session *session, int stream_id,
+                                         int initiated_on, 
+                                         const h2_request *req);
+
 
 /**
  * Returns if client settings have push enabled.

modules/http2/h2_stream.c

@@ -137,16 +137,29 @@ static int output_open(h2_stream *stream)
 
 static h2_sos *h2_sos_mplx_create(h2_stream *stream, h2_response *response);
 
-h2_stream *h2_stream_open(int id, apr_pool_t *pool, h2_session *session)
+h2_stream *h2_stream_open(int id, apr_pool_t *pool, h2_session *session,
+                          int initiated_on, const h2_request *creq)
 {
+    h2_request *req;
     h2_stream *stream = apr_pcalloc(pool, sizeof(h2_stream));
+    
     stream->id        = id;
     stream->state     = H2_STREAM_ST_IDLE;
     stream->pool      = pool;
     stream->session   = session;
     set_state(stream, H2_STREAM_ST_OPEN);
-    stream->request   = h2_request_create(id, pool, 
+    
-        h2_config_geti(session->config, H2_CONF_SER_HEADERS));
+    if (creq) {
+        /* take it into out pool and assure correct id's */
+        req = h2_request_clone(pool, creq);
+        req->id = id;
+        req->initiated_on = initiated_on;
+    }
+    else {
+        req = h2_request_create(id, pool, 
+                h2_config_geti(session->config, H2_CONF_SER_HEADERS));
+    }
+    stream->request = req; 
     
     ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, APLOGNO(03082)
                   "h2_stream(%ld-%d): opened", session->id, stream->id);
@@ -231,14 +244,6 @@ apr_status_t h2_stream_set_request(h2_stream *stream, request_rec *r)
     return status;
 }
 
-void h2_stream_set_h2_request(h2_stream *stream, int initiated_on,
-                              const h2_request *req)
-{
-    h2_request_copy(stream->pool, stream->request, req);
-    stream->request->initiated_on = initiated_on;
-    stream->request->eoh = 0;
-}
-
 apr_status_t h2_stream_add_header(h2_stream *stream,
                                   const char *name, size_t nlen,
                                   const char *value, size_t vlen)
@@ -298,7 +303,7 @@ apr_status_t h2_stream_schedule(h2_stream *stream, int eos, int push_enabled,
     }
     else {
         h2_stream_rst(stream, H2_ERR_INTERNAL_ERROR);
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, stream->session->c,
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, stream->session->c,
                       "h2_stream(%ld-%d): RST=2 (internal err) %s %s://%s%s",
                       stream->session->id, stream->id,
                       stream->request->method, stream->request->scheme,

modules/http2/h2_stream.h

@@ -71,7 +71,8 @@ struct h2_stream {
  * @param session the session this stream belongs to
  * @return the newly opened stream
  */
-h2_stream *h2_stream_open(int id, apr_pool_t *pool, struct h2_session *session);
+h2_stream *h2_stream_open(int id, apr_pool_t *pool, struct h2_session *session,
+                          int initiated_on, const struct h2_request *req);
 
 /**
  * Destroy any resources held by this stream. Will destroy memory pool
@@ -106,15 +107,6 @@ apr_pool_t *h2_stream_detach_pool(h2_stream *stream);
  */
 apr_status_t h2_stream_set_request(h2_stream *stream, request_rec *r);
 
-/**
- * Initialize stream->request with the given h2_request.
- * 
- * @param stream the stream to init the request for
- * @param req the request for initializing, will be copied
- */
-void h2_stream_set_h2_request(h2_stream *stream, int initiated_on,
-                              const struct h2_request *req);
-
 /*
  * Add a HTTP/2 header (including pseudo headers) or trailer 
  * to the given stream, depending on stream state.