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

ctypes s_set() uses strlen() and so truncates string at null character #83774

Closed
shihai1991 opened this issue Feb 9, 2020 · 7 comments
Closed
Labels
3.10 only security fixes tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@shihai1991
Copy link
Member

BPO 39593
Nosy @vstinner, @shihai1991
PRs
  • bpo-39593: Adding an unit test of ctypes #18424
  • 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 = None
    closed_at = <Date 2020-06-01.16:55:09.527>
    created_at = <Date 2020-02-09.16:17:25.605>
    labels = ['type-feature', 'tests', '3.10']
    title = 'ctypes s_set() uses strlen() and so truncates string at null character'
    updated_at = <Date 2020-06-01.16:55:09.525>
    user = 'https://github.com/shihai1991'

    bugs.python.org fields:

    activity = <Date 2020-06-01.16:55:09.525>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-06-01.16:55:09.527>
    closer = 'vstinner'
    components = ['Tests']
    creation = <Date 2020-02-09.16:17:25.605>
    creator = 'shihai1991'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39593
    keywords = ['patch']
    message_count = 7.0
    messages = ['361656', '361690', '361782', '361789', '361790', '370562', '370563']
    nosy_count = 2.0
    nosy_names = ['vstinner', 'shihai1991']
    pr_nums = ['18424']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue39593'
    versions = ['Python 3.10']

    @shihai1991
    Copy link
    Member Author

    strlen(data) can not be replaced by Py_SIZE(value) in https://github.com/python/cpython/blob/master/Modules/_ctypes/cfield.c#L1297.

    victor have give a great example about it in #18419

    I create this bpo for two reasons:

    1. This question info could be removed(some questions info will catch my attention)
    2. Current tests should be enhanced(It can not help me found this backward incompatible:( ).

    @shihai1991 shihai1991 added 3.9 only security fixes tests Tests in the Lib/test dir type-feature A feature request or enhancement labels Feb 9, 2020
    @vstinner
    Copy link
    Member

    Using strlen() seems to be as old as ctypes itself.

    --

    I don't know which behavior is correct. It's a little bit strange that ctypes truncate the string at "\0". A "char" buffer can be an arbitrary array of bytes. It may be a binary format which doesn't use null byte as string terminator, but just to encode a 16-bit integer as two bytes.

    My attempt to understand the current behavior:

    #18419 (review)

    "size = strlen(data);" instruction was added when the ctypes was added by this commit:

    commit d4c9320
    Author: Thomas Heller <theller@ctypes.org>
    Date: Wed Mar 8 19:35:11 2006 +0000

    Copy ctypes-0.9.9.4 sources from external into the trunk.
    

    Sadly, Thomas Heller no longer contributes to Python since 2011:
    https://blog.python.org/2011/04/thomas-heller-steps-down-as-ctypes.html

    The original project is hosted at:
    https://sourceforge.net/p/ctypes/code/

    It seems like s_set() function was added between these source/cfield.c two versions:

    revision 1.116
    date: 2006/03/15 20:35:55; author: theller; state: Exp; lines: +14 -4
    PyString_FromFormat()b understands the C99 "z" qualifier on all
    platforms, in Python 2.5. Patch from Tim Peters, adapted to work with
    older Python versions.

    revision 1.115
    date: 2006/03/03 20:17:15; author: theller; state: Exp; lines: +636 -279
    Moving files from branch_1_0 to HEAD.

    Sadly, the commit message doesn't say much about the rationale.

    @vstinner vstinner changed the title Adding a unit test of ctypes ctypes s_set() uses strlen() and so truncates string at null character Feb 10, 2020
    @vstinner vstinner changed the title Adding a unit test of ctypes ctypes s_set() uses strlen() and so truncates string at null character Feb 10, 2020
    @shihai1991
    Copy link
    Member Author

    releated bpo: bpo-12769

    s_get() in cfield.c function have similar behavior. So far, this is a planned action(lack evidence).

    @shihai1991
    Copy link
    Member Author

    I am not sure it have realtion of libffi's type or not?

    s_set() and s_get()'s type of ffi is FFI_TYPE_POINTER;

    REF: https://www.manpagez.com/info/libffi/libffi-3.0.13/libffi_6.php#index-ffi_005ftype_005fpointer

    @shihai1991
    Copy link
    Member Author

    I guess the xx_get() and xx_set() function's process logic have relation with ffi's type, after I checked some xx_get() and xx_set()'s process logic.

    @vstinner
    Copy link
    Member

    vstinner commented Jun 1, 2020

    New changeset a97011b by Hai Shi in branch 'master':
    bpo-39593: Add test on ctypes cfield.c s_set() (GH-18424)
    a97011b

    @vstinner
    Copy link
    Member

    vstinner commented Jun 1, 2020

    Thanks Hai Shi.

    @vstinner vstinner added 3.10 only security fixes and removed 3.9 only security fixes labels Jun 1, 2020
    @vstinner vstinner closed this as completed Jun 1, 2020
    @vstinner vstinner added 3.10 only security fixes and removed 3.9 only security fixes labels Jun 1, 2020
    @vstinner vstinner closed this as completed Jun 1, 2020
    @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
    3.10 only security fixes tests Tests in the Lib/test dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants