revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 14:52:41 +01:00
Code Issues Releases Wiki Activity

Fix isValidGitShallowCloneResponse

Browse source 
Summary:
Changes the heuristic method by which non-zero exit statuses from git-http-backend are found to be due to packfile negotiation during shallow fetches, etc.

Instead of checking git-http-backend stderr for a generic "hung up" error message, see if the pack-result response contains a terminating flush packet ("0000"). This should give a greater assurance that the request was handled correctly and the response is complete.

Test Plan: Run `GIT_CURL_VERBOSE=1 git fetch --depth 1 https://host.example/source/repo.git HEAD` to ensure it completes and includes two successful POST requests during packfile negotiation (the last one actually receives the packfile).

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, dzduvall

Tags: #diffusion

Differential Revision: https://secure.phabricator.com/D21484
This commit is contained in:
epriestley 2020-10-30 13:03:34 -07:00
parent 671986592b
commit c04147328f
1 changed files with 14 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
src/applications/diffusion/controller/DiffusionServeController.php
View file

@ -922,18 +922,26 @@ final class DiffusionServeController extends DiffusionController {
// This is a pretty funky fix: it would be nice to more precisely detect // This is a pretty funky fix: it would be nice to more precisely detect
// that a request is a `--depth N` clone request, but we don't have any code // that a request is a `--depth N` clone request, but we don't have any code
// to decode protocol frames yet. Instead, look for reasonable evidence // to decode protocol frames yet. Instead, look for reasonable evidence
// in the error and output that we're looking at a `--depth` clone. // in the output that we're looking at a `--depth` clone.
// For evidence this isn't completely crazy, see: // A valid x-git-upload-pack-result response during packfile negotiation
// https://github.com/schacon/grack/pull/7 // should end with a flush packet ("0000"). As long as that packet
// terminates the response body in the response, we'll assume the response
// is correct and complete.
// See https://git-scm.com/docs/pack-protocol#_packfile_negotiation
$stdout_regexp = '(^Content-Type: application/x-git-upload-pack-result)m'; $stdout_regexp = '(^Content-Type: application/x-git-upload-pack-result)m';
$stderr_regexp = '(The remote end hung up unexpectedly)';
$has_pack = preg_match($stdout_regexp, $stdout); $has_pack = preg_match($stdout_regexp, $stdout);
$is_hangup = preg_match($stderr_regexp, $stderr);
return $has_pack && $is_hangup; if (strlen($stdout) >= 4) {
$has_flush_packet = (substr($stdout, -4) === "0000");
} else {
$has_flush_packet = false;
}
return ($has_pack && $has_flush_packet);
} }
private function getCommonEnvironment(PhabricatorUser $viewer) { private function getCommonEnvironment(PhabricatorUser $viewer) {