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

Build method for content type #2

Closed
moznion opened this issue Nov 16, 2013 · 8 comments · Fixed by #11
Closed

Build method for content type #2

moznion opened this issue Nov 16, 2013 · 8 comments · Fixed by #11

Comments

@moznion
Copy link

moznion commented Nov 16, 2013

Hello,

When this module had construction method for Content-Type which takes result of parse_content_type() as the argument, I thought that it was convenient.

Like so (Since it wrote promptly, there may be a point that something is not good);
https://gist.github.com/moznion/7496660

Since this processing can be written easily, a user may actually be what to write individually. However, I thought that the way which offered unific processing was good because it doesn't make trifling difference (e.g. treatment of white space, etc...) will not be induced.

@rjbs
Copy link
Owner

rjbs commented Jan 29, 2014

I think this is a good idea, but am not adding it just as provided, yet. There are (awful) rules about forming content type strings, and I'd like to take some time to get it right. I'll leave this ticket open, though.

Please don't delete the gist, in case I give up on RFC compliance. ;)

Thanks!

@rjbs
Copy link
Owner

rjbs commented Aug 8, 2014

I'm just bringing the gist content in here. "please don't delete the gist" is silly.

sub build_content_type {
  my ($self, $content_type) = @_;

  if (!$content_type->{discrete} || !$content_type->{composite}) {
    die "! Invalid Content-Type";
  }
  my $content_type_str = "$content_type->{discrete}/$content_type->{composite}";
  for my $k (keys %{$content_type->{attributes}}) {
    my $v = $content_type->{attributes}->{$k};
    next unless $v;
    $content_type_str .= ";$k=$v";
  }
  return $content_type_str;
}

@rjbs
Copy link
Owner

rjbs commented Aug 8, 2014

Notes for the future:

  • Email::MIME::ContentType is not a class, so we don't expect $self
  • we want this to round trip, except it probably won't, because of encoding, unless we make the build dumb
  • …therefore, maybe we want a better parse_content_type that decodes
    • we should expect type and subtype not discrete and composite
  • is there even an easy-to-use library for the weird-o encoding used for multi-part CT parameters?

Should be fun.

@pali
Copy link
Contributor

pali commented Apr 28, 2017

Another problems:

  • does not handle space and tspecials characters
  • does not handle non-ASCII strings
  • does not handle long values

pali added a commit to pali/Email-MIME-ContentType that referenced this issue Sep 4, 2017
These functions compose Content-Type and Content-Disposition headers.

Non-ASCII attributes are encoded to UTF-8 and too long (> 78 bytes)
attributes are split, both according to RFC 2231.

For compatibility reasons, ASCII version of too long or non-ASCII attribute
is also present, but is truncated and encoded to ASCII via Text::Unidecode
module.

Fixes rjbs#2
@pali
Copy link
Contributor

pali commented Sep 4, 2017

I tried to implement function build_content_type correctly according to RFC 2045 and RFC 2231. Code is in pull request #11. So please review & test.

@pali
Copy link
Contributor

pali commented Aug 31, 2018

Can somebody review or test my code in pull request? Now it is year since I opened that pull request. Is still somebody interested in it?

@dxdc
Copy link

dxdc commented Aug 31, 2018

second this! thanks @pali

@rjbs rjbs closed this as completed in #11 May 9, 2020
rjbs pushed a commit that referenced this issue May 9, 2020
These functions compose Content-Type and Content-Disposition headers.

Non-ASCII attributes are encoded to UTF-8 and too long (> 78 bytes)
attributes are split, both according to RFC 2231.

For compatibility reasons, ASCII version of too long or non-ASCII attribute
is also present, but is truncated and encoded to ASCII via Text::Unidecode
module.

Fixes #2
@moznion
Copy link
Author

moznion commented May 11, 2020

Thank you so much for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants