fix(git-export): more robust subprocess handling

Make sure the process is waited for in all branches after reading data.
WeblateOrg · Jun 18, 2024 · a1bdded · a1bdded
1 parent 7af1807
commit a1bdded
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 6 deletions.
diff --git a/weblate/gitexport/tests.py b/weblate/gitexport/tests.py
@@ -98,8 +98,9 @@ def test_redirect_link(self) -> None:
         )
         self.assertRedirects(
             response,
-            "/git/test/test/info/refs??service=git-upload-pack",
+            "/git/test/test/info/refs?service=git-upload-pack",
             status_code=301,
+            fetch_redirect_response=False,
         )
 
     def test_reject_push(self) -> None:

diff --git a/weblate/gitexport/views.py b/weblate/gitexport/views.py
@@ -96,7 +96,7 @@ def git_export(request, path, git_request):
         raise Http404("Not a git repository")
     if obj.is_repo_link:
         return redirect(
-            "{}?{}".format(
+            "{}{}".format(
                 reverse(
                     "git-export",
                     kwargs={
@@ -114,6 +114,18 @@ def git_export(request, path, git_request):
     return wrapper.get_response()
 
 
+class GitStreamingHttpResponse(StreamingHttpResponse):
+    def __init__(self, streaming_content, *args, **kwargs):
+        super().__init__(streaming_content.stream(), *args, **kwargs)
+        self.wrapper = streaming_content
+
+    def close(self):
+        if self.wrapper.process.poll() is None:
+            self.wrapper.process.kill()
+        self.wrapper.process.wait()
+        super().close()
+
+
 class GitHTTPBackendWrapper:
     def __init__(self, obj, request, git_request: str) -> None:
         self.path = os.path.join(obj.full_path, git_request)
@@ -202,7 +214,7 @@ def get_response(self):
         self.selector.unregister(stderr)
         self.selector.close()
 
-        retcode = self.process.wait()
+        retcode = self.process.poll()
 
         output_err = b"".join(self._stderr).decode()
 
@@ -217,16 +229,17 @@ def get_response(self):
             )
 
         # Handle failure
-        if retcode:
+        if retcode is not None and retcode != 0:
             return HttpResponseServerError(output_err)
 
         message = message_from_string(self._headers.decode())
 
         # Handle status in response
         if "status" in message:
+            self.process.wait()
             return HttpResponse(status=int(message["status"].split()[0]))
 
         # Send streaming content as response
-        return StreamingHttpResponse(
-            streaming_content=self.stream(), content_type=message["content-type"]
+        return GitStreamingHttpResponse(
+            streaming_content=self, content_type=message["content-type"]
         )