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

xmlrpc.server assumes sys.stdout will have a buffer attribute #51414

Closed
ncoghlan opened this issue Oct 18, 2009 · 8 comments
Closed

xmlrpc.server assumes sys.stdout will have a buffer attribute #51414

ncoghlan opened this issue Oct 18, 2009 · 8 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ncoghlan
Copy link
Contributor

BPO 7165
Nosy @loewis, @ncoghlan, @pitrou, @vstinner, @bitdancer
Files
  • test_xmlrpc.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/ncoghlan'
    closed_at = <Date 2010-01-31.06:24:20.289>
    created_at = <Date 2009-10-18.12:40:47.468>
    labels = ['type-bug', 'library']
    title = 'xmlrpc.server assumes sys.stdout will have a buffer attribute'
    updated_at = <Date 2010-01-31.06:24:20.288>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2010-01-31.06:24:20.288>
    actor = 'loewis'
    assignee = 'ncoghlan'
    closed = True
    closed_date = <Date 2010-01-31.06:24:20.289>
    closer = 'loewis'
    components = ['Library (Lib)']
    creation = <Date 2009-10-18.12:40:47.468>
    creator = 'ncoghlan'
    dependencies = []
    files = ['15176']
    hgrepos = []
    issue_num = 7165
    keywords = ['patch']
    message_count = 8.0
    messages = ['94212', '94317', '94469', '94476', '94479', '94514', '94516', '98591']
    nosy_count = 5.0
    nosy_names = ['loewis', 'ncoghlan', 'pitrou', 'vstinner', 'r.david.murray']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue7165'
    versions = ['Python 3.1', 'Python 3.2']

    @ncoghlan
    Copy link
    Contributor Author

    The xmlrpc tests were changed to use a StringIO object instead of a
    temporary file (this change reflects the corresponding change made on
    the 2.x trunk).

    The tests then started failing, since xmlrpc.server assumes sys.stdout
    will provide the buffer attribute, which is not a valid assumption. From
    the io.TestIOBase documentation:

    buffer
    The underlying binary buffer (a BufferedIOBase instance) that
    TextIOBase deals with. This is not part of the TextIOBase API and may
    not exist on some implementations.

    Assuming this attribute exists is a bug in xmlrpc.server that should be
    fixed.

    @ncoghlan ncoghlan added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Oct 18, 2009
    @pitrou
    Copy link
    Member

    pitrou commented Oct 21, 2009

    Well this is in the CGI implementation, and it's reasonable to assume
    that CGI will only be used with a real stream. Besides, the encoding
    used with stdout must be the xmlrpc encoding (probably utf-8), not the
    default encoding of standard streams on the system, which means direct
    access to the underlying binary stream must be possible.

    (however, _marshaled_dispatch() is quite strange in that it accepts str
    but returns bytes)

    I propose to fix test_xmlrpc instead so that it uses a BytesIO wrapped
    in a TextIOWrapper. Patch included.

    @bitdancer
    Copy link
    Member

    I think Antoine's suggestion is reasonable, and that we should apply the
    patch to stop the buildbots from failing, but leave this issue open
    until someone with more xmlrpc knowledge can respond to Nick and
    Antoine's questions.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Oct 26, 2009

    I agree with Antoine that CGIXML... is right in accessing
    sys.stdout.buffer; this really needs binary IO.

    Consequentially, his patch also looks right to me; please apply.

    @loewis loewis mannequin assigned pitrou Oct 26, 2009
    @bitdancer
    Copy link
    Member

    Patch applied in r75710.

    @ncoghlan
    Copy link
    Contributor Author

    Fine by me (I was very close to changing the test when I first triggered
    the problem, but wasn't sure silencing the error was the right thing to do).

    Assigning back to myself to add a comment to the relevant line (i.e.
    that relying on the buffer attribute being present is a deliberate
    design choice).

    @ncoghlan ncoghlan assigned ncoghlan and unassigned pitrou Oct 26, 2009
    @pitrou
    Copy link
    Member

    pitrou commented Oct 26, 2009

    Assigning back to myself to add a comment to the relevant line (i.e.
    that relying on the buffer attribute being present is a deliberate
    design choice).

    The remaining question, however, is why it doesn't also rely on
    stdin.buffer.

    @vstinner
    Copy link
    Member

    antoine's patch (fixing this issue) is commited. Can we close this issue?

    @loewis loewis mannequin closed this as completed Jan 31, 2010
    @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
    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