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

os.urandom(1.1): infinite loop #47958

Closed
devdanzin mannequin opened this issue Aug 27, 2008 · 12 comments
Closed

os.urandom(1.1): infinite loop #47958

devdanzin mannequin opened this issue Aug 27, 2008 · 12 comments
Assignees
Labels
easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@devdanzin
Copy link
Mannequin

devdanzin mannequin commented Aug 27, 2008

BPO 3708
Nosy @gpshead, @amauryfa, @pitrou, @devdanzin, @benjaminp
Files
  • urandom.diff: Check len(bytes) against int(n)
  • urandom-float-and-close.diff: fix os.urandom infinite loop when passed a non integral float
  • 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/gpshead'
    closed_at = <Date 2008-09-02.05:36:35.825>
    created_at = <Date 2008-08-27.23:30:11.865>
    labels = ['easy', 'type-bug', 'library']
    title = 'os.urandom(1.1): infinite loop'
    updated_at = <Date 2008-09-18.22:53:36.436>
    user = 'https://github.com/devdanzin'

    bugs.python.org fields:

    activity = <Date 2008-09-18.22:53:36.436>
    actor = 'rpetrov'
    assignee = 'gregory.p.smith'
    closed = True
    closed_date = <Date 2008-09-02.05:36:35.825>
    closer = 'gregory.p.smith'
    components = ['Library (Lib)']
    creation = <Date 2008-08-27.23:30:11.865>
    creator = 'ajaksu2'
    dependencies = []
    files = ['11277', '11281']
    hgrepos = []
    issue_num = 3708
    keywords = ['patch', 'easy']
    message_count = 12.0
    messages = ['72050', '72064', '72065', '72074', '72107', '72109', '72117', '72291', '72314', '72791', '72794', '73407']
    nosy_count = 6.0
    nosy_names = ['gregory.p.smith', 'amaury.forgeotdarc', 'pitrou', 'ajaksu2', 'benjamin.peterson', 'rpetrov']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue3708'
    versions = ['Python 2.6']

    @devdanzin
    Copy link
    Mannequin Author

    devdanzin mannequin commented Aug 27, 2008

    Calling os.urandom(1 + float(x)) ends in a infinite loop due to a naive
    condition check:

    while len(bytes) < n:
        bytes += read(_urandomfd, n - len(bytes))

    Trivial patch attached.

    @devdanzin devdanzin mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Aug 27, 2008
    @gpshead
    Copy link
    Member

    gpshead commented Aug 28, 2008

    i'll fix this and add a unit test.

    @gpshead gpshead added the easy label Aug 28, 2008
    @gpshead gpshead self-assigned this Aug 28, 2008
    @gpshead
    Copy link
    Member

    gpshead commented Aug 28, 2008

    better patch with tests attached, no explicit int conversion is done.

    i also wrapped the use of the fd returned by open with a try: finally:
    to avoid any chance of a leak and renamed the bytes variable to bs to
    match whats in py3k and avoid overriding the builtin type.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 28, 2008

    The explicit int() conversion looked saner to me, rather than passing a
    float argument to read().
    By the way, is there a reason this code still uses os.open rather than
    the builtin io.open?

    @gpshead
    Copy link
    Member

    gpshead commented Aug 28, 2008

    if i did

     n = int(n)

    that would change the API to allow bytes/unicode to be passed in which
    is not something i want. i don't even like that it allows floats.

    by not doing the int conversion at all, a DeprecationWarning is raised
    by the read() about the argument being a float which I figure it not a
    bad thing given that the API really should only accept ints...

    fwiw, daniel's patch would still cause this deprecation warning from
    read so I guess using while len(bs) < int(n): isn't that bad either.
    but it would allow a string such as '0' to be passed in as an argument
    without an exception...

    @devdanzin
    Copy link
    Mannequin Author

    devdanzin mannequin commented Aug 28, 2008

    Gregory,
    IMHO your patch is better in all aspects.

    Regarding my patch, the API wouldn't change at all, as the source reads:
    while len(bytes) < int(n):
    bytes += read(_urandomfd, n - len(bytes))

    So "n - len(bytes)" restricts the API to what it was before. But it
    would call int() for each loop iteration :/

    @pitrou: My patch still passed the float to read (to keep the current
    behavior, warning included), but if doing that can be considered a bug,
    let's get rid of the DeprecationWarning by passing an int.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 29, 2008

    Le jeudi 28 août 2008 à 19:30 +0000, Gregory P. Smith a écrit :

    if i did

    n = int(n)

    that would change the API to allow bytes/unicode to be passed in which
    is not something i want.

    Ok, I hadn't thought about that. You convinced me.

    @amauryfa
    Copy link
    Member

    amauryfa commented Sep 1, 2008

    The patch looks fine to me as well.

    @gpshead
    Copy link
    Member

    gpshead commented Sep 2, 2008

    committed to trunk r66142

    @gpshead gpshead closed this as completed Sep 2, 2008
    @benjaminp
    Copy link
    Contributor

    Gregory, could you merge this into py3k, please?

    @benjaminp
    Copy link
    Contributor

    Apparently this isn't an issue in py3k, so no worries! :)

    @rpetrov
    Copy link
    Mannequin

    rpetrov mannequin commented Sep 18, 2008

    It seems to me that test case will fail on windows and vms platforms.
    The case contain os.urandom(1.1) but in posixmodule.c for urandon
    functions (windows and vms) exits:
    PyArg_ParseTuple(args, "i:urandom", &howMany))

    ./python.exe -c 'import os; print "%s" %(os.urandom(1.9))' =>
    -c:1: DeprecationWarning: integer argument expected, got float

    @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 stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants