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 Content-Location attachment header support #2065
base: master
Are you sure you want to change the base?
Conversation
Reference: RFC2110. Reading the RFC, it strikes me that adding support for Content-Location without also having support for Content-Base (at both message and part levels) isn't especially useful. Do you have examples of clients that actually support this in some useful way? I would fully expect clients like gmail to completely ignore this. |
09dad49
to
0e725bb
Compare
I could certainly try to add Content-Base support as an optional additional parameter if you'd prefer that. I don't think Content-Base support here obligates Content-Base support elsewhere in the MIME structure (i.e. for HTML), as the RFC pretty clearly states that Content-Base only applies to the particular part it is attached to, not recursively: These two headers are valid only for exactly the content heading or That said, Content-Base isn't required for absolute URLs, which is why I didn't do it in this version of the PR. The use case here is being able to have an MTA pre-retrieve linked remote images and the like and "caching" it by adding it as an inline attachment to the message, without modification of the body which would invalidate DKIM or other signatures or losing the URL information. This would be a nice privacy and convenience feature which we (ProtonMail) would like to build. Doing it via Content-Location would allow us to do it in a standards-compliant way which would have a good shot at not breaking on forwarding (worst case, it's just an extra attachment ignored by the client does a remote lookup anyway). Anyway, the last step would be supporting forwarding such emails outside our system with Content-Location, hence this PR. I can add Content-Base support for embedded images for completeness if you'd like. |
In the previous message I basically talked myself into the necessity of adding optional Content-Base header support for feature completeness. Given the current codebase I'd just add it via an additional optional parameter to the methods for adding embedded images. Let me know if this approach sounds good and I'll update the PR. |
Sorry for sitting on this! I've just been revisiting this PR a look and think it's a good idea - would you care to add the content-base support? Maybe add some tests to go with it? |
Hi @Synchro, will do, this dropped off my radar as well (inter-continental move tends to push other things down in priority). I'll try to get to it soon. |
Fix exception thrown for connection issue
The Content-Location header is also sometimes used to embed content, in particular when we want to maintain the link to that content's original URL. This PR supports Content-Location as an alternative to Content-ID for embedded images while maintaining complete backwards compatibility.