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

2018-09-08 Thread Max Kirillov
On Fri, Sep 07, 2018 at 02:49:23AM -0700, Junio C Hamano wrote: > In any case, hopefully we can fix this before the final, as this is > a regression introduced during this cycle? I think I am going to stop at the v4. Unless there are some corrections requested.

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

2018-09-07 Thread Max Kirillov
On Fri, Sep 07, 2018 at 02:49:23AM -0700, Junio C Hamano wrote: > Max Kirillov writes: > >> Actually, another reason for the latest issue was that CONTENT_LENGTH >> is parsed for GET requests at all. It should be parsed only for POST >> requests, or, rather, only for upoad-pack and receive-pack

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

2018-09-07 Thread Junio C Hamano
Max Kirillov writes: > Actually, another reason for the latest issue was that CONTENT_LENGTH > is parsed for GET requests at all. It should be parsed only for POST > requests, or, rather, only for upoad-pack and receive-pack requests. Not really. The layered design of the HTTP protocol means

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

2018-09-06 Thread Max Kirillov
Actually, another reason for the latest issue was that CONTENT_LENGTH is parsed for GET requests at all. It should be parsed only for POST requests, or, rather, only for upoad-pack and receive-pack requests.

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

2018-09-06 Thread Max Kirillov
On Thu, Sep 06, 2018 at 11:38:31PM -0400, Jeff King wrote: > My understanding from Jelmer's report is that a present-but-empty > variable should be counted as "0" to mean "do not read any body bytes". > That matches my reading of RFC 3875, which says: > > If no data is attached, then NULL (or

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

2018-09-06 Thread Jeff King
On Fri, Sep 07, 2018 at 06:27:40AM +0300, Max Kirillov wrote: > On Thu, Sep 06, 2018 at 02:54:18PM -0700, Junio C Hamano wrote: > > Max Kirillov writes: > >> 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

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

2018-09-06 Thread Max Kirillov
On Thu, Sep 06, 2018 at 02:54:18PM -0700, Junio C Hamano wrote: > Max Kirillov writes: >> 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

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

2018-09-06 Thread Jonathan Nieder
Hi, Max Kirillov wrote: > According to RFC3875, empty environment variable is equivalent to unset, > and for CONTENT_LENGTH it should mean zero body to read. > > However, as discussed in [1], unset CONTENT_LENGTH is also used for > chunked encoding to indicate reading until EOF, so keep this

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

2018-09-06 Thread Junio C Hamano
Max Kirillov writes: > According to RFC3875, empty environment variable is equivalent to unset, > and for CONTENT_LENGTH it should mean zero body to read. > > However, as discussed in [1], unset CONTENT_LENGTH is also used for > chunked encoding to indicate reading until EOF, so keep this

[PATCH] http-backend: allow empty CONTENT_LENGTH

2018-09-06 Thread Max Kirillov
According to RFC3875, empty environment variable is equivalent to unset, and for CONTENT_LENGTH it should mean zero body to read. However, as discussed in [1], unset CONTENT_LENGTH is also used for chunked encoding to indicate reading until EOF, so keep this behavior also for empty