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

Add 2to3 fixer to change send and recv methods of socket object. #79074

Closed
devarakondapranav mannequin opened this issue Oct 4, 2018 · 14 comments
Closed

Add 2to3 fixer to change send and recv methods of socket object. #79074

devarakondapranav mannequin opened this issue Oct 4, 2018 · 14 comments
Assignees
Labels
topic-2to3 type-feature A feature request or enhancement

Comments

@devarakondapranav
Copy link
Mannequin

devarakondapranav mannequin commented Oct 4, 2018

BPO 34893
Nosy @rhettinger, @benjaminp, @serhiy-storchaka, @tirkarthi, @devarakondapranav
Files
  • fix_socket_send_recv.py: Fix to change send and recv of socket.
  • fix_socket_send_recv_updated.py: Updated fixer that handles cases when data is converted to bytes.
  • fix_socket_send_recv_reupdated.py
  • 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/benjaminp'
    closed_at = <Date 2018-11-27.04:41:27.440>
    created_at = <Date 2018-10-04.14:45:00.475>
    labels = ['type-feature', 'expert-2to3']
    title = 'Add 2to3 fixer to change send and recv methods of socket object.'
    updated_at = <Date 2018-11-27.04:41:27.439>
    user = 'https://github.com/devarakondapranav'

    bugs.python.org fields:

    activity = <Date 2018-11-27.04:41:27.439>
    actor = 'benjamin.peterson'
    assignee = 'benjamin.peterson'
    closed = True
    closed_date = <Date 2018-11-27.04:41:27.440>
    closer = 'benjamin.peterson'
    components = ['2to3 (2.x to 3.x conversion tool)']
    creation = <Date 2018-10-04.14:45:00.475>
    creator = 'devarakondapranav'
    dependencies = []
    files = ['47850', '47854', '47864']
    hgrepos = []
    issue_num = 34893
    keywords = []
    message_count = 14.0
    messages = ['327053', '327064', '327070', '327072', '327079', '327198', '327222', '327257', '327295', '327299', '327300', '327320', '327557', '330489']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'benjamin.peterson', 'serhiy.storchaka', 'xtreak', 'devarakondapranav']
    pr_nums = []
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue34893'
    versions = []

    @devarakondapranav
    Copy link
    Mannequin Author

    devarakondapranav mannequin commented Oct 4, 2018

    The send() method of the the Socket object in Python 3.x requires the data to be sent to be first converted into bytes(object) which was not the case with Python 2.x. The 2to3 tool doesn't handle this case and hence an explicit fixer would help many beginners learning the socket module(most tutorials regarding this online are in Python 2.x)

    Similarly the fixer can change the recv() method by converting the returned bytes object back to a str object.
    For example, consider the following lines in Python 2.x, (for demonstration only)

    s.send("Foo") #where 's' is a socket object
    data = s.recv(1024)

    After the 2to3 fixer has been applied the above lines will change to,

    s.send(str.encode("Foo"))
    data = s.recv(1024).decode("utf-8")

    PS: I am a beginner in open source so any changes or suggestions are welcome.

    @devarakondapranav devarakondapranav mannequin added topic-2to3 type-feature A feature request or enhancement labels Oct 4, 2018
    @tirkarthi
    Copy link
    Member

    Thanks for the report. In my opinion it's difficult to handle this scenario and as far as I know 2to3 can only do syntax level changes with modular fixers for each case. Since Python is dynamic and there is no static type system available it's difficult to differentiate between a socket object and any other object that has the same API with send and recv methods like below :

    if foo():
        s = Socket()
    else:
        s = AnotherSocketAPI()

    Hence in this case s might be another object that might have the same interface like Socket expecting string for send and recv. Since there is no static typing in Python it depends on runtime introspection to get it right. I will wait for others opinion on this.

    @devarakondapranav
    Copy link
    Mannequin Author

    devarakondapranav mannequin commented Oct 4, 2018

    Thanks for taking time and updating this, Karthikeyan Singaravelan. I do agree that there there is no proper way to find out if an object is of type socket or not.

    However other fixers in lib2to3 are not any different. For example the fix_dict.py changes every instance of viewkeys() method to keys() irrespective of the type of the object. And I guess that applies to all other fixers in lib2to3 as well. So that convinced me that a fix for this can also be accommodated.

    A compromise that we could foster, as I already mentioned is to make this fix explicit so users can use this only if they need it. Please find attached the fixer I wrote. I haven't made a PR yet.

    @tirkarthi
    Copy link
    Member

    Ah, sorry I totally forgot about dict fixer. Thanks for the details. I would wait for Benjamin's call on this.

    Thanks

    @devarakondapranav
    Copy link
    Mannequin Author

    devarakondapranav mannequin commented Oct 4, 2018

    Thanks Karthikeyan

    @rhettinger
    Copy link
    Contributor

    Please find attached the fixer I wrote.

    Wouldn't this break programs that had already run encode() on the input (as they were already supposed to be doing, and as they would be doing for code that runs under both Python2 and Python3)?

    @serhiy-storchaka
    Copy link
    Member

    I'm sure that this would break the code which sends bytes objects and expects to receive bytes objects.

    s.send(struct.pack('!I', x))
    q, w, e = struct.unpack('!IHQ', s.recv(4))

    @devarakondapranav
    Copy link
    Mannequin Author

    devarakondapranav mannequin commented Oct 6, 2018

    Thanks for pointing out these cases. I kept in mind the Python 2 documentation which says socket.send method expects a string and hence made that fixer. I have tweaked that fixer to handle the pointed cases and a few additional ones too. Please find attached the updated fixer. To summarize, the following send() cases would not be changed,

    data = "Data"
    s.send(str.encode(data))
    s.send(data.encode('utf-8))
    s.send(bytes(data, 'utf-8'))
    s.send(struct.pack('!I', x))

    Similary, the following recv() cases would not be changed,

    data = s.recv(1024).decode('utf-8')
    q, w, e = struct.unpack('!IHQ', s.recv(4))

    The remaining generic cases will be handled as expected. I know we cannot handle cases where the data has already been converted to bytes(since there is no way to find the 'type' of the object here) and then sent as an argument to socket.send(). But if we have to go by the documentation, then this is probably the right thing to do.

    @rhettinger
    Copy link
    Contributor

    I have tweaked that fixer to handle the pointed cases
    and a few additional ones too

    Playing whack-a-mole with a few cases will always fall short of being able to guarantee correct transformations and not break existing code that is working correctly.

    One possibility is to add a type check to the generated code, "send(x)" -> send(x.encode() if type(x)==bytes else x)" but this has its own issues and isn't satisfying.

    @devarakondapranav
    Copy link
    Mannequin Author

    devarakondapranav mannequin commented Oct 7, 2018

    One possibility is to add a type check to the generated code, "send(x)" -> send(x.encode() if type(x)==bytes else x)"

    That would have solved the problem. However we cannot check the type of the object while the parsing is going on. For example, printing out the type(x) for the above example in any of the fixers would print "lib2to3.pytree.Node" (or "lib2to3.pytree.Leaf" depending on the object) but not the expected type() of the object like str or bytes.

    I haven't found out any method that can actually find out the type of the object in the fixer. If that can be done, then writing the fixer is pretty much straight forward.

    The other fixers in lib2to3, for example fix_dict.py, would convert all instances of viewkeys() to keys() irrespective of the type of the object that has called the method. That is also the case with all other fixers as well. Would appreciate very much if somebody can suggest how to do this.

    But since that is not the case, the fixer code has to handle these cases individually and I expect the current fixer to do a good job for the same.

    @devarakondapranav
    Copy link
    Mannequin Author

    devarakondapranav mannequin commented Oct 7, 2018

    I am sorry. I got you completely wrong with this > "One possibility is to add a type check to the generated code"

    I thought you were asking to check the type in the fixer itself.
    I get it now. So you meant to add the type check for the changed/generate code.I think that is a good idea.

    but this has its own issues and isn't satisfying.
    Why do you think this way? Can you please elaborate?

    @rhettinger
    Copy link
    Contributor

    Benjamin, I'm dubious about this going forward, but it is up to you.

    @devarakondapranav
    Copy link
    Mannequin Author

    devarakondapranav mannequin commented Oct 12, 2018

    I have added a final condition that converts the arguments passed to bytes only if the type of object is socket and the method is send() in the generated code. All the other conversions still function as expected.

    @benjaminp
    Copy link
    Contributor

    I concur with Raymond. Thank you for the patch, but this seems too error-prone to commit.

    @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
    topic-2to3 type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants