Thread (5 messages) flat view 5 messages, 3 authors, 2018-09-07

Re: [PATCH] http-backend: allow empty CONTENT_LENGTH

From: Max Kirillov <hidden>
Date: 2018-09-07 03:27:50

On Thu, Sep 06, 2018 at 02:54:18PM -0700, Junio C Hamano wrote:
Max Kirillov [off-list ref] writes:
quoted
This should fix it. I'm not sure should it treat it as 0 or "-1"
At least the tests mentioned by Jeff fails if I try to treat missing CONTENT_LENGTH as "-1"
So keep the existing behavior as much as possible
I am not sure what you mean by the above, between 0 and -1.  The
code signals the caller of get_content_length() that req_len is -1
which is used as a sign to read through to the EOF, so it appears to
me that the code treats missing content-length (i.e. str == NULL
case) as "-1".
I made a mistake in this, it should be "if I try to treat missing
CONTENT_LENGTH as 0". This, as far as I understand, what the
RFC specifies.

That is, after the following change, the test "large fetch-pack
requests can be split across POSTs" from t5551 starts faliing:

-- >8 --
@@ -353,8 +353,12 @@ static ssize_t get_content_length(void)
        ssize_t val = -1;
        const char *str = getenv("CONTENT_LENGTH");
 
-       if (str && *str && !git_parse_ssize_t(str, &val))
-               die("failed to parse CONTENT_LENGTH: %s", str);
+       if (str && *str) {
+               if (!git_parse_ssize_t(str, &val))
+                       die("failed to parse CONTENT_LENGTH: %s", str);
+       } else
+               val = 0;
+
        return val;
 }
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help