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.

Sign up for GitHub

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

Jump to bottom

Add support for object representation of the header value #35

Closed
wants to merge 3 commits into from
Closed

Add support for object representation of the header value #35

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
94c0aaf
Email::MIME: Allow to pass objects with as_mime_string() method into …
pali Jan 14, 2017
485ab2b
Email::MIME: Add new method header_as_obj() which returns an object r…
pali Jan 14, 2017
bae21c2
Email::MIME::Header: Add function set_class_for_header
pali Feb 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/Email/MIME.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,11 @@ sub new {

This method creates a new MIME part. The C<header_str> parameter is a list of
headers pairs to include in the message. The value for each pair is expected to
be a text string that will be MIME-encoded as needed. A similar C<header>
be a text string that will be MIME-encoded as needed. Alternatively it can be
an object with C<as_mime_string> method which implements conversion of that
object to MIME-encoded string. That object method is called with two input
parameters: charset and length of header name and should return MIME-encoded
representation of the object. A similar C<header>
parameter can be provided in addition to or instead of C<header_str>. Its
values will be used verbatim.

Expand Down Expand Up @@ -875,6 +879,9 @@ This behaves like C<header_raw_set>, but expects Unicode (character) strings as
the values to set, rather than pre-encoded byte strings. It will encode them
as MIME encoded-words if they contain any control or 8-bit characters.

Alternatively, values can be objects with C<as_mime_string> method. Same as in
method C<create>.

=method header_str_pairs

my @pairs = $email->header_str_pairs;
Expand Down
5 changes: 5 additions & 0 deletions lib/Email/MIME/Encode.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package Email::MIME::Encode;
use Email::Address;
use Encode ();
use MIME::Base64();
use Scalar::Util;

my %address_list_headers = map { $_ => undef } qw(from sender reply-to to cc bcc);
my %no_mime_headers = map { $_ => undef } qw(date message-id in-reply-to references downgraded-message-id downgraded-in-reply-to downgraded-references);
Expand All @@ -16,6 +17,10 @@ sub maybe_mime_encode_header {
$header = lc $header;

my $header_length = length($header) + length(": ");

return $val->as_mime_string($charset, $header_length)
Copy link
Owner

Choose a reason for hiding this comment

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

A look suggests that nothing should be passing anything to maybe_mime_encode_header other than the value UTF-8. If this is the case, I think we should eliminate the argument. Header objects that know how to encode their strings should be free to pick the appropriate encoding (which imho should either be ASCII or UTF-8 and nothing else), and shouldn't have to deal with someone asking for emoji to be encoded as KOI-8.

Any reason to keep this argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still see email clients or email services which do not support UTF-8, only ISO-8859-1. IIRC RFC 2047 does not require implementation to really support UTF-8, but require some ISO-8859-1 encoding.

As we know that MIME encoder and decoder in core perl (via Encode package) was terrible broken for a long time I would rather have needed functions for other encodings available. Just in case somebody needs to deal with other encoding as UTF-8 (e.g. that ISO-8859-1).

Copy link
Owner

Choose a reason for hiding this comment

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

Okay, I'm sold. Let's reverse the argument order so that it's easier to give the header length and let the encoder pick an encoding, if the client doesn't care?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably $header and $header_length are misleading here. $header is field name (according to RFC2822) and $header_length length of field name + 2 (for colon and space).

Purpose of passing $header_length into encoder is ability to know how many octets are already print on first line before field body. Encoder can use this information and optimize whole header to fil into less lines. But it is just optional... Email::Simple can wrap correctly field body produced by encoder...

Parameter $header_length is optional for encoder, it does not have to use it. So I put it as second argument.

On the other hand, if $charset is passed, then encoder should use it and encode field body into that charset. Therefore I think $charset is more "required" as $header_length so I put $charset before $header_length.

If you have other opinion or better idea for naming variables just propose it...

Copy link
Owner

Choose a reason for hiding this comment

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

Alright. I'm not 💯 on either ordering, so I'll go with what you have and hope that nobody uses either one of those arguments. :-)

Copy link
Owner

Choose a reason for hiding this comment

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

I've changed the code to use named arguments and avoid the whole question of order.

if Scalar::Util::blessed($val) && $val->can("as_mime_string");

my $min_wrap_length = 78 - $header_length + 1;

return $val
Expand Down