Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

audioop functions shouldn't accept strings #60889

Closed
serhiy-storchaka opened this issue Dec 14, 2012 · 18 comments
Closed

audioop functions shouldn't accept strings #60889

serhiy-storchaka opened this issue Dec 14, 2012 · 18 comments
Assignees
Labels
easy extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 16685
Nosy @jcea, @jaraco, @vstinner, @ezio-melotti, @serhiy-storchaka
Dependencies
  • bpo-16682: Document that audioop works with bytes, not strings
  • Files
  • audioop_buffer.patch
  • audioop_buffer_2.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2013-11-11.17:54:29.465>
    created_at = <Date 2012-12-14.17:37:35.570>
    labels = ['extension-modules', 'easy', 'type-feature']
    title = "audioop functions shouldn't accept strings"
    updated_at = <Date 2014-03-10.01:35:02.697>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2014-03-10.01:35:02.697>
    actor = 'python-dev'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2013-11-11.17:54:29.465>
    closer = 'serhiy.storchaka'
    components = ['Extension Modules']
    creation = <Date 2012-12-14.17:37:35.570>
    creator = 'serhiy.storchaka'
    dependencies = ['16682']
    files = ['32241', '32276']
    hgrepos = []
    issue_num = 16685
    keywords = ['patch', 'easy']
    message_count = 18.0
    messages = ['177477', '177487', '177488', '177513', '177515', '177516', '177517', '177522', '200556', '200675', '200738', '202499', '202560', '202596', '202606', '202635', '203030', '213004']
    nosy_count = 6.0
    nosy_names = ['jcea', 'jaraco', 'vstinner', 'ezio.melotti', 'python-dev', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue16685'
    versions = ['Python 3.4']

    @serhiy-storchaka
    Copy link
    Member Author

    Inadvertently strings accepted as arguments of audioop functions. This is a meaningless behavior and remnant of Python 2. We should drop string support.

    @serhiy-storchaka serhiy-storchaka added extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Dec 14, 2012
    @ezio-melotti
    Copy link
    Member

    What happens if you pass strings?
    If it doesn't work we can just fix it to raise an exception and stop returning random results, if it works but bytes should be used instead, we should go through a deprecation process.

    @serhiy-storchaka
    Copy link
    Member Author

    What happens if you pass strings?

    They are encoded with UTF-8. See 's#' and 's*' formats in PyArg_ParseTuple() ('y*' recommended for bytes).

    @vstinner
    Copy link
    Member

    Accepting Unicode strings is surprising and must fail. I don't think that a deprecation process is required here.

    @serhiy-storchaka
    Copy link
    Member Author

    Accepting Unicode strings is surprising and must fail. I don't think that a deprecation process is required here.

    Should it be fixed in 3.4 only or in all 3.x?

    @ezio-melotti
    Copy link
    Member

    The deprecation process can be avoided only if passing strings results in a meaningless result. If there are cases where users are passing strings and everything works fine -- even if they should be passing bytes instead -- we should deprecate strings first to warn the users that the "feature" is going to be removed.

    @serhiy-storchaka
    Copy link
    Member Author

    In 2.x it was have a little sense (if you use some 8-bit encoding as default encoding, but the default was 7-bit ascii). But with utf-8 it doesn't have sense and should quickly lead to an unexpected result (or just fail in most cases).

    @vstinner
    Copy link
    Member

    Should it be fixed in 3.4 only or in all 3.x?

    Such change cannot be done in a minor release (like 3.3.x), so it can
    only be done in Python 3.4.

    @serhiy-storchaka
    Copy link
    Member Author

    Here is a patch. Now audioop functions no more accept str, but accept bytes-like objects instead.

    @serhiy-storchaka serhiy-storchaka changed the title Deprecate accepting strings as arguments in audioop functions audioop functions shouldn't accept strings Oct 20, 2013
    @vstinner
    Copy link
    Member

    audioop_buffer.patch: In general, I don't like goto in C. But to cleanup something when exiting a function, I like goto to implement something similar to Python try/finally. What do you think Serhiy?

    @serhiy-storchaka
    Copy link
    Member Author

    Well. In updated patch I use gotos where it decreases the number of lines (total decreasing is almost 30 lines).

    @serhiy-storchaka
    Copy link
    Member Author

    If there are no objections I will commit last patch tomorrow.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 10, 2013

    New changeset bab0cbf86835 by Serhiy Storchaka in branch 'default':
    Issue bpo-16685: Added support for any bytes-like objects in the audioop module.
    http://hg.python.org/cpython/rev/bab0cbf86835

    @jaraco
    Copy link
    Member

    jaraco commented Nov 11, 2013

    The patch as committed causes the Windows 64-bit builds to fail to compile. http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/3187/steps/compile/logs/stdio

    @jaraco jaraco reopened this Nov 11, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 11, 2013

    New changeset 35cd00465624 by Serhiy Storchaka in branch 'default':
    Fixed compile error on Windows caused by arithmetic with void * pointers
    http://hg.python.org/cpython/rev/35cd00465624

    @serhiy-storchaka
    Copy link
    Member Author

    Fixed. Thank you Jason.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 16, 2013

    New changeset b96f4ee1b08b by Serhiy Storchaka in branch 'default':
    Issue bpo-16685: Added support for writing any bytes-like objects in the aifc,
    http://hg.python.org/cpython/rev/b96f4ee1b08b

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 10, 2014

    New changeset c375697f062e by R David Murray in branch 'default':
    whatsnew: auidioop.byteswap (bpo-16685).
    http://hg.python.org/cpython/rev/c375697f062e

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    easy extension-modules C modules in the Modules dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants