subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kot...@apache.org
Subject svn commit: r1758207 - in /subversion/trunk/subversion/mod_dav_svn: dav_svn.h merge.c reports/dated-rev.c reports/deleted-rev.c reports/get-location-segments.c reports/get-locations.c reports/get-locks.c repos.c util.c
Date Mon, 29 Aug 2016 11:25:25 GMT
Author: kotkov
Date: Mon Aug 29 11:25:24 2016
New Revision: 1758207

URL: http://svn.apache.org/viewvc?rev=1758207&view=rev
Log:
In mod_dav_svn, switch every remaining usage of ap_fputs(), ap_fprintf()
and ap_fputstrs() to dav_svn__brigade functions.

Doing so ensures proper error handling in case of dead sockets (this
is issue 1709, see https://issues.apache.org/jira/browse/SVN-1709).

And with this change, every output that happens in mod_dav_svn now either
uses the dav_svn__brigade functions or manually prepares a bucket and
shoves it to the output filter stack by calling ap_pass_brigade().
This is an important prerequisite to fix the unbounded memory usage in
a case when mod_dav_svn is paired up with mod_headers or mod_deflate.

 * subversion/mod_dav_svn/dav_svn.h
   (dav_svn__brigade_putstrs): New wrapper for apr_brigade_vputstrs().

 * subversion/mod_dav_svn/util.c
   (dav_svn__brigade_putstrs): Implement new wrapper.

 * subversion/mod_dav_svn/repos.c
   (emit_collection_head, emit_collection_entry, emit_collection_tail):
    Use dav_svn__brigade functions.  Return possible errors, and ...
   (deliver): ...handle them here.

 * subversion/mod_dav_svn/merge.c
   (send_response, dav_svn__merge_response): Use dav_svn__brigade functions.

 * subversion/mod_dav_svn/reports/dated-rev.c
   (dav_svn__dated_rev_report): Use dav_svn__brigade functions.

 * subversion/mod_dav_svn/reports/deleted-rev.c
   (dav_svn__get_deleted_rev_report): Use dav_svn__brigade functions.

 * subversion/mod_dav_svn/reports/get-location-segments.c
   (location_segment_receiver): Use dav_svn__brigade functions.

 * subversion/mod_dav_svn/reports/get-locations.c
   (send_get_locations_report): Use dav_svn__brigade functions.  Return
    an svn_error_t instead of an apr_status_t, and handle it ...
   (dav_svn__get_locations_report): ...here.

 * subversion/mod_dav_svn/reports/get-locks.c
   (SVN_APR_ERR): Remove this macro.
   (send_get_lock_response): Use dav_svn__brigade functions.  Return
    an svn_error_t of an apr_status_t, and handle it ...
   (dav_svn__get_locks_report): ...here.

Modified:
    subversion/trunk/subversion/mod_dav_svn/dav_svn.h
    subversion/trunk/subversion/mod_dav_svn/merge.c
    subversion/trunk/subversion/mod_dav_svn/reports/dated-rev.c
    subversion/trunk/subversion/mod_dav_svn/reports/deleted-rev.c
    subversion/trunk/subversion/mod_dav_svn/reports/get-location-segments.c
    subversion/trunk/subversion/mod_dav_svn/reports/get-locations.c
    subversion/trunk/subversion/mod_dav_svn/reports/get-locks.c
    subversion/trunk/subversion/mod_dav_svn/repos.c
    subversion/trunk/subversion/mod_dav_svn/util.c

Modified: subversion/trunk/subversion/mod_dav_svn/dav_svn.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/dav_svn.h?rev=1758207&r1=1758206&r2=1758207&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/dav_svn.h (original)
+++ subversion/trunk/subversion/mod_dav_svn/dav_svn.h Mon Aug 29 11:25:24 2016
@@ -957,6 +957,11 @@ svn_error_t *dav_svn__brigade_printf(apr
                                      ...)
   __attribute__((format(printf, 3, 4)));
 
+/* Write an unspecified number of strings to OUTPUT using BB.  */
+svn_error_t *dav_svn__brigade_putstrs(apr_bucket_brigade *bb,
+                                      ap_filter_t *output,
+                                      ...);
+
 
 
 

Modified: subversion/trunk/subversion/mod_dav_svn/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/merge.c?rev=1758207&r1=1758206&r2=1758207&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/merge.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/merge.c Mon Aug 29 11:25:24 2016
@@ -78,7 +78,6 @@ send_response(const dav_svn_repos *repos
 {
   const char *href;
   const char *vsn_url;
-  apr_status_t status;
   svn_revnum_t rev_to_use;
 
   href = dav_svn__build_uri(repos, DAV_SVN__BUILD_URI_PUBLIC,
@@ -86,7 +85,7 @@ send_response(const dav_svn_repos *repos
   rev_to_use = dav_svn__get_safe_cr(root, path, pool);
   vsn_url = dav_svn__build_uri(repos, DAV_SVN__BUILD_URI_VERSION,
                                rev_to_use, path, FALSE /* add_href */, pool);
-  status = ap_fputstrs(output, bb,
+  SVN_ERR(dav_svn__brigade_putstrs(bb, output,
                        "<D:response>" DEBUG_CR
                        "<D:href>",
                        apr_xml_quote_string(pool, href, 1),
@@ -103,9 +102,7 @@ send_response(const dav_svn_repos *repos
                        "<D:status>HTTP/1.1 200 OK</D:status>" DEBUG_CR
                        "</D:propstat>" DEBUG_CR
                        "</D:response>" DEBUG_CR,
-                       NULL);
-  if (status != APR_SUCCESS)
-    return svn_error_wrap_apr(status, "Can't write response to output");
+                       NULL));
 
   return SVN_NO_ERROR;
 }
@@ -289,7 +286,7 @@ dav_svn__merge_response(ap_filter_t *out
   creationdate = svn_hash_gets(revprops, SVN_PROP_REVISION_DATE);
   creator_displayname = svn_hash_gets(revprops, SVN_PROP_REVISION_AUTHOR);
 
-  status = ap_fputstrs(output, bb,
+  serr = dav_svn__brigade_putstrs(bb, output,
                      DAV_XML_HEADER DEBUG_CR
                      "<D:merge-response xmlns:D=\"DAV:\"",
                      post_commit_header_info,
@@ -309,47 +306,46 @@ dav_svn__merge_response(ap_filter_t *out
                      post_commit_err_elem, DEBUG_CR
                      "<D:version-name>", rev, "</D:version-name>" DEBUG_CR,
                      NULL);
-  if (status != APR_SUCCESS)
-    return dav_svn__new_error(repos->pool, HTTP_INTERNAL_SERVER_ERROR,
-                              0, status,
-                              "Could not write output");
+  if (serr != NULL)
+    return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+                                "Could not write output",
+                                repos->pool);
 
   if (creationdate)
     {
-      status = ap_fputstrs(output, bb,
+      serr = dav_svn__brigade_putstrs(bb, output,
                          "<D:creationdate>",
                          apr_xml_quote_string(pool, creationdate->data, 1),
                          "</D:creationdate>" DEBUG_CR,
                          NULL);
-      if (status != APR_SUCCESS)
-        return dav_svn__new_error(repos->pool, HTTP_INTERNAL_SERVER_ERROR,
-                                  0, status,
-                                  "Could not write output");
+      if (serr != NULL)
+        return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+                                    "Could not write output",
+                                    repos->pool);
     }
   if (creator_displayname)
     {
-      status = ap_fputstrs(output, bb,
+      serr = dav_svn__brigade_putstrs(bb, output,
                          "<D:creator-displayname>",
                          apr_xml_quote_string(pool,
                                               creator_displayname->data, 1),
                          "</D:creator-displayname>" DEBUG_CR,
                          NULL);
-      if (status != APR_SUCCESS)
-        return dav_svn__new_error(repos->pool, HTTP_INTERNAL_SERVER_ERROR,
-                                  0, status,
-                                  "Could not write output");
+      if (serr != NULL)
+        return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+                                    "Could not write output",
+                                    repos->pool);
     }
-  status = ap_fputstrs(output, bb,
+  serr = dav_svn__brigade_putstrs(bb, output,
                      "</D:prop>" DEBUG_CR
                      "<D:status>HTTP/1.1 200 OK</D:status>" DEBUG_CR
                      "</D:propstat>" DEBUG_CR
                      "</D:response>" DEBUG_CR,
-
                      NULL);
-  if (status != APR_SUCCESS)
-    return dav_svn__new_error(repos->pool, HTTP_INTERNAL_SERVER_ERROR,
-                              0, status,
-                              "Could not write output");
+  if (serr != NULL)
+    return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+                                "Could not write output",
+                                repos->pool);
 
   /* ONLY have dir_delta drive the editor if the caller asked us to
      generate a full MERGE response.  svn clients can ask us to
@@ -378,13 +374,13 @@ dav_svn__merge_response(ap_filter_t *out
     }
 
   /* wrap up the merge response */
-  status = ap_fputs(output, bb,
-                  "</D:updated-set>" DEBUG_CR
-                  "</D:merge-response>" DEBUG_CR);
-  if (status != APR_SUCCESS)
-    return dav_svn__new_error(repos->pool, HTTP_INTERNAL_SERVER_ERROR,
-                              0, status,
-                              "Could not write output");
+  serr = dav_svn__brigade_puts(bb, output,
+                               "</D:updated-set>" DEBUG_CR
+                               "</D:merge-response>" DEBUG_CR);
+  if (serr != NULL)
+    return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+                                "Could not write output",
+                                repos->pool);
 
   /* send whatever is left in the brigade */
   status = ap_pass_brigade(output, bb);

Modified: subversion/trunk/subversion/mod_dav_svn/reports/dated-rev.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/reports/dated-rev.c?rev=1758207&r1=1758206&r2=1758207&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/reports/dated-rev.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/reports/dated-rev.c Mon Aug 29 11:25:24 2016
@@ -58,7 +58,6 @@ dav_svn__dated_rev_report(const dav_reso
   svn_revnum_t rev;
   apr_bucket_brigade *bb;
   svn_error_t *err;
-  apr_status_t apr_err;
   dav_error *derr = NULL;
 
   /* Find the DAV:creationdate element and get the requested time from it. */
@@ -96,14 +95,14 @@ dav_svn__dated_rev_report(const dav_reso
     }
 
   bb = apr_brigade_create(resource->pool, output->c->bucket_alloc);
-  apr_err = ap_fprintf(output, bb,
+  err = dav_svn__brigade_printf(bb, output,
                        DAV_XML_HEADER DEBUG_CR
                        "<S:dated-rev-report xmlns:S=\"" SVN_XML_NAMESPACE "\" "
                        "xmlns:D=\"DAV:\">" DEBUG_CR
                        "<D:" SVN_DAV__VERSION_NAME ">%ld</D:"
                        SVN_DAV__VERSION_NAME ">""</S:dated-rev-report>", rev);
-  if (apr_err)
-    derr = dav_svn__convert_err(svn_error_create(apr_err, 0, NULL),
+  if (err)
+    derr = dav_svn__convert_err(err,
                                 HTTP_INTERNAL_SERVER_ERROR,
                                 "Error writing REPORT response.",
                                 resource->pool);

Modified: subversion/trunk/subversion/mod_dav_svn/reports/deleted-rev.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/reports/deleted-rev.c?rev=1758207&r1=1758206&r2=1758207&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/reports/deleted-rev.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/reports/deleted-rev.c Mon Aug 29 11:25:24 2016
@@ -52,7 +52,6 @@ dav_svn__get_deleted_rev_report(const da
   svn_revnum_t deleted_rev;
   apr_bucket_brigade *bb;
   svn_error_t *err;
-  apr_status_t apr_err;
   dav_error *derr = NULL;
 
   /* Sanity check. */
@@ -119,15 +118,15 @@ dav_svn__get_deleted_rev_report(const da
     }
 
   bb = apr_brigade_create(resource->pool, output->c->bucket_alloc);
-  apr_err = ap_fprintf(output, bb,
+  err = dav_svn__brigade_printf(bb, output,
                        DAV_XML_HEADER DEBUG_CR
                        "<S:get-deleted-rev-report xmlns:S=\""
                        SVN_XML_NAMESPACE "\" xmlns:D=\"DAV:\">" DEBUG_CR
                        "<D:" SVN_DAV__VERSION_NAME ">%ld</D:"
                        SVN_DAV__VERSION_NAME ">""</S:get-deleted-rev-report>",
                        deleted_rev);
-  if (apr_err)
-    derr = dav_svn__convert_err(svn_error_create(apr_err, 0, NULL),
+  if (err)
+    derr = dav_svn__convert_err(err,
                                 HTTP_INTERNAL_SERVER_ERROR,
                                 "Error writing REPORT response.",
                                 resource->pool);

Modified: subversion/trunk/subversion/mod_dav_svn/reports/get-location-segments.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/reports/get-location-segments.c?rev=1758207&r1=1758206&r2=1758207&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/reports/get-location-segments.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/reports/get-location-segments.c Mon Aug 29 11:25:24
2016
@@ -79,28 +79,26 @@ location_segment_receiver(svn_location_s
                           apr_pool_t *pool)
 {
   struct location_segment_baton *b = baton;
-  apr_status_t apr_err;
 
   SVN_ERR(maybe_send_opener(b));
 
   if (segment->path)
     {
       const char *path_quoted = apr_xml_quote_string(pool, segment->path, 1);
-      apr_err = ap_fprintf(b->output, b->bb,
+
+      SVN_ERR(dav_svn__brigade_printf(b->bb, b->output,
                            "<S:location-segment path=\"%s\" "
                            "range-start=\"%ld\" range-end=\"%ld\"/>" DEBUG_CR,
                            path_quoted,
-                           segment->range_start, segment->range_end);
+                           segment->range_start, segment->range_end));
     }
   else
     {
-      apr_err = ap_fprintf(b->output, b->bb,
+      SVN_ERR(dav_svn__brigade_printf(b->bb, b->output,
                            "<S:location-segment "
                            "range-start=\"%ld\" range-end=\"%ld\"/>" DEBUG_CR,
-                           segment->range_start, segment->range_end);
+                           segment->range_start, segment->range_end));
     }
-  if (apr_err)
-    return svn_error_create(apr_err, 0, NULL);
   return SVN_NO_ERROR;
 }
 

Modified: subversion/trunk/subversion/mod_dav_svn/reports/get-locations.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/reports/get-locations.c?rev=1758207&r1=1758206&r2=1758207&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/reports/get-locations.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/reports/get-locations.c Mon Aug 29 11:25:24 2016
@@ -44,23 +44,20 @@
 #include "../dav_svn.h"
 
 
-static apr_status_t
+static svn_error_t *
 send_get_locations_report(ap_filter_t *output,
                           apr_bucket_brigade *bb,
                           const dav_resource *resource,
                           apr_hash_t *fs_locations)
 {
   apr_hash_index_t *hi;
-  apr_pool_t *pool;
-  apr_status_t apr_err;
+  apr_pool_t *pool = resource->pool;
 
-  pool = resource->pool;
-
-  apr_err = ap_fprintf(output, bb, DAV_XML_HEADER DEBUG_CR
+  SVN_ERR(dav_svn__brigade_printf(
+                       bb, output,
+                       DAV_XML_HEADER DEBUG_CR
                        "<S:get-locations-report xmlns:S=\"" SVN_XML_NAMESPACE
-                       "\" xmlns:D=\"DAV:\">" DEBUG_CR);
-  if (apr_err)
-    return apr_err;
+                       "\" xmlns:D=\"DAV:\">" DEBUG_CR));
 
   for (hi = apr_hash_first(pool, fs_locations); hi; hi = apr_hash_next(hi))
     {
@@ -70,13 +67,15 @@ send_get_locations_report(ap_filter_t *o
 
       apr_hash_this(hi, &key, NULL, &value);
       path_quoted = apr_xml_quote_string(pool, value, 1);
-      apr_err = ap_fprintf(output, bb, "<S:location "
+      SVN_ERR(dav_svn__brigade_printf(
+                           bb, output, "<S:location "
                            "rev=\"%ld\" path=\"%s\"/>" DEBUG_CR,
-                           *(const svn_revnum_t *)key, path_quoted);
-      if (apr_err)
-        return apr_err;
+                           *(const svn_revnum_t *)key, path_quoted));
     }
-  return ap_fprintf(output, bb, "</S:get-locations-report>" DEBUG_CR);
+
+  SVN_ERR(dav_svn__brigade_printf(bb, output,
+                                  "</S:get-locations-report>" DEBUG_CR));
+  return SVN_NO_ERROR;
 }
 
 
@@ -87,7 +86,6 @@ dav_svn__get_locations_report(const dav_
 {
   svn_error_t *serr;
   dav_error *derr = NULL;
-  apr_status_t apr_err;
   apr_bucket_brigade *bb;
   dav_svn__authz_read_baton arb;
 
@@ -173,10 +171,10 @@ dav_svn__get_locations_report(const dav_
 
   bb = apr_brigade_create(resource->pool, output->c->bucket_alloc);
 
-  apr_err = send_get_locations_report(output, bb, resource, fs_locations);
+  serr = send_get_locations_report(output, bb, resource, fs_locations);
 
-  if (apr_err)
-    derr = dav_svn__convert_err(svn_error_create(apr_err, 0, NULL),
+  if (serr)
+    derr = dav_svn__convert_err(serr,
                                 HTTP_INTERNAL_SERVER_ERROR,
                                 "Error writing REPORT response.",
                                 resource->pool);

Modified: subversion/trunk/subversion/mod_dav_svn/reports/get-locks.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/reports/get-locks.c?rev=1758207&r1=1758206&r2=1758207&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/reports/get-locks.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/reports/get-locks.c Mon Aug 29 11:25:24 2016
@@ -44,23 +44,9 @@
    report in libsvn_ra_neon/get_locks.c.  */
 
 
-#define SVN_APR_ERR(expr)                       \
-  do {                                          \
-    apr_status_t apr_status__temp = (expr);     \
-    if (apr_status__temp)                       \
-      return apr_status__temp;                  \
-  } while (0)
-
-
 /* Transmit LOCKS (a hash of Subversion filesystem locks keyed by
-   path) across OUTPUT using BB.  Use POOL for necessary allocations.
-
-   NOTE:  As written, this function currently returns one of only two
-   status values -- "success", and "we had trouble writing out to the
-   output stream".  If you need to return something more interesting,
-   you'll probably want to generate dav_error's here instead of
-   passing back only apr_status_t's.  */
-static apr_status_t
+   path) across OUTPUT using BB.  Use POOL for necessary allocations. */
+static svn_error_t *
 send_get_lock_response(apr_hash_t *locks,
                        ap_filter_t *output,
                        apr_bucket_brigade *bb,
@@ -70,7 +56,7 @@ send_get_lock_response(apr_hash_t *locks
   apr_hash_index_t *hi;
 
   /* start sending report */
-  SVN_APR_ERR(ap_fprintf(output, bb,
+  SVN_ERR(dav_svn__brigade_printf(bb, output,
                          DAV_XML_HEADER DEBUG_CR
                          "<S:get-locks-report xmlns:S=\"" SVN_XML_NAMESPACE
                          "\" xmlns:D=\"DAV:\">" DEBUG_CR));
@@ -86,7 +72,7 @@ send_get_lock_response(apr_hash_t *locks
 
       /* Begin the <S:lock> tag, transmitting the path, token, and
          creation date. */
-      SVN_APR_ERR(ap_fprintf(output, bb,
+      SVN_ERR(dav_svn__brigade_printf(bb, output,
                              "<S:lock>" DEBUG_CR
                              "<S:path>%s</S:path>" DEBUG_CR
                              "<S:token>%s</S:token>" DEBUG_CR
@@ -98,7 +84,7 @@ send_get_lock_response(apr_hash_t *locks
 
       /* Got expiration date?  Tell the client. */
       if (lock->expiration_date)
-        SVN_APR_ERR(ap_fprintf(output, bb,
+        SVN_ERR(dav_svn__brigade_printf(bb, output,
                                "<S:expirationdate>%s</S:expirationdate>"
                                DEBUG_CR,
                                svn_time_to_cstring(lock->expiration_date,
@@ -126,7 +112,7 @@ send_get_lock_response(apr_hash_t *locks
               owner = encoded_owner->data;
               owner_base64 = TRUE;
             }
-          SVN_APR_ERR(ap_fprintf(output, bb,
+          SVN_ERR(dav_svn__brigade_printf(bb, output,
                                  "<S:owner %s>%s</S:owner>" DEBUG_CR,
                                  owner_base64 ? "encoding=\"base64\"" : "",
                                  owner));
@@ -154,25 +140,24 @@ send_get_lock_response(apr_hash_t *locks
               comment = encoded_comment->data;
               comment_base64 = TRUE;
             }
-          SVN_APR_ERR(ap_fprintf(output, bb,
+          SVN_ERR(dav_svn__brigade_printf(bb, output,
                                  "<S:comment %s>%s</S:comment>" DEBUG_CR,
                                  comment_base64 ? "encoding=\"base64\"" : "",
                                  comment));
         }
 
       /* Okay, finish up this lock by closing the <S:lock> tag. */
-      SVN_APR_ERR(ap_fprintf(output, bb, "</S:lock>" DEBUG_CR));
+      SVN_ERR(dav_svn__brigade_printf(bb, output, "</S:lock>" DEBUG_CR));
     }
   svn_pool_destroy(iterpool);
 
   /* Finish the report */
-  SVN_APR_ERR(ap_fprintf(output, bb, "</S:get-locks-report>" DEBUG_CR));
+  SVN_ERR(dav_svn__brigade_printf(bb, output,
+                                  "</S:get-locks-report>" DEBUG_CR));
 
   return APR_SUCCESS;
 }
 
-#undef SVN_APR_ERR
-
 dav_error *
 dav_svn__get_locks_report(const dav_resource *resource,
                           const apr_xml_doc *doc,
@@ -181,7 +166,6 @@ dav_svn__get_locks_report(const dav_reso
   apr_bucket_brigade *bb;
   svn_error_t *err;
   dav_error *derr = NULL;
-  apr_status_t apr_err;
   apr_hash_t *locks;
   dav_svn__authz_read_baton arb;
   svn_depth_t depth = svn_depth_unknown;
@@ -229,8 +213,9 @@ dav_svn__get_locks_report(const dav_reso
 
   bb = apr_brigade_create(resource->pool, output->c->bucket_alloc);
 
-  if ((apr_err = send_get_lock_response(locks, output, bb, resource->pool)))
-    derr = dav_svn__convert_err(svn_error_create(apr_err, 0, NULL),
+  err = send_get_lock_response(locks, output, bb, resource->pool);
+  if (err)
+    derr = dav_svn__convert_err(err,
                                 HTTP_INTERNAL_SERVER_ERROR,
                                 "Error writing REPORT response.",
                                 resource->pool);

Modified: subversion/trunk/subversion/mod_dav_svn/repos.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/repos.c?rev=1758207&r1=1758206&r2=1758207&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/repos.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/repos.c Mon Aug 29 11:25:24 2016
@@ -3278,7 +3278,7 @@ close_filter(void *baton)
 }
 
 
-static void
+static svn_error_t *
 emit_collection_head(const dav_resource *resource,
                      apr_bucket_brigade *bb,
                      ap_filter_t *output,
@@ -3333,8 +3333,10 @@ emit_collection_head(const dav_resource
                                  title);
         }
 
-      ap_fprintf(output, bb, "<html><head><title>%s</title></head>\n"
-                 "<body>\n <h2>%s</h2>\n <ul>\n", title, title);
+      SVN_ERR(dav_svn__brigade_printf(bb, output,
+                                      "<html><head><title>%s</title></head>\n"
+                                      "<body>\n <h2>%s</h2>\n <ul>\n",
+                                      title, title));
     }
   else
     {
@@ -3342,28 +3344,33 @@ emit_collection_head(const dav_resource
       const char *href = resource->info->repos_path;
       const char *base = resource->info->repos->repo_basename;
 
-      ap_fputs(output, bb, "<?xml version=\"1.0\"?>\n");
-      ap_fprintf(output, bb,
-                 "<?xml-stylesheet type=\"text/xsl\" href=\"%s\"?>\n",
-                 resource->info->repos->xslt_uri);
-      ap_fputs(output, bb, xml_index_dtd);
-      ap_fputs(output, bb,
-               "<svn version=\"" SVN_VERSION "\"\n"
-               "     href=\"http://subversion.apache.org/\">\n");
-      ap_fputs(output, bb, "  <index");
+      SVN_ERR(dav_svn__brigade_puts(bb, output, "<?xml version=\"1.0\"?>\n"));
+      SVN_ERR(dav_svn__brigade_printf(bb, output,
+                                      "<?xml-stylesheet type=\"text/xsl\" "
+                                      "href=\"%s\"?>\n",
+                                      resource->info->repos->xslt_uri));
+      SVN_ERR(dav_svn__brigade_puts(bb, output, xml_index_dtd));
+      SVN_ERR(dav_svn__brigade_puts(bb, output,
+                         "<svn version=\"" SVN_VERSION "\"\n"
+                         "     href=\"http://subversion.apache.org/\">\n"));
+      SVN_ERR(dav_svn__brigade_puts(bb, output, "  <index"));
+
       if (name)
-        ap_fprintf(output, bb, " name=\"%s\"",
-                   apr_xml_quote_string(pool, name, 1));
+        SVN_ERR(dav_svn__brigade_printf(bb, output,
+                                        " name=\"%s\"",
+                                        apr_xml_quote_string(resource->pool,
+                                                             name, 1)));
       if (SVN_IS_VALID_REVNUM(resource->info->root.rev))
-        ap_fprintf(output, bb, " rev=\"%ld\"",
-                   resource->info->root.rev);
+        SVN_ERR(dav_svn__brigade_printf(bb, output, " rev=\"%ld\"",
+                                        resource->info->root.rev));
       if (href)
-        ap_fprintf(output, bb, " path=\"%s\"",
-                   apr_xml_quote_string(pool, href, 1));
+        SVN_ERR(dav_svn__brigade_printf(bb, output, " path=\"%s\"",
+                                        apr_xml_quote_string(resource->pool,
+                                                             href, 1)));
       if (base)
-        ap_fprintf(output, bb, " base=\"%s\"", base);
+        SVN_ERR(dav_svn__brigade_printf(bb, output, " base=\"%s\"", base));
 
-      ap_fputs(output, bb, ">\n");
+      SVN_ERR(dav_svn__brigade_puts(bb, output, ">\n"));
     }
 
   if ((resource->info->restype != DAV_SVN_RESTYPE_PARENTPATH_COLLECTION)
@@ -3383,17 +3390,23 @@ emit_collection_head(const dav_resource
 
       if (gen_html)
         {
-          ap_fprintf(output, bb, "  <li><a href=\"%s\">..</a></li>\n",
href);
+          SVN_ERR(dav_svn__brigade_printf(bb, output,
+                                          "  <li><a href=\"%s\">..</a></li>\n",
+                                          href));
         }
       else
         {
-          ap_fprintf(output, bb, "    <updir href=\"%s\"/>\n", href);
+          SVN_ERR(dav_svn__brigade_printf(bb, output,
+                                          "    <updir href=\"%s\"/>\n",
+                                          href));
         }
     }
+
+  return SVN_NO_ERROR;
 }
 
 
-static void
+static svn_error_t *
 emit_collection_entry(const dav_resource *resource,
                       apr_bucket_brigade *bb,
                       ap_filter_t *output,
@@ -3436,15 +3449,15 @@ emit_collection_entry(const dav_resource
          peg-rev, too. */
       if (resource->info->pegged)
         {
-          ap_fprintf(output, bb,
+          SVN_ERR(dav_svn__brigade_printf(bb, output,
                      "  <li><a href=\"%s?p=%ld\">%s</a></li>\n",
-                     href, resource->info->root.rev, name);
+                     href, resource->info->root.rev, name));
         }
       else
         {
-          ap_fprintf(output, bb,
+          SVN_ERR(dav_svn__brigade_printf(bb, output,
                      "  <li><a href=\"%s\">%s</a></li>\n",
-                     href, name);
+                     href, name));
         }
     }
   else
@@ -3458,21 +3471,23 @@ emit_collection_entry(const dav_resource
          peg-rev, too. */
       if (resource->info->pegged)
         {
-          ap_fprintf(output, bb,
+          SVN_ERR(dav_svn__brigade_printf(bb, output,
                      "    <%s name=\"%s\" href=\"%s?p=%ld\" />\n",
-                     tag, name, href, resource->info->root.rev);
+                     tag, name, href, resource->info->root.rev));
         }
       else
         {
-          ap_fprintf(output, bb,
+          SVN_ERR(dav_svn__brigade_printf(bb, output,
                      "    <%s name=\"%s\" href=\"%s\" />\n",
-                     tag, name, href);
+                     tag, name, href));
         }
     }
+
+  return SVN_NO_ERROR;
 }
 
 
-static void
+static svn_error_t *
 emit_collection_tail(const dav_resource *resource,
                      apr_bucket_brigade *bb,
                      ap_filter_t *output,
@@ -3492,18 +3507,20 @@ emit_collection_tail(const dav_resource
              to change. (Perhaps we should try to get the Apache folks to
              make this promise, though.  Seems harmless/useful enough...)
           */
-          ap_fputs(output, bb,
+          SVN_ERR(dav_svn__brigade_puts(bb, output,
                    " </ul>\n <hr noshade><em>Powered by "
                    "<a href=\"http://subversion.apache.org/\">"
                    "Apache Subversion"
                    "</a> version " SVN_VERSION "."
-                   "</em>\n</body></html>");
+                   "</em>\n</body></html>"));
         }
       else
-        ap_fputs(output, bb, " </ul>\n</body></html>");
+        SVN_ERR(dav_svn__brigade_puts(bb, output, " </ul>\n</body></html>"));
     }
   else
-    ap_fputs(output, bb, "  </index>\n</svn>\n");
+    SVN_ERR(dav_svn__brigade_puts(bb, output, "  </index>\n</svn>\n"));
+
+  return SVN_NO_ERROR;
 }
 
 
@@ -3617,7 +3634,12 @@ deliver(const dav_resource *resource, ap
 
       bb = apr_brigade_create(resource->pool, output->c->bucket_alloc);
 
-      emit_collection_head(resource, bb, output, gen_html, resource->pool);
+      serr = emit_collection_head(resource, bb, output, gen_html,
+                                  resource->pool);
+      if (serr != NULL)
+        return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+                                    "could not output collection",
+                                    resource->pool);
 
       /* get a sorted list of the entries */
       sorted = svn_sort__hash(entries, svn_sort_compare_items_as_paths,
@@ -3656,13 +3678,22 @@ deliver(const dav_resource *resource, ap
                   continue;
             }
 
-          emit_collection_entry(resource, bb, output, entry, gen_html,
-                                entry_pool);
+          serr = emit_collection_entry(resource, bb, output, entry, gen_html,
+                                       entry_pool);
+          if (serr != NULL)
+            return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+                                        "could not output collection entry",
+                                        resource->pool);
         }
 
       svn_pool_destroy(entry_pool);
 
-      emit_collection_tail(resource, bb, output, gen_html, resource->pool);
+      serr = emit_collection_tail(resource, bb, output, gen_html,
+                                  resource->pool);
+      if (serr != NULL)
+        return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+                                    "could not output collection",
+                                    resource->pool);
 
       bkt = apr_bucket_eos_create(output->c->bucket_alloc);
       APR_BRIGADE_INSERT_TAIL(bb, bkt);

Modified: subversion/trunk/subversion/mod_dav_svn/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/util.c?rev=1758207&r1=1758206&r2=1758207&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/util.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/util.c Mon Aug 29 11:25:24 2016
@@ -527,6 +527,27 @@ dav_svn__brigade_printf(apr_bucket_briga
 }
 
 
+svn_error_t *
+dav_svn__brigade_putstrs(apr_bucket_brigade *bb,
+                         ap_filter_t *output,
+                         ...)
+{
+  apr_status_t apr_err;
+  va_list ap;
+
+  va_start(ap, output);
+  apr_err = apr_brigade_vputstrs(bb, ap_filter_flush, output, ap);
+  va_end(ap);
+  if (apr_err)
+    return svn_error_create(apr_err, NULL, NULL);
+  /* Check for an aborted connection, since the brigade functions don't
+     appear to return useful errors when the connection is dropped. */
+  if (output->c->aborted)
+    return svn_error_create(SVN_ERR_APMOD_CONNECTION_ABORTED, NULL, NULL);
+  return SVN_NO_ERROR;
+}
+
+
 
 
 dav_error *



Mime
View raw message