Page MenuHomePhabricator

arc seems to be broken with curl 7.43
Closed, ResolvedPublic

Description

I recently updated curl on my system to use 7.43 (I used 7.38 before). arc was clearly not happy with it.

All call conduit end up as follow:

ERR-CONDUIT-CORE: Invalid parameter information was passed to method 'differential.query', could not decode JSON serialization. Data: XXXX

Data being a truncated JSON, which explains why decoding failed.

Event Timeline

deadalnix raised the priority of this task from to High.
deadalnix updated the task description. (Show Details)
deadalnix added a project: Arcanist.
deadalnix added a subscriber: deadalnix.
epriestley raised the priority of this task from High to Needs Triage.Jun 24 2015, 11:28 AM

I ran into this and dug in a bit. It looked like the data was getting truncated, so I turned on CURLOPT_VERBOSE in HTTPSFuture.php. I noticed that the Content-Length header was getting set to the same value (39, for me) every time curl did an outbound request - even though strlen of the JSON body was something around 200.

This sounded like a stale client, so I commented out the curl client pooling code at line 204-211 and instead switched to just calling curl_init() every time with great success - removing the reuse of the client fixed this for me.

It looks like this has been reported to the cURL upstream in slightly different form already:

  • Thread starts here, but mis-identifies the root cause of the issue initially.
  • It looks like this describes the root cause better, and is consistent with the description above.
  • The libcurl maintainer proposes a patch, then merges it.

So this is likely fixed in cURL HEAD.

There doesn't seem to be an easy way for us to version-detect cURL -- phpversion("curl") returns false on my system. We could technically parse it out of phpinfo(), but this is really messy. We could run curl --version and assume whatever we find there is the same as the library version, but that's pretty sketchy too.

Oh, but there's a curl_version() function.

So I guess we can use that to version detect, then maybe disable handle reuse for 7.43, although it looks like the last couple of point releases were pretty quick so maybe we can just exit with a message if 7.43.1 gets cut shortly.

epriestley added a revision: Unknown Object (Differential Revision).Jun 24 2015, 11:12 PM

I think that patch fixes this, can someone who has a repro verify that it works for them? You can apply it with arc patch D13417 in libphutil/.

epriestley claimed this task.
epriestley added a subscriber: naitik.

Thanks. If this workaround doesn't work for anyone, let me know.

(+@naitik)

If you are unable to use arc patch due to https://secure.phabricator.com/T9361, you can do the following:

(1) Go to https://secure.phabricator.com/D13417
(2) Click download raw diff, and save it to ~/Desktop/diff.diff
(3) cd /path/to/phabricator/libphutil
(4) patch -p1 < ~/Desktop/diff.diff

No one should need to follow those instructions, because this patch has been in HEAD since late June.