git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Cc: git@vger.kernel.org
Subject: [PATCH 1/2] http-backend: fix die recursion with custom handler
Date: Fri, 15 May 2015 02:29:27 -0400	[thread overview]
Message-ID: <20150515062926.GA30890@peff.net> (raw)
In-Reply-To: <20150515062901.GA30768@peff.net>

When we die() in http-backend, we call a custom handler that
writes an HTTP 500 response to stdout, then reports the
error to stderr. Our routines for writing out the HTTP
response may themselves die, leading to us entering die()
again.

When it was originally written, that was OK; our custom
handler keeps a variable to notice this and does not
recurse. However, since cd163d4 (usage.c: detect recursion
in die routines and bail out immediately, 2012-11-14), the
main die() implementation detects recursion before we even
get to our custom handler, and bails without printing
anything useful.

We can handle this case by doing two things:

  1. Installing a custom die_is_recursing handler that
     allows us to enter up to one level of recursion. Only
     the first call to our custom handler will try to write
     out the error response. So if we die again, that is OK.
     If we end up dying more than that, it is a sign that we
     are in an infinite recursion.

  2. Reporting the error to stderr before trying to write
     out the HTTP response. In the current code, if we do
     die() trying to write out the response, we'll exit
     immediately from this second die(), and never get a
     chance to output the original error (which is almost
     certainly the more interesting one; the second die is
     just going to be along the lines of "I tried to write
     to stdout but it was closed").

Signed-off-by: Jeff King <peff@peff.net>
---
 http-backend.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/http-backend.c b/http-backend.c
index b6c0484..3ad82a8 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -500,21 +500,24 @@ static void service_rpc(char *service_name)
 	strbuf_release(&buf);
 }
 
+static int dead;
 static NORETURN void die_webcgi(const char *err, va_list params)
 {
-	static int dead;
+	if (dead <= 1) {
+		vreportf("fatal: ", err, params);
 
-	if (!dead) {
-		dead = 1;
 		http_status(500, "Internal Server Error");
 		hdr_nocache();
 		end_headers();
-
-		vreportf("fatal: ", err, params);
 	}
 	exit(0); /* we successfully reported a failure ;-) */
 }
 
+static int die_webcgi_recursing(void)
+{
+	return dead++ > 1;
+}
+
 static char* getdir(void)
 {
 	struct strbuf buf = STRBUF_INIT;
@@ -569,6 +572,7 @@ int main(int argc, char **argv)
 
 	git_extract_argv0_path(argv[0]);
 	set_die_routine(die_webcgi);
+	set_die_is_recursing_routine(die_webcgi_recursing);
 
 	if (!method)
 		die("No REQUEST_METHOD from server");
-- 
2.4.1.396.g7ba6d7b

  reply	other threads:[~2015-05-15  6:29 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-13 21:04 Clone hangs when done over http with --reference Konstantin Ryabitsev
2015-05-14  0:47 ` Jeff King
2015-05-14  1:02   ` [PATCH] http-backend: fix die recursion with custom handler Jeff King
2015-05-15  6:20     ` Jeff King
2015-05-14 17:08   ` Clone hangs when done over http with --reference Konstantin Ryabitsev
2015-05-14 19:52     ` Jeff King
2015-05-15  6:29   ` [PATCH 0/2] fix http deadlock on giant ref negotiations Jeff King
2015-05-15  6:29     ` Jeff King [this message]
2015-05-15  6:33     ` [PATCH 2/2] http-backend: spool ref negotiation requests to buffer Jeff King
2015-05-15 18:22       ` Junio C Hamano
2015-05-15 18:28         ` Konstantin Ryabitsev
2015-05-20  7:35           ` [PATCH v2 0/3] fix http deadlock on giant ref negotiations Jeff King
2015-05-20  7:36             ` [PATCH v2 1/3] http-backend: fix die recursion with custom handler Jeff King
2015-05-20  7:36             ` [PATCH v2 2/3] t5551: factor out tag creation Jeff King
2015-05-20  7:37             ` [PATCH v2 3/3] http-backend: spool ref negotiation requests to buffer Jeff King
2015-05-26  2:07               ` Konstantin Ryabitsev
2015-05-26  2:24                 ` Jeff King
2015-05-26  3:43                   ` Junio C Hamano
2015-05-15 19:16         ` [PATCH 2/2] " Jeff King
2015-05-15  7:41     ` [PATCH 0/2] fix http deadlock on giant ref negotiations Dennis Kaarsemaker
2015-05-15  8:38       ` Jeff King
2015-05-15  8:44         ` Dennis Kaarsemaker
2015-05-15  8:53           ` Jeff King
2015-05-15  9:11             ` Dennis Kaarsemaker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150515062926.GA30890@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=konstantin@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).