Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 45217200B73 for ; Mon, 29 Aug 2016 13:00:25 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 43A32160AB8; Mon, 29 Aug 2016 11:00:25 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 15E12160AA7 for ; Mon, 29 Aug 2016 13:00:23 +0200 (CEST) Received: (qmail 43817 invoked by uid 500); 29 Aug 2016 11:00:23 -0000 Mailing-List: contact commits-help@subversion.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@subversion.apache.org Delivered-To: mailing list commits@subversion.apache.org Received: (qmail 43807 invoked by uid 99); 29 Aug 2016 11:00:23 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 29 Aug 2016 11:00:23 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id D488EC50D1 for ; Mon, 29 Aug 2016 11:00:22 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -0.426 X-Spam-Level: X-Spam-Status: No, score=-0.426 tagged_above=-999 required=6.31 tests=[KAM_LAZY_DOMAIN_SECURITY=1, RP_MATCHES_RCVD=-1.426] autolearn=disabled Received: from mx2-lw-us.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id jSzkBArX-W65 for ; Mon, 29 Aug 2016 11:00:20 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx2-lw-us.apache.org (ASF Mail Server at mx2-lw-us.apache.org) with ESMTP id BF1455FBB6 for ; Mon, 29 Aug 2016 11:00:19 +0000 (UTC) Received: from svn01-us-west.apache.org (svn.apache.org [10.41.0.6]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id F35ADE05DC for ; Mon, 29 Aug 2016 11:00:18 +0000 (UTC) Received: from svn01-us-west.apache.org (localhost [127.0.0.1]) by svn01-us-west.apache.org (ASF Mail Server at svn01-us-west.apache.org) with ESMTP id 0DDA83A028F for ; Mon, 29 Aug 2016 11:00:17 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1758204 - /subversion/trunk/subversion/mod_dav_svn/repos.c Date: Mon, 29 Aug 2016 11:00:17 -0000 To: commits@subversion.apache.org From: kotkov@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20160829110018.0DDA83A028F@svn01-us-west.apache.org> archived-at: Mon, 29 Aug 2016 11:00:25 -0000 Author: kotkov Date: Mon Aug 29 11:00:17 2016 New Revision: 1758204 URL: http://svn.apache.org/viewvc?rev=1758204&view=rev Log: In mod_dav_svn, factor out three functions responsible for sending the header, trailer and a single entry of the collection. This lays the groundwork required for switching to dav_svn__brigade functions whenever we write something to the output filter stack. And this switch would be required to fix the unbounded memory usage when mod_dav_svn is paired up with mod_headers or mod_deflate. * subversion/mod_dav_svn/repos.c (deliver): Factor out parts responsible for sending the header, trailer and the entry of the collection into ... (emit_collection_head, emit_collection_tail, emit_collection_entry): ...these new functions. Modified: subversion/trunk/subversion/mod_dav_svn/repos.c Modified: subversion/trunk/subversion/mod_dav_svn/repos.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/repos.c?rev=1758204&r1=1758203&r2=1758204&view=diff ============================================================================== --- subversion/trunk/subversion/mod_dav_svn/repos.c (original) +++ subversion/trunk/subversion/mod_dav_svn/repos.c Mon Aug 29 11:00:17 2016 @@ -3278,6 +3278,235 @@ close_filter(void *baton) } +static void +emit_collection_head(const dav_resource *resource, + apr_bucket_brigade *bb, + ap_filter_t *output, + svn_boolean_t gen_html, + apr_pool_t *pool) +{ + /* XML schema for the directory index if xslt_uri is set: + + + repos->xslt_uri]"?> */ + static const char xml_index_dtd[] = + "\n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + "]>\n"; + + if (gen_html) + { + const char *title; + if (resource->info->repos_path == NULL) + title = "unknown location"; + else + title = resource->info->repos_path; + + if (resource->info->restype != DAV_SVN_RESTYPE_PARENTPATH_COLLECTION) + { + if (SVN_IS_VALID_REVNUM(resource->info->root.rev)) + title = apr_psprintf(pool, + "Revision %ld: %s", + resource->info->root.rev, title); + if (resource->info->repos->repo_basename) + title = apr_psprintf(pool, "%s - %s", + resource->info->repos->repo_basename, + title); + if (resource->info->repos->repo_name) + title = apr_psprintf(pool, "%s: %s", + resource->info->repos->repo_name, + title); + } + + ap_fprintf(output, bb, "%s\n" + "\n

%s

\n
    \n", title, title); + } + else + { + const char *name = resource->info->repos->repo_name; + const char *href = resource->info->repos_path; + const char *base = resource->info->repos->repo_basename; + + ap_fputs(output, bb, "\n"); + ap_fprintf(output, bb, + "\n", + resource->info->repos->xslt_uri); + ap_fputs(output, bb, xml_index_dtd); + ap_fputs(output, bb, + "\n"); + ap_fputs(output, bb, " info->root.rev)) + ap_fprintf(output, bb, " rev=\"%ld\"", + resource->info->root.rev); + if (href) + ap_fprintf(output, bb, " path=\"%s\"", + apr_xml_quote_string(pool, href, 1)); + if (base) + ap_fprintf(output, bb, " base=\"%s\"", base); + + ap_fputs(output, bb, ">\n"); + } + + if ((resource->info->restype != DAV_SVN_RESTYPE_PARENTPATH_COLLECTION) + && resource->info->repos_path + && ((resource->info->repos_path[1] != '\0') + || dav_svn__get_list_parentpath_flag(resource->info->r))) + { + const char *href; + if (resource->info->pegged) + { + href = apr_psprintf(pool, "../?p=%ld", resource->info->root.rev); + } + else + { + href = "../"; + } + + if (gen_html) + { + ap_fprintf(output, bb, "
  • ..
    • \n", href); + } + else + { + ap_fprintf(output, bb, " \n", href); + } + } +} + + +static void +emit_collection_entry(const dav_resource *resource, + apr_bucket_brigade *bb, + ap_filter_t *output, + const svn_fs_dirent_t *entry, + svn_boolean_t gen_html, + apr_pool_t *pool) +{ + const char *name = entry->name; + const char *href = name; + svn_boolean_t is_dir = (entry->kind == svn_node_dir); + + /* append a trailing slash onto the name for directories. we NEED + this for the href portion so that the relative reference will + descend properly. for the visible portion, it is just nice. */ + /* ### The xml output doesn't like to see a trailing slash on + ### the visible portion, so avoid that. */ + if (is_dir) + href = apr_pstrcat(pool, href, "/", SVN_VA_NULL); + + if (gen_html) + name = href; + + /* We quote special characters in both XML and HTML. */ + name = apr_xml_quote_string(pool, name, !gen_html); + + /* According to httpd-2.0.54/include/httpd.h, ap_os_escape_path() + behaves differently on different platforms. It claims to + "convert an OS path to a URL in an OS dependant way". + Nevertheless, there appears to be only one implementation + of the function in httpd, and the code seems completely + platform independent, so we'll assume it's appropriate for + mod_dav_svn to use it to quote outbound paths. */ + href = ap_os_escape_path(pool, href, 0); + href = apr_xml_quote_string(pool, href, 1); + + if (gen_html) + { + /* If our directory was access using the public peg-rev + CGI query interface, we'll let its dirents carry that + peg-rev, too. */ + if (resource->info->pegged) + { + ap_fprintf(output, bb, + "
  • %s
    • \n", + href, resource->info->root.rev, name); + } + else + { + ap_fprintf(output, bb, + "
  • %s
    • \n", + href, name); + } + } + else + { + const char *const tag = (is_dir ? "dir" : "file"); + + /* This is where we could search for props */ + + /* If our directory was access using the public peg-rev + CGI query interface, we'll let its dirents carry that + peg-rev, too. */ + if (resource->info->pegged) + { + ap_fprintf(output, bb, + " <%s name=\"%s\" href=\"%s?p=%ld\" />\n", + tag, name, href, resource->info->root.rev); + } + else + { + ap_fprintf(output, bb, + " <%s name=\"%s\" href=\"%s\" />\n", + tag, name, href); + } + } +} + + +static void +emit_collection_tail(const dav_resource *resource, + apr_bucket_brigade *bb, + ap_filter_t *output, + svn_boolean_t gen_html, + apr_pool_t *pool) +{ + if (gen_html) + { + if (strcmp(ap_psignature("FOO", resource->info->r), "") != 0) + { + /* Apache's signature generation code didn't eat our prefix. + ServerSignature must be enabled. Print our version info. + + WARNING: This is a kludge!! ap_psignature() doesn't promise + to return the empty string when ServerSignature is off. We + know it does by code inspection, but this behavior is subject + to change. (Perhaps we should try to get the Apache folks to + make this promise, though. Seems harmless/useful enough...) + */ + ap_fputs(output, bb, + "
\n
Powered by " + "" + "Apache Subversion" + " version " SVN_VERSION "." + "\n"); + } + else + ap_fputs(output, bb, " \n"); + } + else + ap_fputs(output, bb, " \n\n"); +} + + static dav_error * deliver(const dav_resource *resource, ap_filter_t *output) { @@ -3306,30 +3535,6 @@ deliver(const dav_resource *resource, ap svn_revnum_t dir_rev = SVN_INVALID_REVNUM; int i; - /* XML schema for the directory index if xslt_uri is set: - - - repos->xslt_uri]"?> */ - static const char xml_index_dtd[] = - "\n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - "]>\n"; - /* pool, output->c->bucket_alloc); - if (gen_html) - { - const char *title; - if (resource->info->repos_path == NULL) - title = "unknown location"; - else - title = resource->info->repos_path; - - if (resource->info->restype != DAV_SVN_RESTYPE_PARENTPATH_COLLECTION) - { - if (SVN_IS_VALID_REVNUM(resource->info->root.rev)) - title = apr_psprintf(resource->pool, - "Revision %ld: %s", - resource->info->root.rev, title); - if (resource->info->repos->repo_basename) - title = apr_psprintf(resource->pool, "%s - %s", - resource->info->repos->repo_basename, - title); - if (resource->info->repos->repo_name) - title = apr_psprintf(resource->pool, "%s: %s", - resource->info->repos->repo_name, - title); - } - - ap_fprintf(output, bb, "%s\n" - "\n

%s

\n
    \n", title, title); - } - else - { - const char *name = resource->info->repos->repo_name; - const char *href = resource->info->repos_path; - const char *base = resource->info->repos->repo_basename; - - ap_fputs(output, bb, "\n"); - ap_fprintf(output, bb, - "\n", - resource->info->repos->xslt_uri); - ap_fputs(output, bb, xml_index_dtd); - ap_fputs(output, bb, - "\n"); - ap_fputs(output, bb, " pool, name, 1)); - if (SVN_IS_VALID_REVNUM(resource->info->root.rev)) - ap_fprintf(output, bb, " rev=\"%ld\"", - resource->info->root.rev); - if (href) - ap_fprintf(output, bb, " path=\"%s\"", - apr_xml_quote_string(resource->pool, - href, - 1)); - if (base) - ap_fprintf(output, bb, " base=\"%s\"", base); - - ap_fputs(output, bb, ">\n"); - } - - if ((resource->info->restype != DAV_SVN_RESTYPE_PARENTPATH_COLLECTION) - && resource->info->repos_path - && ((resource->info->repos_path[1] != '\0') - || dav_svn__get_list_parentpath_flag(resource->info->r))) - { - const char *href; - if (resource->info->pegged) - { - href = apr_psprintf(resource->pool, "../?p=%ld", - resource->info->root.rev); - } - else - { - href = "../"; - } - - if (gen_html) - { - ap_fprintf(output, bb, - "
  • ..
    • \n", href); - } - else - { - ap_fprintf(output, bb, " \n", href); - } - } + emit_collection_head(resource, bb, output, gen_html, resource->pool); /* get a sorted list of the entries */ sorted = svn_sort__hash(entries, svn_sort_compare_items_as_paths, @@ -3510,8 +3631,6 @@ deliver(const dav_resource *resource, ap const svn_sort__item_t); const svn_fs_dirent_t *entry = item->value; const char *name = item->key; - const char *href = name; - svn_boolean_t is_dir = (entry->kind == svn_node_dir); const char *repos_relpath = NULL; svn_pool_clear(entry_pool); @@ -3537,99 +3656,13 @@ deliver(const dav_resource *resource, ap continue; } - /* append a trailing slash onto the name for directories. we NEED - this for the href portion so that the relative reference will - descend properly. for the visible portion, it is just nice. */ - /* ### The xml output doesn't like to see a trailing slash on - ### the visible portion, so avoid that. */ - if (is_dir) - href = apr_pstrcat(entry_pool, href, "/", SVN_VA_NULL); - - if (gen_html) - name = href; - - /* We quote special characters in both XML and HTML. */ - name = apr_xml_quote_string(entry_pool, name, !gen_html); - - /* According to httpd-2.0.54/include/httpd.h, ap_os_escape_path() - behaves differently on different platforms. It claims to - "convert an OS path to a URL in an OS dependant way". - Nevertheless, there appears to be only one implementation - of the function in httpd, and the code seems completely - platform independent, so we'll assume it's appropriate for - mod_dav_svn to use it to quote outbound paths. */ - href = ap_os_escape_path(entry_pool, href, 0); - href = apr_xml_quote_string(entry_pool, href, 1); - - if (gen_html) - { - /* If our directory was access using the public peg-rev - CGI query interface, we'll let its dirents carry that - peg-rev, too. */ - if (resource->info->pegged) - { - ap_fprintf(output, bb, - "
  • %s
    • \n", - href, resource->info->root.rev, name); - } - else - { - ap_fprintf(output, bb, - "
  • %s
    • \n", - href, name); - } - } - else - { - const char *const tag = (is_dir ? "dir" : "file"); - - /* This is where we could search for props */ - - /* If our directory was access using the public peg-rev - CGI query interface, we'll let its dirents carry that - peg-rev, too. */ - if (resource->info->pegged) - { - ap_fprintf(output, bb, - " <%s name=\"%s\" href=\"%s?p=%ld\" />\n", - tag, name, href, resource->info->root.rev); - } - else - { - ap_fprintf(output, bb, - " <%s name=\"%s\" href=\"%s\" />\n", - tag, name, href); - } - } + emit_collection_entry(resource, bb, output, entry, gen_html, + entry_pool); } svn_pool_destroy(entry_pool); - if (gen_html) - { - if (strcmp(ap_psignature("FOO", resource->info->r), "") != 0) - { - /* Apache's signature generation code didn't eat our prefix. - ServerSignature must be enabled. Print our version info. - - WARNING: This is a kludge!! ap_psignature() doesn't promise - to return the empty string when ServerSignature is off. We - know it does by code inspection, but this behavior is subject - to change. (Perhaps we should try to get the Apache folks to - make this promise, though. Seems harmless/useful enough...) - */ - ap_fputs(output, bb, - "
\n
Powered by " - "" - "Apache Subversion" - " version " SVN_VERSION "." - "\n"); - } - else - ap_fputs(output, bb, " \n"); - } - else - ap_fputs(output, bb, " \n\n"); + emit_collection_tail(resource, bb, output, gen_html, resource->pool); bkt = apr_bucket_eos_create(output->c->bucket_alloc); APR_BRIGADE_INSERT_TAIL(bb, bkt);