[Rt-commit] rt branch, 4.4/txn-content-skip-mime-attachments, created. rt-4.4.4-34-g2505933f07
? sunnavy
sunnavy at bestpractical.com
Fri Aug 14 15:13:37 EDT 2020
The branch, 4.4/txn-content-skip-mime-attachments has been created
at 2505933f070b68f0bc59feb4e060f59c05359eb6 (commit)
- Log -----------------------------------------------------------------
commit 5dc91ca1477e4d8e10ab2406adb80835deff4dd7
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Jun 26 03:30:30 2018 +0800
Exclude MIME attachments when questing a transaction's content
It doesn't make sense to use MIME attachments' content here, what we
really want is the main body of the MIME.
diff --git a/lib/RT/Attachment.pm b/lib/RT/Attachment.pm
index 66a0e4e6d1..45bd479073 100644
--- a/lib/RT/Attachment.pm
+++ b/lib/RT/Attachment.pm
@@ -561,6 +561,18 @@ sub IsMessageContentType {
return $self->ContentType =~ m{^\s*message/}i ? 1 : 0;
}
+=head2 IsAttachmentContentDisposition
+
+Returns a boolean indicating if the Content-Disposition of this attachment
+is a MIME attachment.
+
+=cut
+
+sub IsAttachmentContentDisposition {
+ my $self = shift;
+ return ( $self->GetHeader('Content-Disposition') // '' ) =~ m{^\s*attachment\b}i ? 1 : 0;
+}
+
=head2 Addresses
Returns a hashref of all addresses related to this attachment.
diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index b027ef6648..f60e9e548d 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -341,6 +341,9 @@ part. If $args{'Type'} is missing, it defaults to the value of
C<$RT::Transaction::PreferredContentType>, if that's missing too,
defaults to textual.
+All the MIME attachments are excluded here, because it doesn't make much sense
+to use them as transaction content.
+
=cut
sub Content {
@@ -556,8 +559,10 @@ sub ContentObj {
# displayable". For now, this maintains backcompat
my $all_parts = $self->Attachments;
while ( my $part = $all_parts->Next ) {
- next unless _IsDisplayableTextualContentType($part->ContentType)
- && $part->Content;
+ next unless _IsDisplayableTextualContentType( $part->ContentType )
+ && !$part->Filename
+ && !$part->IsAttachmentContentDisposition
+ && $part->Content;
return $part;
}
@@ -572,6 +577,10 @@ sub _FindPreferredContentObj {
# If we don't have any content, return undef now.
return undef unless $Attachment;
+ # If it's a MIME attachment, return undef.
+ return undef if $Attachment->Filename;
+ return undef if $Attachment->IsAttachmentContentDisposition;
+
# If it's a textual part, just return the body.
if ( _IsDisplayableTextualContentType($Attachment->ContentType) ) {
return ($Attachment);
commit 2505933f070b68f0bc59feb4e060f59c05359eb6
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Jun 26 03:35:11 2018 +0800
Add tests for transaction content
diff --git a/t/api/transaction.t b/t/api/transaction.t
index 22c3cfe72f..149b7bada3 100644
--- a/t/api/transaction.t
+++ b/t/api/transaction.t
@@ -47,6 +47,108 @@ use_ok ('RT::Transaction');
"Caught URI warning";
is( $brief, 'Reference to ticket 42 deleted', "Got string description: $brief");
+
+}
+
+diag 'Test Content';
+{
+ require MIME::Entity;
+
+ my $plain_file = File::Spec->catfile( RT::Test->temp_directory, 'attachment.txt' );
+ open my $plain_fh, '>', $plain_file or die $!;
+ print $plain_fh 'this is attachment';
+ close $plain_fh;
+
+ my @mime;
+
+ my $mime = MIME::Entity->build( Data => [ 'main body' ] );
+ push @mime, { object => $mime, expected => 'main body', description => 'no attachment' };
+
+ $mime = MIME::Entity->build( Type => 'multipart/mixed' );
+ $mime->attach(
+ Type => 'text/plain',
+ Data => [ 'main body' ],
+ );
+ $mime->attach(
+ Path => $plain_file,
+ Type => 'text/plain',
+ );
+ push @mime, { object => $mime, expected => 'main body', description => 'has an attachment' };
+
+ $mime = MIME::Entity->build( Type => 'multipart/mixed' );
+ $mime->attach(
+ Path => $plain_file,
+ Type => 'text/plain',
+ );
+ $mime->attach(
+ Type => 'text/plain',
+ Data => [ 'main body' ],
+ );
+ push @mime, { object => $mime, expected => 'main body', description => 'has an attachment as the first part' };
+
+ $mime = MIME::Entity->build( Type => 'multipart/mixed' );
+ $mime->attach(
+ Path => $plain_file,
+ Type => 'text/plain',
+ );
+ push @mime,
+ { object => $mime, expected => 'This transaction appears to have no content', description => 'has an attachment but no main part' };
+
+ my $parser = MIME::Parser->new();
+ $parser->output_to_core(1);
+ $mime = $parser->parse_data( <<EOF );
+Content-Type: multipart/mixed; boundary="=-=-="
+
+--=-=-=
+Content-Type: message/rfc822
+Content-Disposition: inline
+
+Content-Type: text/plain
+Subject: test
+
+main body
+--=-=-=
+EOF
+
+ push @mime, { object => $mime, expected => "main body", description => 'has an rfc822 message' };
+
+ $mime = $parser->parse_data( <<EOF );
+Content-Type: multipart/mixed; boundary="=-=-="
+
+--=-=-=
+Content-Type: message/rfc822
+Content-Disposition: attachment
+
+Content-Type: text/plain
+Subject: test
+
+inner body of rfc822
+
+--=-=-=
+Content-Type: text/plain
+Subject: test
+
+main body
+--=-=-=
+
+EOF
+
+ push @mime,
+ { object => $mime, expected => 'main body', description => 'has an attachment of rfc822 message and main part' };
+
+ for my $mime ( @mime ) {
+ my $ticket = RT::Ticket->new( RT->SystemUser );
+ my ( $id, $txn_id ) = $ticket->Create(
+ Queue => 'General',
+ Subject => 'Testing content',
+ MIMEObj => $mime->{object},
+ );
+ ok( $id, 'Created ticket' );
+ ok( $txn_id, 'Created transaction' );
+ my $txn = RT::Transaction->new( RT->SystemUser );
+ $txn->Load( $txn_id );
+ is( $txn->Content, $mime->{expected}, "Got expected content for MIME: $mime->{description}" );
+ }
}
done_testing;
diff --git a/t/web/ticket_txn_content.t b/t/web/ticket_txn_content.t
index 096d78e314..773f3afe87 100644
--- a/t/web/ticket_txn_content.t
+++ b/t/web/ticket_txn_content.t
@@ -1,7 +1,7 @@
use strict;
use warnings;
-use RT::Test tests => 63;
+use RT::Test;
my $plain_file = File::Spec->catfile( RT::Test->temp_directory, 'attachment.txt' );
open my $plain_fh, '>', $plain_file or die $!;
print $plain_fh "this is plain content";
@@ -112,3 +112,42 @@ for my $type ( 'text/plain', 'text/html' ) {
$m->content_contains("this is main reply content", 'email contains main reply content');
$m->back;
}
+
+$m->goto_create_ticket( $qid );
+$m->submit_form_ok(
+ {
+ form_name => 'TicketCreate',
+ fields => {
+ Subject => 'with main body',
+ Content => 'this is main body',
+ Attach => $plain_file,
+ }
+ },
+ 'submit TicketCreate form'
+);
+$m->text_like( qr/Ticket \d+ created in queue/, 'ticket is created' );
+$m->content_contains( "Download $plain_name", 'download plain file link' );
+$m->follow_link_ok( { text => 'Reply', url_regex => qr/QuoteTransaction=/ }, 'reply the create transaction' );
+my $form = $m->form_name( 'TicketUpdate' );
+my $content = $form->find_input( 'UpdateContent' );
+like( $content->value, qr/this is main body/, 'has transaction content' );
+
+$m->goto_create_ticket( $qid );
+$m->submit_form_ok(
+ {
+ form_name => 'TicketCreate',
+ fields => {
+ Subject => 'without main body',
+ Attach => $plain_file,
+ }
+ },
+ 'submit TicketCreate form'
+);
+$m->text_like( qr/Ticket \d+ created in queue/, 'ticket is created' );
+$m->content_contains( "Download $plain_name", 'download plain file link' );
+$m->follow_link_ok( { text => 'Reply', url_regex => qr/QuoteTransaction=/ }, 'reply the create transaction' );
+$form = $m->form_name( 'TicketUpdate' );
+$content = $form->find_input( 'UpdateContent' );
+like( $content->value, qr/This transaction appears to have no content/, 'no transaction content' );
+
+done_testing;
-----------------------------------------------------------------------
More information about the rt-commit
mailing list