On Saturday 03 September 2016 18:24:56 Ricardo Signes wrote: > > Look at my proposal just for first version and lets change parts which > > are not OK for you. I do not believe that everything is totally wrong. > > Okay! > > First, I think we should just leave Email::Simple alone. In general, I think > the cases for using Email::Simple are very few, and almost nobody should ever > use it. Giving it new and ostensibly MIME-related features seems unnecessary. > Having said that, I'm not going to look at the Email::Simple changes in depth. > (We definitely don't want to make installing Email::Simple require loading > Email::Address::List::XS, I'll note.) > > I think that ->format is probably not a great name choice, as it might exist > other places too easily. For example, Email::Address has a ->format, but I > don't think it will be suitable for this, as it doesn't encode properly. This
For Email::Simple it is correct. And for _raw functions in Email::MIME too. ->format from Email::Address (and also Email::Address::XS as it is replacement) correctly generate that header. But it expect that input fields are ASCII, so MIME encoding must be done by caller, who are creating that /Email::Address(::XS)?/ object. > is why I originally suggested something almost guaranteed not to clash, like > ->as_mime_header. We can assume that programmers won't have to call this very > often, only the innards of Email::MIME, so it's okay if it's a bit wordy. I do not want to add ->as_mime_header (or other function) which will do MIME encoding/decoding into Email::Address::XS. That module is for formating and parsing email addresses headers, not for MIME encoding/decoding. Same as it is for Email::Address module. And because whole MIME encoding/decoding is done in Email::MIME, I think that encoding/decoding of Email::Address should be done in Email::MIME too. Maybe code can be moved to some submodule e.g. Email::MIME::<Etc>, but still part of Email-MIME distribution. > The Email::MIME changes look like they could be broken up into several PRs, > some of which would be obviously good to apply immediately, like removals of > dead code and pointers to bad modules. If you think that some of those changes can be merged immediately, please specify commits and I create new pull request for them. Btw, I'm preparing another big patch series for Encode::MIME::Header module (call encode("MIEM-Header", ...)) which will fix remaining bugs. So if you know about some in that, let me know ASAP, so I can fix it in my patch series ;-) ..Which means that removing pointer to that module will not be needed.. > Primarily, I don't like the special weight given to the addrlist header. > While > it's likely to be the most common one, I think that implementing it as a > special case rather than an application of the general case, is going to lead > to problems. (Just yesterday I spent much of the day on DKIM, and it was > clear > that Authentication-Results and Domain-Signature could both usefully have > special objects.) Ok. > > [...] > > So easy extensible API needs to have one method which do that. Now I > > have only idea with something like this: > > > > my $addrlist = $email->header_to_obj("Cc", "Email::Address::List::XS"); > > > > That will convert header "Cc" to object Email::Address::List::XS and > > MIME decode parts which needs to be decoded. > > > > (Maybe class name could be optional and some mapping table for most > > common headers could be prepared) > > I think this is all plausible. The parts that are important to me are: > > * objects working for all headers equally well That just mean to create classes for needed headers. > * a registry of common field-name-to-class-name mappings Problem is what is "common". From RFC5322? From some subset of RFC5322? >From later RFCs which update RFC5322? Or all RFCs which define some structured header? > > That method still needs to be know how to MIME decode object > > Email::Address::List::XS... > > I'm not sure what you mean, here. Do you mean that if we've stored a header > entry as an object that has an as-mime-encoded-string method, we also end up > needed a means to get it as-decoded-string? I'm afraid I just don't > understand > the sentence. Ok, I will try to explain it differently. You already pointed to module Email::Address. That module represent structure of From header (contains one email address) as specified in RFC5322 (or 2822 or 822). But it does not do any MIME encoding/decoding. So if somebody fill Unicode strings in Email::Address module and you want to include that header (which is represented by Email::Address object), you first need to call MIME encoder/decoder on ->phrase and ->comment members of that object. If you create new class which will represent other structured header defined in RFC5322, then it again does will not deal with MIME encoding and decoding. But it will have different members (not ->phrase and ->comment) which will be needed to encode/decode. So Email::MIME is not able to automatically encode/decode arbitrary object which represent RFC5322 email header. Email::MIME needs to know which members of objects must be encoded/decoded. > Your changes to Email::Simple don't store objects, but produce them on demand. Yes. > I'm thinking of: > https://github.com/rjbs/Email-Simple/compare/master...pali:master#diff-8816e211b9069c6bfa4cc4c82b7410b3R224 > > If we never *store* objects, but only produce them as requested, then I think > the total needed changes are -- but I'm sure I'll miss things -- as follows: > > * allow header_str and header args to Email::MIME->create to include objects, > which are immediately asked to encode themselves for storage Yes. > * add header_as_obj that takes a header name and, optionally, a class name and > offset (an offset so you can ask for an object of the nth Received header) When you have e.g. M headers in email with same name and you want to get just Nth. So offset represent this N? > * a registry used by header_as_obj to get a default class name from header > name Yes. > The downside is that if you call ->header_str(...) for something structured > and > there's no registered class for that field name, you get a less than stellar > answer. You will get string output which could be garbage, not useful for later processing, etc... E.g. when phrase part of email address contains MIME encoded string 'u...@host.tld, '. Calling ->header_str will show you another email address, which is not correct... > Really, header_str becomes an odd method, because many headers aren't > meant to exist as unencoded single strings, but are structures. Still, we can > fake it as needed, advising people to consider using object forms directly. For email address: you can correctly write phrase which contains 'u...@host.tld, ' even without MIME. You can need properly escape some characters... Email::Address should do it (but do not know if correctly) and Email::Address::XS is doing it correctly (tested)! > I think the way that header_addrlist and header_addrlist_raw behave is > confusing. Rather than have two object representations, it seems that the > data > should be represented as the same object, either way, and then data within it > should be asked for in encoded or decoded format. That is: the header's raw > content is data that classes exist to interpret, access, and produce. If we > generalized this to header_valueobj and header_valueobj_raw, I think it would > be quite bizarre. I would really like to see option to pass "already encoded" object to Email::MIME. Same as header_raw for "already encoded" string value.