python · remilapeyre · Jun 22, 2018 · Jun 26, 2018 · Jun 27, 2018 · Jun 29, 2018
diff --git a/Lib/json/tool.py b/Lib/json/tool.py
@@ -22,7 +22,7 @@ def main():
     parser = argparse.ArgumentParser(prog=prog, description=description)
     parser.add_argument('infile', nargs='?', type=argparse.FileType(),
                         help='a JSON file to be validated or pretty-printed')
-    parser.add_argument('outfile', nargs='?', type=argparse.FileType('w'),
+    parser.add_argument('outfile', nargs='?', default='-',
                         help='write the output of infile to outfile')
     parser.add_argument('--sort-keys', action='store_true', default=False,
                         help='sort the output of dictionaries alphabetically by key')
@@ -31,21 +31,26 @@ def main():
     options = parser.parse_args()
 
     infile = options.infile or sys.stdin
-    outfile = options.outfile or sys.stdout
     sort_keys = options.sort_keys
     json_lines = options.json_lines
-    with infile, outfile:
+    with infile:
         try:
             if json_lines:
-                objs = (json.loads(line) for line in infile)
+                objs = tuple(json.loads(line) for line in infile)
             else:
                 objs = (json.load(infile), )
-            for obj in objs:
-                json.dump(obj, outfile, sort_keys=sort_keys, indent=4)
-                outfile.write('\n')
         except ValueError as e:
             raise SystemExit(e)
 
+    try:
+        outfile = sys.stdout if options.outfile == '-' else open(options.outfile, 'w')
+    except IOError as e:
+        parser.error(f"can't open '{options.outfile}': {str(e)}")
+    with outfile:
+        for obj in objs:
+            json.dump(obj, outfile, sort_keys=sort_keys, indent=4)
+            outfile.write('\n')
+
 
 if __name__ == '__main__':
     main()
diff --git a/Lib/test/test_json/test_tool.py b/Lib/test/test_json/test_tool.py
@@ -1,10 +1,13 @@
+import io
 import os
 import sys
 import textwrap
 import unittest
+import types
 from subprocess import Popen, PIPE
 from test import support
-from test.support.script_helper import assert_python_ok
+from test.support.script_helper import assert_python_ok, assert_python_failure
+from unittest import mock
 
 
 class TestTool(unittest.TestCase):
@@ -99,7 +102,6 @@ def _create_infile(self):
     def test_infile_stdout(self):
         infile = self._create_infile()
         rc, out, err = assert_python_ok('-m', 'json.tool', infile)
-        self.assertEqual(rc, 0)
         self.asse
8000
rtEqual(out.splitlines(), self.expect.encode().splitlines())
         self.assertEqual(err, b'')
 
@@ -110,10 +112,22 @@ def test_infile_outfile(self):
         self.addCleanup(os.remove, outfile)
         with open(outfile, "r") as fp:
             self.assertEqual(fp.read(), self.expect)
-        self.assertEqual(rc, 0)
         self.assertEqual(out, b'')
         self.assertEqual(err, b'')
 
+    def test_infile_same_outfile(self):
+        infile = self._create_infile()
+        rc, out, err = assert_python_ok('-m', 'json.tool', infile, infile)
+        self.assertEqual(out, b'')
+        self.assertEqual(err, b'')
+
+    def test_unavailable_outfile(self):
+        infile = self._create_infile()
+        rc, out, err = assert_python_failure('-m', 'json.tool', infile, '/bla/outfile')
+        self.assertEqual(rc, 2)
+        self.assertEqual(out, b'')
+        self.assertIn(b"error: can't open '/bla/outfile': [Errno 2]", err)
+
     def test_jsonlines(self):
         args = sys.executable, '-m', 'json.tool', '--json-lines'
         with Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE) as proc:
@@ -123,14 +137,54 @@ def test_jsonlines(self):
 
     def test_help_flag(self):
         rc, out, err = assert_python_ok('-m', 'json.tool', '-h')
-        self.assertEqual(rc, 0)
         self.assertTrue(out.startswith(b'usage: '))
         self.assertEqual(err, b'')
 
     def test_sort_keys_flag(self):
         infile = self._create_infile()
         rc, out, err = assert_python_ok('-m', 'json.tool', '--sort-keys', infile)
-        self.assertEqual(rc, 0)
         self.assertEqual(out.splitlines(),
                          self.expect_without_sort_keys.encode().splitlines())
         self.assertEqual(err, b'')
+
+    def test_no_fd_leak_infile_outfile(self):
+        infile = self._create_infile()
+        closed, opened, open = mock_open()
+        with mock.patch('builtins.open', side_effect=open):
+            with mock.patch.object(sys, 'argv', ['tool.py', infile, infile + '.out']):
+                import json.tool
+                json.tool.main()
+
+        os.unlink(infile + '.out')
+        self.assertEqual(opened, closed)
+        self.assertEqual(len(opened), len(closed))
+
+    def test_no_fd_leak_same_infile_outfile(self):
+        infile = self._create_infile()
+        closed, opened, open = mock_open()
+        with mock.patch('builtins.open', side_effect=open):
+            with mock.patch.object(sys, 'argv', ['tool.py', infile, infile]):
+                try:
+                    import json.tool
+                    json.tool.main()
+                except SystemExit:
+                    pass
+
+        self.assertEqual(opened, closed)
+        self.assertEqual(len(opened), len(closed))
+
+def mock_open():
+    closed = []
+    opened = []
+    io_open = io.open
+
+    def _open(*args, **kwargs):
+        fd = io_open(*args, **kwargs)
+        opened.append(fd)
+        fd_close = fd.close
+        def close(self):
+            closed.append(self)
+            fd_close()
+        fd.close = types.MethodType(close, fd)
+        return fd
+    return closed, opened, _open
diff --git a/Misc/NEWS.d/next/Library/2018-06-27-14-46-03.bpo-33927.felCrI.rst b/Misc/NEWS.d/next/Library/2018-06-27-14-46-03.bpo-33927.felCrI.rst
@@ -0,0 +1,2 @@
+`json.tool` can now take the same file as input and ouput. Patch by Rémi
+Lapeyre.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		`json.tool` can now take the same file as input and ouput. Patch by Rémi
		Lapeyre.