Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-22 Thread Peter Krefting
Jeff King: I was really hoping to avoid getting into all of the real-world messiness that a real http client needs to deal with (as opposed to just following the standards). Yeah, I agree, you're probably fine without all this detail in over 99% of the cases where this code would ever be expo

Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-22 Thread Jeff King
On Thu, May 22, 2014 at 08:12:58AM +0100, Peter Krefting wrote: > Kyle J. McKay: > > >I think that a strict reading of RFC 2616 allows "text/plain ; > >charset=utf-8" as well as "text/plain;charset=utf-8" and "text/plain; > >charset=utf-8". > > It does indeed, and I have seen servers send both v

Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-22 Thread Jeff King
On Thu, May 22, 2014 at 12:27:38AM -0700, Kyle J. McKay wrote: > Yeah I think so too. It's probably enough though just to just strip all " " > and "\t" characters at the same time the content type is lowercased. While > that would cause invalid content types such as "text / plain" to be > recogn

Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-22 Thread Kyle J. McKay
On May 21, 2014, at 23:05, Jeff King wrote: On Wed, May 21, 2014 at 05:07:38PM -0700, Kyle J. McKay wrote: + p = skip_prefix(type->buf, "text/plain"); + if (!p || (*p && *p != ';')) + return 0; + + return 1; +} + I think that a strict reading of RFC 2616 allow

Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-22 Thread Peter Krefting
Kyle J. McKay: I think that a strict reading of RFC 2616 allows "text/plain ; charset=utf-8" as well as "text/plain;charset=utf-8" and "text/plain; charset=utf-8". It does indeed, and I have seen servers send both variants, so they do need to be catered for. The number of servers that would

Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-21 Thread Jeff King
On Wed, May 21, 2014 at 05:07:38PM -0700, Kyle J. McKay wrote: > >+p = skip_prefix(type->buf, "text/plain"); > >+if (!p || (*p && *p != ';')) > >+return 0; > >+ > >+return 1; > >+} > >+ > > I think that a strict reading of RFC 2616 allows "text/plain ; > charset=utf-8" as

Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-21 Thread Kyle J. McKay
On May 21, 2014, at 03:33, Jeff King wrote: Commit 426e70d (remote-curl: show server content on http errors, 2013-04-05) tried to recognize text/plain content types, but fails to do so if they have any parameters. This patches makes our parsing more liberal, though we still do not do anything u

[PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

2014-05-21 Thread Jeff King
Commit 426e70d (remote-curl: show server content on http errors, 2013-04-05) tried to recognize text/plain content types, but fails to do so if they have any parameters. This patches makes our parsing more liberal, though we still do not do anything useful with a charset parameter. Signed-off-by: