Issue #13589: Fix some serialization primitives in the aifc module.

Patch by Oleg Plakhotnyuk.

4 files changed, 71 insertions, 18 deletions

diff --git a/Lib/aifc.py b/Lib/aifc.py
index 7774325..775f39c 100644
--- a/Lib/aifc.py
+++ b/Lib/aifc.py
@@ -162,6 +162,12 @@ def _read_short(file):
     except struct.error:
         raise EOFError
 
+def _read_ushort(file):
+    try:
+        return struct.unpack('>H', file.read(2))[0]
+    except struct.error:
+        raise EOFError
+
 def _read_string(file):
     length = ord(file.read(1))
     if length == 0:
@@ -194,13 +200,19 @@ def _read_float(f): # 10 bytes
 def _write_short(f, x):
     f.write(struct.pack('>h', x))
 
+def _write_ushort(f, x):
+    f.write(struct.pack('>H', x))
+
 def _write_long(f, x):
+    f.write(struct.pack('>l', x))
+
+def _write_ulong(f, x):
     f.write(struct.pack('>L', x))
 
 def _write_string(f, s):
     if len(s) > 255:
         raise ValueError("string exceeds maximum pstring length")
-    f.write(struct.pack('b', len(s)))
+    f.write(struct.pack('B', len(s)))
     f.write(s)
     if len(s) & 1 == 0:
         f.write(b'\x00')
@@ -218,7 +230,7 @@ def _write_float(f, x):
         lomant = 0
     else:
         fmant, expon = math.frexp(x)
-        if expon > 16384 or fmant >= 1:     # Infinity or NaN
+        if expon > 16384 or fmant >= 1 or fmant != fmant: # Infinity or NaN
             expon = sign|0x7FFF
             himant = 0
             lomant = 0
@@ -234,9 +246,9 @@ def _write_float(f, x):
             fmant = math.ldexp(fmant - fsmant, 32)
             fsmant = math.floor(fmant)
             lomant = int(fsmant)
-    _write_short(f, expon)
-    _write_long(f, himant)
-    _write_long(f, lomant)
+    _write_ushort(f, expon)
+    _write_ulong(f, himant)
+    _write_ulong(f, lomant)
 
 from chunk import Chunk
 
@@ -762,15 +774,15 @@ class Aifc_write:
         if self._aifc:
             self._file.write(b'AIFC')
             self._file.write(b'FVER')
-            _write_long(self._file, 4)
-            _write_long(self._file, self._version)
+            _write_ulong(self._file, 4)
+            _write_ulong(self._file, self._version)
         else:
             self._file.write(b'AIFF')
         self._file.write(b'COMM')
-        _write_long(self._file, commlength)
+        _write_ulong(self._file, commlength)
         _write_short(self._file, self._nchannels)
         self._nframes_pos = self._file.tell()
-        _write_long(self._file, self._nframes)
+        _write_ulong(self._file, self._nframes)
         _write_short(self._file, self._sampwidth * 8)
         _write_float(self._file, self._framerate)
         if self._aifc:
@@ -778,9 +790,9 @@ class Aifc_write:
             _write_string(self._file, self._compname)
         self._file.write(b'SSND')
         self._ssnd_length_pos = self._file.tell()
-        _write_long(self._file, self._datalength + 8)
-        _write_long(self._file, 0)
-        _write_long(self._file, 0)
+        _write_ulong(self._file, self._datalength + 8)
+        _write_ulong(self._file, 0)
+        _write_ulong(self._file, 0)
 
     def _write_form_length(self, datalength):
         if self._aifc:
@@ -791,8 +803,8 @@ class Aifc_write:
         else:
             commlength = 18
             verslength = 0
-        _write_long(self._file, 4 + verslength + self._marklength + \
-                    8 + commlength + 16 + datalength)
+        _write_ulong(self._file, 4 + verslength + self._marklength + \
+                     8 + commlength + 16 + datalength)
         return commlength
 
     def _patchheader(self):
@@ -810,9 +822,9 @@ class Aifc_write:
         self._file.seek(self._form_length_pos, 0)
         dummy = self._write_form_length(datalength)
         self._file.seek(self._nframes_pos, 0)
-        _write_long(self._file, self._nframeswritten)
+        _write_ulong(self._file, self._nframeswritten)
         self._file.seek(self._ssnd_length_pos, 0)
-        _write_long(self._file, datalength + 8)
+        _write_ulong(self._file, datalength + 8)
         self._file.seek(curpos, 0)
         self._nframes = self._nframeswritten
         self._datalength = datalength
@@ -827,13 +839,13 @@ class Aifc_write:
             length = length + len(name) + 1 + 6
             if len(name) & 1 == 0:
                 length = length + 1
-        _write_long(self._file, length)
+        _write_ulong(self._file, length)
         self._marklength = length + 8
         _write_short(self._file, len(self._markers))
         for marker in self._markers:
             id, pos, name = marker
             _write_short(self._file, id)
-            _write_long(self._file, pos)
+            _write_ulong(self._file, pos)
             _write_string(self._file, name)
 
 def open(f, mode=None):
diff --git a/Lib/test/test_aifc.py b/Lib/test/test_aifc.py
index 085b949..236f9b6 100644
--- a/Lib/test/test_aifc.py
+++ b/Lib/test/test_aifc.py
@@ -144,8 +144,45 @@ class AIFCTest(unittest.TestCase):
         self.assertRaises(aifc.Error, f.getmark, 3)
 
 
+class AIFCLowLevelTest(unittest.TestCase):
+
+    def test_read_written(self):
+        def read_written(self, what):
+            f = io.BytesIO()
+            getattr(aifc, '_write_' + what)(f, x)
+            f.seek(0)
+            return getattr(aifc, '_read_' + what)(f)
+        for x in (-1, 0, 0.1, 1):
+            self.assertEqual(read_written(x, 'float'), x)
+        for x in (float('NaN'), float('Inf')):
+            self.assertEqual(read_written(x, 'float'), aifc._HUGE_VAL)
+        for x in (b'', b'foo', b'a' * 255):
+            self.assertEqual(read_written(x, 'string'), x)
+        for x in (-0x7FFFFFFF, -1, 0, 1, 0x7FFFFFFF):
+            self.assertEqual(read_written(x, 'long'), x)
+        for x in (0, 1, 0xFFFFFFFF):
+            self.assertEqual(read_written(x, 'ulong'), x)
+        for x in (-0x7FFF, -1, 0, 1, 0x7FFF):
+            self.assertEqual(read_written(x, 'short'), x)
+        for x in (0, 1, 0xFFFF):
+            self.assertEqual(read_written(x, 'ushort'), x)
+
+    def test_read_raises(self):
+        f = io.BytesIO(b'\x00')
+        self.assertRaises(EOFError, aifc._read_ulong, f)
+        self.assertRaises(EOFError, aifc._read_long, f)
+        self.assertRaises(EOFError, aifc._read_ushort, f)
+        self.assertRaises(EOFError, aifc._read_short, f)
+
+    def test_write_long_string_raises(self):
+        f = io.BytesIO()
+        with self.assertRaises(ValueError):
+            aifc._write_string(f, b'too long' * 255)
+
+
 def test_main():
     run_unittest(AIFCTest)
+    run_unittest(AIFCLowLevelTest)
 
 
 if __name__ == "__main__":
diff --git a/Misc/ACKS b/Misc/ACKS
index 24f3f45..c888a8c 100644
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -719,6 +719,7 @@ Zach Pincus
 Michael Piotrowski
 Antoine Pitrou
 Jean-François Piéronne
+Oleg Plakhotnyuk
 Guilherme Polo
 Michael Pomraning
 Iustin Pop
diff --git a/Misc/NEWS b/Misc/NEWS
index c061809..4bdf418 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -100,6 +100,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #13589: Fix some serialization primitives in the aifc module.
+  Patch by Oleg Plakhotnyuk.
+
 - Issue #13642: Unquote before b64encoding user:password during Basic
   Authentication. Patch contributed by Joonas Kuorilehto.