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

Implement RFC 6855 (IMAP Support for UTF-8) in imaplib. #65999

Closed
zvyn mannequin opened this issue Jun 17, 2014 · 10 comments
Closed

Implement RFC 6855 (IMAP Support for UTF-8) in imaplib. #65999

zvyn mannequin opened this issue Jun 17, 2014 · 10 comments
Labels
expert-email type-feature A feature request or enhancement

Comments

@zvyn
Copy link
Mannequin

zvyn mannequin commented Jun 17, 2014

BPO 21800
Nosy @warsaw, @pitrou, @ericvsmith, @bitdancer, @soltysh, @zvyn
Files
  • imaplib_utf8_no_doc.patch: Proposed solution.
  • imaplib_utf8_issue21800.patch
  • issue21800.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 = None
    closed_at = <Date 2015-05-16.18:06:45.846>
    created_at = <Date 2014-06-17.23:28:22.752>
    labels = ['type-feature', 'expert-email']
    title = 'Implement RFC 6855 (IMAP Support for UTF-8) in imaplib.'
    updated_at = <Date 2015-05-16.18:06:45.845>
    user = 'https://github.com/zvyn'

    bugs.python.org fields:

    activity = <Date 2015-05-16.18:06:45.845>
    actor = 'r.david.murray'
    assignee = 'none'
    closed = True
    closed_date = <Date 2015-05-16.18:06:45.846>
    closer = 'r.david.murray'
    components = ['email']
    creation = <Date 2014-06-17.23:28:22.752>
    creator = 'zvyn'
    dependencies = []
    files = ['35808', '39284', '39324']
    hgrepos = []
    issue_num = 21800
    keywords = ['patch']
    message_count = 10.0
    messages = ['222000', '236713', '242538', '242620', '242622', '242648', '242662', '242778', '242873', '242876']
    nosy_count = 10.0
    nosy_names = ['barry', 'pitrou', 'eric.smith', 'r.david.murray', 'jesstess', 'BreamoreBoy', 'python-dev', 'maciej.szulik', 'zvyn', 'H\xc3\xa5kan L\xc3\xb6vdahl']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue21800'
    versions = ['Python 3.5']

    @zvyn zvyn mannequin added the expert-email label Jun 17, 2014
    @jimjjewett jimjjewett mannequin added the type-feature A feature request or enhancement label Jun 23, 2014
    @zvyn
    Copy link
    Mannequin Author

    zvyn mannequin commented Jul 1, 2014

    I made a patch implementing the following changes to the IMAP4 class:

    • add a method 'enable_UTF8_accept()' sending "ENABLE UTF8=ACCEPT" to the server and setting internal encoding to UTF-8
    • use the UTF8 extencion in the 'append()' method if the internal encoding is UTF-8
    • add a keyword argument 'enable_UTF8=False' to the init method to trigger 'enable_UTF8_accept()' as soon as the authentication is done
    • always use UTF-8 for encoding credentials in authentication (before encoding it to base64)

    Does this look like a good idea to you? (I'll make a patch including docs when we agree on the API.)

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Feb 26, 2015

    The patch contains changes to code and tests have been added so can we have a formal review please.

    @bitdancer
    Copy link
    Member

    bitdancer commented May 4, 2015

    Here is an updated patch based on Milan's work, including docs. I've tweaked the API slightly: no dedicated method for doing the enable (instead it is inlined in authenticate), I added 'enable' to the exposed API (with a doc caveat about not using it for UTF8=ACCEPT), added None as a valid value for enable_UTF8 with the meaning "enable if possible" (True is now "must enable"), and exposed a utf8_enabled attribute so the program can easily tell what mode to use when specifying enable_UTF8=None. Someday, None should become the default. "None" is not the best choice for value, especially when it is not the default, so perhaps someone could suggest better values for that keyword.

    It would be great if Milan or Maciej could give me a review (or anyone else who feels like it). I want to get this in before the beta deadline.

    @soltysh
    Copy link

    soltysh commented May 5, 2015

    David I did the review and there's one thing that worries me the most, actually two:

    1. changing the usual meaning of None in the IMAP's __init__ method, where None has the same meaning as True, where I think it should be the opposite.
    2. I'm not sure we want to have UTF8 enabled based on the init's flag. I've seen our IMAP library as a wrapper around protocol itself. Whereas the user must be aware of required steps needed to proceed. In this case enabling UTF8 support is just the next command the client can, but doesn't have to sent directly, but only in AUTH state.

    @bitdancer
    Copy link
    Member

    bitdancer commented May 5, 2015

    Well, the problem with that is that we then have to parse the capability to see if it is utf8 that is being enabled. I don't like that as an API, it feels fragile. Since capabilities cannot later be disabled, there's no functional reason to keep it separate. However, it would solve the problem of values to use in the init flag, and would remove the caveat that you shouldn't use UTF8=ENABLE in an explicit enable call, so perhaps it is best after all.

    Do you have any interest in updating the patch? I won't be able to get back to it until this weekend.

    @soltysh
    Copy link

    soltysh commented May 6, 2015

    Yes, I can update that (that IMAP testing bug - http://bugs.python.org/issue22137, is taking me longer than I expected it ;)).
    I just want to make sure if I understand you correctly what's needs to be done is removing the utf8_enable code from init, we will enable ascii by default and only explicit call to enable method will enable it, am I missing something?

    @bitdancer
    Copy link
    Member

    bitdancer commented May 6, 2015

    An explicit call to enable with an argument string that contains 'UTF8=ACCEPT'. I did not go far enough through the BNF to determine if enable can be passed more than one capability at a time, but I suspect it can. In which case we should factor out the capability parsing such that we can reuse it for parsing the enable argument.

    @soltysh
    Copy link

    soltysh commented May 8, 2015

    David, I've changed according to your suggestion, appreciate review.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 10, 2015

    New changeset 195343b5e64f by R David Murray in branch 'default':
    bpo-21800: Add RFC 6855 support to imaplib.
    https://hg.python.org/cpython/rev/195343b5e64f

    @bitdancer
    Copy link
    Member

    bitdancer commented May 11, 2015

    Thanks, Maciek (and Milan).

    I tweaked your patch slightly (mostly doc changes...I moved the discussion of the utf8 RFC into the enable method only, and added back the docs for utf8_enabled). I made some review comments about the changes other than that doc reorg that I made before commit, just FYI.

    @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
    expert-email type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants