Patchwork [2,of,3] hook: flush stdout before restoring stderr redirection

login
register
mail settings
Submitter Yuya Nishihara
Date Nov. 10, 2016, 2:29 p.m.
Message ID <0cba9f6a152e2b175e34.1478788158@mimosa>
Download mbox | patch
Permalink /patch/17447/
State Accepted
Headers show

Comments

Yuya Nishihara - Nov. 10, 2016, 2:29 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1478611342 -32400
#      Tue Nov 08 22:22:22 2016 +0900
# Node ID 0cba9f6a152e2b175e344bae23d0ec76a0ffe59d
# Parent  2d667ad51c1f2daa8ea5365709365d951f610aaa
hook: flush stdout before restoring stderr redirection

There was a similar issue to 8b011ededfb2. If an in-process hook writes
to stdout, the data may be buffered. In which case, stdout must be flushed
before restoring its file descriptor. Otherwise, remaining data would be sent
over the ssh wire and corrupts the protocol.

Note that this is a different redirection from the one I've just removed.

Patch

diff --git a/mercurial/hook.py b/mercurial/hook.py
--- a/mercurial/hook.py
+++ b/mercurial/hook.py
@@ -258,6 +258,7 @@  def runhooks(ui, repo, name, hooks, thro
             sys.stderr.flush()
     finally:
         if _redirect and oldstdout >= 0:
+            sys.__stdout__.flush()  # write hook output to stderr fd
             os.dup2(oldstdout, stdoutno)
             os.close(oldstdout)
 
diff --git a/tests/test-ssh.t b/tests/test-ssh.t
--- a/tests/test-ssh.t
+++ b/tests/test-ssh.t
@@ -265,8 +265,17 @@  a bad, evil hook that prints to stdout
   > sys.stdout.write("KABOOM\n")
   > EOF
 
-  $ echo '[hooks]' >> ../remote/.hg/hgrc
-  $ echo "changegroup.stdout = python $TESTTMP/badhook" >> ../remote/.hg/hgrc
+  $ cat <<EOF > $TESTTMP/badpyhook.py
+  > import sys
+  > def hook(ui, repo, hooktype, **kwargs):
+  >     sys.stdout.write("KABOOM IN PROCESS\n")
+  > EOF
+
+  $ cat <<EOF >> ../remote/.hg/hgrc
+  > [hooks]
+  > changegroup.stdout = python $TESTTMP/badhook
+  > changegroup.pystdout = python:$TESTTMP/badpyhook.py:hook
+  > EOF
   $ echo r > r
   $ hg ci -A -m z r
 
@@ -281,6 +290,7 @@  push should succeed even though it has a
   remote: adding file changes
   remote: added 1 changesets with 1 changes to 1 files
   remote: KABOOM
+  remote: KABOOM IN PROCESS
   $ hg -R ../remote heads
   changeset:   5:1383141674ec
   tag:         tip
@@ -447,6 +457,7 @@  stderr from remote commands should be pr
   remote: adding file changes
   remote: added 1 changesets with 1 changes to 1 files
   remote: KABOOM
+  remote: KABOOM IN PROCESS
   local stdout
 
 debug output