update imap & smtp URI for ease of maintenance #5111
Conversation
This change cleans up the imapURI a bit. If we call the current state "legacy", then legacy looks a bit like this: legacy: imap[+(ssl|tls)+]://auth:user:(password|certAlias)@host[:port]/[(0|1)\|][prefix] If we were to imagine a cleaned up version that didn't totally break the current scheme, we'd move most options to query parameters, which are named (hence ordering doesn't matter) and it is easier to mantain forwards and backwards compatability. This also "split" tls-cert from auth external, so they can be set independenty in the future. It also merged "autoPrefix" with "prefix" so they cannot be set independently. cleanish: imap[+(ssl|tls)+]://user[:password]@host[:port]/?prefix=(auto|\/INBOX)&auth-type=(external|plain|oauth2|)&tls-cert=certAlias What we can do, and be 100% backwards compatable today, is encode the values both as query parameters and in the "legacy" form. This is called hybrid, and it doesn't break old decoders since they ignore query, and all the information in query is redundant. It looks like this: hybrid: imap[+(ssl|tls)+]://[auth:]user[:(password|certAlias)]@host[:port]/[(0|1)\|][prefix]?prefix=(auto|\/prefix)&auth-type=(external|plain|xoauth2|)&tls-cert=certAlias What I have implemented in this patch is encoding the hybrid uri format. The uri decoder has been updated to read in legacy, hybrid, and cleanish formats.
|
I created this pull request based off of the comment in this old pull request. |
…as to query params this cleanup allows for more flexible and less brittle configuration of settings. This supports a clean(-ish) uri syntax, the current syntax, and a hybrid combination. Here is a summation of those syntax. legacy: smtp[+(tls|ssl)+]://user:(password|certAlias):auth@server[:port] hybrid: smtp[+(tls|ssl)+]://user:(password|certAlias):auth@server[:port]?auth-type=(plain|external)&tls-cert=certAlias cleanish: smtp[+(tls|ssl)+]://user[:password]@server[:port]?auth-type=(plain|external|cram_md5)&tls-cert=certAlias
toppk
changed the title
|
second commit does the same for smtp transport uri. |
This is only needed when query contain user input and may contain reserved chars.
- smtp may not have auth - smtp may not have auth or tls (no query params) - tidy up no username/password/auth encoding test cases added for these issues
|
Thanks for taking a stab at this. Judging by the number of updates to this pull request, you have now experienced first-hand that URIs are not a great serialization format for K-9 Mail's server settings 😄 If we wanted to keep URIs as the serialization format for server settings your new format would be an improvement 👍 However, I want to get rid of the URIs altogether. Many parts of the app already use Closing this issue because we're going with a different approach. |
|
Are you also planning on writing the new encoder/decoder and migration code? If not I'd be happy to help. |
This change cleans up the imapURI a bit. If we call the current state "legacy", then legacy looks a bit like this:
legacy: imap[+(ssl|tls)+]://auth:user:(password|certAlias)@host[:port]/[(0|1)|][prefix]
If we were to imagine a cleaned up version that didn't totally break the
current scheme, we'd move most options to query parameters, which are
named (hence ordering doesn't matter) and it is easier to mantain
forwards and backwards compatability.
This also "split" tls-cert from auth external, so they can be set
independenty in the future. It also merged "autoPrefix" with "prefix"
so they cannot be set independently.
cleanish: imap[+(ssl|tls)+]://user[:password]@host[:port]/?prefix=(auto|/INBOX)&auth-type=(external|plain|oauth2|)&tls-cert=certAlias
What we can do, and be 100% backwards compatable today, is encode the
values both as query parameters and in the "legacy" form. This is
called hybrid, and it doesn't break old decoders since they ignore
query, and all the information in query is redundant. It looks like
this:
hybrid: imap[+(ssl|tls)+]://[auth:]user[:(password|certAlias)]@host[:port]/[(0|1)|][prefix]?prefix=(auto|/prefix)&auth-type=(external|plain|xoauth2|)&tls-cert=certAlias
What I have implemented in this patch is encoding the hybrid uri format.
The uri decoder has been updated to read in legacy, hybrid, and cleanish formats.
If you like this approach, I will update smtp (and pop3, if that's still sticking around) along the same lines, as well as adding a few more test cases. Then I will address any other feedback about the code quality. java is not my native tongue.