classification
Title: Audioop decompression frames size check fix
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Oleg.Plakhotnyuk, ezio.melotti, haypo, neologix, pitrou, python-dev, sandro.tosi
Priority: normal Keywords: patch

Created on 2012-01-17 15:23 by Oleg.Plakhotnyuk, last changed 2012-02-04 17:56 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
audioop_size_check.patch Oleg.Plakhotnyuk, 2012-01-28 06:32 review
Messages (12)
msg151459 - (view) Author: Oleg Plakhotnyuk (Oleg.Plakhotnyuk) * Date: 2012-01-17 15:23
According to documentation (http://docs.python.org/library/audioop.html), adpcm2lin, alaw2lin and ulaw2lin are using 'width' argument to represent output frames width. However, in audioop.c module there are checks that are raising exceptions if input frames length is not multiple of 'width'. I have replaced checking of 'len' to match 'size' with checking of 'len*size' to match 'size' in order to retain only basic length validity checks.
msg151473 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-17 16:37
Well, you should just replace these calls with audioop_check_size() instead. Apparently the checks were blindly added in issue7673.

By the way, I'm surprised audioop accepts unicode strings... :/
msg151673 - (view) Author: Oleg Plakhotnyuk (Oleg.Plakhotnyuk) * Date: 2012-01-20 04:35
Yep, you're right. Didn't noticed audioop_check_size() function at first.

The fact that audioop accepts unicode strings seems weird to me too. I've replaced strings with bytes in tests. However, I'm afraid to add restrictions to library itself because of backward compatibility.
msg151702 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-20 17:54
Do you think it would be possible to add a test for valid compressed data that used to trigger the check?
msg152150 - (view) Author: Oleg Plakhotnyuk (Oleg.Plakhotnyuk) * Date: 2012-01-28 06:32
Yes, that tests really should be there too. Added them to patch.
msg152190 - (view) Author: Roundup Robot (python-dev) Date: 2012-01-28 21:07
New changeset 77188bc37c74 by Antoine Pitrou in branch '3.2':
Issue #13806: The size check in audioop decompression functions was too strict and could reject valid compressed data.
http://hg.python.org/cpython/rev/77188bc37c74

New changeset ce89dcba008f by Antoine Pitrou in branch 'default':
Issue #13806: The size check in audioop decompression functions was too strict and could reject valid compressed data.
http://hg.python.org/cpython/rev/ce89dcba008f

New changeset f1ee3bb6ba64 by Antoine Pitrou in branch '2.7':
Issue #13806: The size check in audioop decompression functions was too strict and could reject valid compressed data.
http://hg.python.org/cpython/rev/f1ee3bb6ba64
msg152191 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-28 21:08
Thank you for your work Oleg! The patch is now committed.
msg152624 - (view) Author: Charles-Fran├žois Natali (neologix) * (Python committer) Date: 2012-02-04 14:51
This broke a sparc buildbot:
"""

======================================================================
FAIL: test_alaw2lin (test.test_audioop.TestAudioop)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home2/buildbot/slave/2.7.loewis-sun/build/Lib/test/test_audioop.py", line 114, in test_alaw2lin
    self.assertEqual(audioop.alaw2lin(d, 2), b'\x08\x00\x08\x01\x10\x02')
AssertionError: '\x00\x08\x01\x08\x02\x10' != '\x08\x00\x08\x01\x10\x02'

======================================================================
FAIL: test_ulaw2lin (test.test_audioop.TestAudioop)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home2/buildbot/slave/2.7.loewis-sun/build/Lib/test/test_audioop.py", line 127, in test_ulaw2lin
    self.assertEqual(audioop.ulaw2lin(d, 2), b'\x00\x00\x04\x01\x0c\x02')
AssertionError: '\x00\x00\x01\x04\x02\x0c' != '\x00\x00\x04\x01\x0c\x02'
"""

It's obviously an endianess issue (sparc is big endian).
msg152627 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-04 15:46
New changeset bc6e768de2cc by Antoine Pitrou in branch '2.7':
Fix failing test on big-endian machines (issue #13806).
http://hg.python.org/cpython/rev/bc6e768de2cc
msg152628 - (view) Author: Roundup Robot (python-dev) Date: 2012-02-04 15:52
New changeset cfbd800af3a4 by Antoine Pitrou in branch '3.2':
Fix failing test on big-endian machines (issue #13806).
http://hg.python.org/cpython/rev/cfbd800af3a4

New changeset 05b40b006565 by Antoine Pitrou in branch 'default':
Fix failing test on big-endian machines (issue #13806).
http://hg.python.org/cpython/rev/05b40b006565
msg152629 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-02-04 15:52
Will wait for the buildbot to clear.
msg152638 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-02-04 17:56
The fix I committed seems ok. Unfortunately the buildbot doesn't compile 3.2 or 3.3, see #13843.
History
Date User Action Args
2012-02-04 17:56:42pitrousetstatus: open -> closed

messages: + msg152638
2012-02-04 15:52:39pitrousetmessages: + msg152629
2012-02-04 15:52:09python-devsetmessages: + msg152628
2012-02-04 15:46:58python-devsetmessages: + msg152627
2012-02-04 14:51:06neologixsetstatus: closed -> open
nosy: + neologix
messages: + msg152624

2012-01-28 21:08:11pitrousetstatus: open -> closed
resolution: fixed
messages: + msg152191

stage: resolved
2012-01-28 21:07:34python-devsetnosy: + python-dev
messages: + msg152190
2012-01-28 06:32:41Oleg.Plakhotnyuksetfiles: + audioop_size_check.patch

messages: + msg152150
2012-01-28 06:30:37Oleg.Plakhotnyuksetfiles: - audioop_size_check.patch
2012-01-20 17:54:56pitrousetmessages: + msg151702
2012-01-20 04:39:34Oleg.Plakhotnyuksetfiles: + audioop_size_check.patch
2012-01-20 04:39:07Oleg.Plakhotnyuksetfiles: - audioop_size_check.patch
2012-01-20 04:35:04Oleg.Plakhotnyuksetfiles: + audioop_size_check.patch

messages: + msg151673
2012-01-20 04:30:22Oleg.Plakhotnyuksetfiles: - audioop_size_check.patch
2012-01-17 16:37:44pitrousetnosy: + haypo, pitrou

messages: + msg151473
versions: + Python 2.7
2012-01-17 16:22:32pitroulinkissue13681 dependencies
2012-01-17 15:23:44Oleg.Plakhotnyukcreate