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

Fixes IMAP UTF8 Authenication #725

Merged
merged 3 commits into from
Apr 29, 2022
Merged

Fixes IMAP UTF8 Authenication #725

merged 3 commits into from
Apr 29, 2022

Conversation

stumpylog
Copy link
Member

Proposed change

In the event of a UnicodeEncodeError during normal login, we'll now try to fallback to an AUTH=PLAIN login type, which encodes the username and password differently.

I successfully tested this with a password containing UTF8 characters and was able to view the inbox.

Fixes #663

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other (please explain)

Checklist:

  • I have read & agree with the contributing guidelines.
  • If applicable, I have tested my code for new features & regressions on both mobile & desktop devices, using the latest version of major browsers.
  • If applicable, I have checked that all tests pass, see documentation.
  • I have run all pre-commit hooks, see documentation.
  • I have made corresponding changes to the documentation as needed.
  • I have checked my modifications for any breaking changes.

@stumpylog stumpylog requested a review from a team as a code owner April 13, 2022 20:39
@paperless-ngx-secretary paperless-ngx-secretary bot added backend non-trivial Requires approval by several team members labels Apr 13, 2022
@stumpylog
Copy link
Member Author

If we can sneak this into the beta, it's probably good to.

@shamoon
Copy link
Member

shamoon commented Apr 13, 2022

Thanks as always for another deep-dive fix! In the case of the new plain auth attempt is the error output still clear? Im just wondering if it would be helpful to include something like "non-ascii chars detected, attempting to fallback to plain auth" to make it more apparent why this all failed?

@stumpylog
Copy link
Member Author

Well, the error before was pretty generic, so I kept that, but I could see having an extra log or two would help in debugging

@stumpylog stumpylog force-pushed the bugfix-imap-utf8-login branch 2 times, most recently from 5a0c495 to 62f70f4 Compare April 25, 2022 19:06
@coveralls
Copy link

Pull Request Test Coverage Report for Build 2222357777

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 6 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.04%) to 91.985%

Files with Coverage Reduction New Missed Lines %
paperless_mail/mail.py 6 96.86%
Totals Coverage Status
Change from base Build 2222108065: 0.04%
Covered Lines: 4315
Relevant Lines: 4691

💛 - Coveralls

@shamoon shamoon added this to the v1.7.1 milestone Apr 29, 2022
Copy link
Member

@qcasey qcasey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks ok to me.

@stumpylog stumpylog added the bug Bug report or a Bug-fix label Apr 29, 2022
@stumpylog
Copy link
Member Author

This is a pretty minor change to me, do we want the pending reviewer still or just merge?

@qcasey
Copy link
Member

qcasey commented Apr 29, 2022

Agreed, let's just merge. This happens a lot, we might have to revisit that rule later on

@github-actions
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backend bug Bug report or a Bug-fix non-trivial Requires approval by several team members
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[BUG] Email processing fails because of non-ascii character in email account password
4 participants