git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Florian Manschwetus <manschwetus@cs-software-gmbh.de>
To: Chris Packham <judge.packham@gmail.com>
Cc: Konstantin Khomoutov <kostix+git@007spb.ru>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: [PATCH] Fix http-backend reading till EOF, ignoring CONTENT_LENGTH, violating rfc3875 -- WAS: Problem with git-http-backend.exe as iis cgi
Date: Tue, 29 Mar 2016 10:38:23 +0000	[thread overview]
Message-ID: <F0F5A56A22F20D4CB4A03BB8D6658797E261A3D6@SERVER2011.CS-SOFTWARE.local> (raw)

-----Ursprüngliche Nachricht-----
Von: Chris Packham [mailto:judge.packham@gmail.com] 
Gesendet: Dienstag, 29. März 2016 11:28
An: Florian Manschwetus
Cc: Konstantin Khomoutov; git@vger.kernel.org
Betreff: Re: Problem with git-http-backend.exe as iis cgi

Hi Florian

On Tue, Mar 29, 2016 at 7:01 PM, Florian Manschwetus <manschwetus@cs-software-gmbh.de> wrote:
> Hi,
> I put together a first patch for the issue.
>
> Mit freundlichen Grüßen / With kind regards Florian Manschwetus
>
> E-Mail: manschwetus@cs-software-gmbh.de
> Tel.: +49-(0)611-8908534
>
> CS Software Concepts and Solutions GmbH Geschäftsführer / Managing 
> director: Dr. Werner Alexi Amtsgericht Wiesbaden HRB 10004 (Commercial 
> registry) Schiersteiner Straße 31
> D-65187 Wiesbaden
> Germany
> Tel.: 0611/8908555
>
>
> -----Ursprüngliche Nachricht-----
> Von: Konstantin Khomoutov [mailto:kostix+git@007spb.ru]
> Gesendet: Donnerstag, 10. März 2016 13:55
> An: Florian Manschwetus
> Cc: git@vger.kernel.org
> Betreff: Re: Problem with git-http-backend.exe as iis cgi
>
> On Thu, 10 Mar 2016 07:28:50 +0000
> Florian Manschwetus <manschwetus@cs-software-gmbh.de> wrote:
>
>> I tried to setup git-http-backend with iis, as iis provides proper 
>> impersonation for cgi under windows, which leads to have the 
>> filesystem access performed with the logon user, therefore the 
>> webserver doesn't need generic access to the files. I stumbled across 
>> a problem, ending up with post requests hanging forever. After some 
>> investigation I managed to get it work by wrapping the http-backend 
>> into a bash script, giving a lot of control about the environmental 
>> things, I was unable to solve within IIS configuration. The 
>> workaround, I use currently, is to use "/bin/head -c 
>> ${CONTENT_LENGTH}
>> | ./git-http-backend.exe", which directly shows the issue. Git
>> http-backend should check if CONTENT_LENGTH is set to something 
>> reasonable (e.g. >0) and should in this case read only CONTENT_LENGTH 
>> bytes from stdin, instead of reading till EOF what I suspect it is 
>> doing currently.
>
> The rfc [1] states in its section 4.2:
>
> | A request-body is supplied with the request if the CONTENT_LENGTH is 
> | not NULL.  The server MUST make at least that many bytes available 
> | for the script to read.  The server MAY signal an end-of-file 
> | condition after CONTENT_LENGTH bytes have been read or it MAY supply 
> | extension data.  Therefore, the script MUST NOT attempt to read more 
> | than CONTENT_LENGTH bytes, even if more data is available.  However, 
> | it is not obliged to read any of the data.
>
> So yes, if Git currently reads until EOF, it's an error.
> The correct way would be:
>
> 1) Check to see if the CONTENT_LENGTH variable is available in the
>    environment.  If no, read nothing.
>
> 2) Otherwise read as many bytes it specifies, and no more.
>
> 1. https://www.ietf.org/rfc/rfc3875

Your patch description seems well thought out but if you want someone to notice it you should have a read of https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches

Moin,
I have cloned git and created a more clean patch...

Signed-off-by: Florian Manschwetus <manschwetus@cs-software-gmbh.de>
---
 http-backend.c | 48 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/http-backend.c b/http-backend.c
index 8870a26..94976df 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -277,16 +277,32 @@ static struct rpc_service *select_service(const char *name)
  */
 static ssize_t read_request(int fd, unsigned char **out)
 {
-	size_t len = 0, alloc = 8192;
-	unsigned char *buf = xmalloc(alloc);
+	unsigned char *buf = null;
+	size_t len = 0;
+	/* get request size */
+	size_t req_len = git_env_ulong("CONTENT_LENGTH",
+					   0);
+
+	/* check request size */
+	if (max_request_buffer < req_len) {
+		die("request was larger than our maximum size (%lu);"
+			    " try setting GIT_HTTP_MAX_REQUEST_BUFFER",
+			    max_request_buffer);
+	}
+
+	if (req_len <= 0) {
+		*out = null;
+		return 0;
+	}
+
+	/* allocate buffer */
+	buf = xmalloc(req_len)
 
-	if (max_request_buffer < alloc)
-		max_request_buffer = alloc;
 
 	while (1) {
 		ssize_t cnt;
 
-		cnt = read_in_full(fd, buf + len, alloc - len);
+		cnt = read_in_full(fd, buf + len, req_len - len);
 		if (cnt < 0) {
 			free(buf);
 			return -1;
@@ -294,21 +310,18 @@ static ssize_t read_request(int fd, unsigned char **out)
 
 		/* partial read from read_in_full means we hit EOF */
 		len += cnt;
-		if (len < alloc) {
+		if (len < req_len) {
+			/* TODO request incomplete?? */
+			/* maybe just remove this block and condition along with the loop, */
+			/* if read_in_full is prooven reliable */
 			*out = buf;
 			return len;
+		} else {
+			/* request complete */
+			*out = buf;
+			return len;
+			
 		}
-
-		/* otherwise, grow and try again (if we can) */
-		if (alloc == max_request_buffer)
-			die("request was larger than our maximum size (%lu);"
-			    " try setting GIT_HTTP_MAX_REQUEST_BUFFER",
-			    max_request_buffer);
-
-		alloc = alloc_nr(alloc);
-		if (alloc > max_request_buffer)
-			alloc = max_request_buffer;
-		REALLOC_ARRAY(buf, alloc);
 	}
 }
 
@@ -701,3 +714,4 @@ int main(int argc, char **argv)
 	cmd->imp(cmd_arg);
 	return 0;
 }
+
-- 
2.7.2.windows.1


Mit freundlichen Grüßen / With kind regards
Florian Manschwetus

CS Software Concepts and Solutions GmbH
Geschäftsführer / Managing director: Dr. Werner Alexi 
Amtsgericht Wiesbaden HRB 10004 (Commercial registry)
Schiersteiner Straße 31
D-65187 Wiesbaden
Germany



             reply	other threads:[~2016-03-29 10:38 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-29 10:38 Florian Manschwetus [this message]
2016-03-29 20:13 ` [PATCH] Fix http-backend reading till EOF, ignoring CONTENT_LENGTH, violating rfc3875 -- WAS: Problem with git-http-backend.exe as iis cgi Jeff King
2016-03-30  9:08   ` AW: " Florian Manschwetus
2016-04-01 23:55     ` Jeff King
2017-11-23 23:45       ` [PATCH] http-backend: respect CONTENT_LENGTH as specified by rfc3875 Max Kirillov
2017-11-24  1:30         ` Eric Sunshine
2017-11-25 21:47           ` Max Kirillov
2017-11-26  0:38             ` Eric Sunshine
2017-11-26  0:43               ` Max Kirillov
2017-11-24  5:54         ` Junio C Hamano
2017-11-24  8:30           ` AW: " Florian Manschwetus
2017-11-26  1:50           ` Max Kirillov
2017-11-26  1:47         ` [PATCH v4 0/2] " Max Kirillov
2017-11-26  1:47           ` [PATCH v4 1/2] " Max Kirillov
2017-11-26  1:47             ` [PATCH v4 2/2] t5560-http-backend-noserver.sh: add CONTENT_LENGTH cases Max Kirillov
2017-11-26  1:54         ` [PATCH v5 0/2] http-backend: respect CONTENT_LENGTH as specified by rfc3875 Max Kirillov
2017-11-26  1:54           ` [PATCH v5 1/2] " Max Kirillov
2017-11-26  3:46             ` Junio C Hamano
2017-11-26  8:13               ` Max Kirillov
2017-11-26  9:38                 ` Junio C Hamano
2017-11-26 19:39                   ` Max Kirillov
2017-11-26  1:54           ` [PATCH v5 2/2] t5560-http-backend-noserver.sh: add CONTENT_LENGTH cases Max Kirillov
2017-11-26 19:38           ` [PATCH v6 0/2] http-backend: respect CONTENT_LENGTH as specified by rfc3875 Max Kirillov
2017-11-26 19:38             ` [PATCH v6 1/2] " Max Kirillov
2017-11-26 22:08               ` Eric Sunshine
2017-11-29  3:22               ` Jeff King
2017-12-03  1:02                 ` Junio C Hamano
2017-12-03  2:49                   ` Jeff King
2017-12-03  6:07                     ` Junio C Hamano
2017-12-04  7:18                       ` AW: " Florian Manschwetus
2017-12-04 17:13                         ` Jeff King
2017-11-26 19:38             ` [PATCH v6 2/2] t5560-http-backend-noserver.sh: add CONTENT_LENGTH cases Max Kirillov
2017-11-26 22:18               ` Eric Sunshine
2017-11-26 22:40                 ` Max Kirillov
2017-11-29  3:26                   ` Jeff King
2017-11-29  5:19                     ` Max Kirillov
2017-12-03  0:46                       ` Junio C Hamano
2017-11-27  0:29               ` Junio C Hamano
2017-11-27  4:02             ` [PATCH v6 0/2] http-backend: respect CONTENT_LENGTH as specified by rfc3875 Junio C Hamano
2017-11-29  5:07               ` Max Kirillov
2017-12-03  0:48                 ` Junio C Hamano
2017-12-12 16:17                   ` Need to add test artifacts to .gitignore Dan Jacques
2017-12-12 19:00                     ` [RFC PATCH] t/helper: Move sources to t/helper-src; gitignore any files in t/helper Stefan Beller
2017-12-12 19:59                       ` Junio C Hamano
2017-12-12 20:56                         ` [PATCH] t/helper: ignore everything but sources Stefan Beller
2017-12-12 21:06                           ` Junio C Hamano
2017-12-13 20:12                             ` Stefan Beller
2017-12-12 21:06                           ` Todd Zullinger
2017-12-19 22:13             ` [PATCH v6 0/2] http-backend: respect CONTENT_LENGTH as specified by rfc3875 Junio C Hamano
2017-12-20  4:30               ` Max Kirillov

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=F0F5A56A22F20D4CB4A03BB8D6658797E261A3D6@SERVER2011.CS-SOFTWARE.local \
    --to=manschwetus@cs-software-gmbh.de \
    --cc=git@vger.kernel.org \
    --cc=judge.packham@gmail.com \
    --cc=kostix+git@007spb.ru \
    /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).