Re: [PATCH] http-push: trim trailing newline from remote symref

From: Kyle J. McKay <mackyle@gmail.com>
Date: Jan 14, 2015, 00:21 UTC
Message-ID: <05091340-887B-4743-AB62-39E609E22CEE@gmail.com>
In-Reply-To: <20150113195806.GA13282@peff.net>

On Jan 13, 2015, at 11:58, Jeff King wrote:

Show 20 quoted lines

> On Tue, Jan 13, 2015 at 08:26:31AM -0800, Kyle J. McKay wrote:
>
>> I have this line in my 2.1.4 test output log:
>>
>> t5540-http-push-webdav.sh .......................... ok
>> [...]
>> I do not build with NO_EXPAT.  This is running the tests on OS X  
>> without
>> this patch applied.  Is something else required to get a failure?
>
> Hmm. I think it is probably this:
>
>  http://curl.haxx.se/docs/adv_20150108B.html
>
> where curl has started complaining about URLs with newlines in them.  
> So
> ae021d8 did introduce a bug, but older versions of curl did not really
> care. The combination of ae021d8 with a new version of curl triggers  
> the
> problem.

I'm running curl 7.38, and in this context "older" is anything before 7.40, so that would explain it. curl 7.38 was released 2014-09-10, so it's only 4 months old at this point. 7.40 was only released 5 days ago on 2015-01-08 which is probably why there have not been a whole lot of reports about this so far.

Show 8 quoted lines

> And that also explains why it worked prior to eecc8367f4; curl was  
> more
> forgiving. Also, interestingly, if you "git log -S'- 6' http-push.c",
> you can see the exact same bug reappear and go away in 2006/2007. The
> implicit "chop one character" behavior is there in the original
> 3dfaf7bc, adding http-push support. Then it disappears as a side  
> effect
> of bfbd0bb6, and then comes back again in eecc8367.

After updating to curl 7.40 I get:

t5540-http-push-webdav.sh (Wstat: 256 Tests: 19 Failed: 1)
   Failed test:  10
   Non-zero exit status: 1

> Anyway. I think my patch is still the right thing. But that does  
> explain
> why we didn't notice the test failure.

And then after applying your patch I'm back to:

t5540-http-push-webdav.sh .. ok

So definitely a needed patch.

Tested-by: Kyle J. McKay <mackyle@gmail.com>

-Kyle