[Rt-commit] rt branch, 4.2/record-attachments-dropping, created. rt-4.2.5-166-ga3aaa71

? sunnavy sunnavy at bestpractical.com
Tue Jul 29 09:28:54 EDT 2014


The branch, 4.2/record-attachments-dropping has been created
        at  a3aaa712f8905cd4620f6b7c5b9183e4a3c90dc8 (commit)

- Log -----------------------------------------------------------------
commit c610009e4e36b19df3ce85ef03646ca390b383d1
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Mon Jul 14 20:28:00 2014 +0800

    record attachments' dropping/truncation as txns

diff --git a/lib/RT/Attachment.pm b/lib/RT/Attachment.pm
index c4f479c..4c5322f 100644
--- a/lib/RT/Attachment.pm
+++ b/lib/RT/Attachment.pm
@@ -198,12 +198,9 @@ sub Create {
     #If it's not multipart
     else {
 
-        my ($encoding, $type);
-        ($encoding, $content, $type, $Filename) = $self->_EncodeLOB(
-            $Attachment->bodyhandle->as_string,
-            $Attachment->mime_type,
-            $Filename
-        );
+        my ( $encoding, $type, $note_args );
+        ( $encoding, $content, $type, $Filename, $note_args ) =
+                $self->_EncodeLOB( $Attachment->bodyhandle->as_string, $Attachment->mime_type, $Filename, );
 
         my $id = $self->SUPER::Create(
             TransactionId   => $args{'TransactionId'},
@@ -217,7 +214,12 @@ sub Create {
             MessageId       => $MessageId,
         );
 
-        unless ($id) {
+        if ($id) {
+            if ($note_args) {
+                $self->TransactionObj->Object->_NewTransaction( %$note_args );
+            }
+        }
+        else {
             $RT::Logger->crit("Attachment insert failed: ". $RT::Handle->dbh->errstr);
         }
         return $id;
diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index c794d49..db2f7be 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -748,7 +748,7 @@ sub _Accessible  {
 =head2 _EncodeLOB BODY MIME_TYPE FILENAME
 
 Takes a potentially large attachment. Returns (ContentEncoding,
-EncodedBody, MimeType, Filename) based on system configuration and
+EncodedBody, MimeType, Filename, NoteArgs) based on system configuration and
 selected database.  Returns a custom (short) text/plain message if
 DropLongAttachments causes an attachment to not be stored.
 
@@ -760,6 +760,10 @@ encoded on databases which are strict.
 This function expects to receive an octet string in order to properly
 evaluate and encode it.  It will return an octet string.
 
+NoteArgs is currently used to indicate caller that the message is too long and
+is truncated or dropped. It's a hashref which is expected to be passed to
+L<RT::Record/_NewTransaction>.
+
 =cut
 
 sub _EncodeLOB {
@@ -769,6 +773,7 @@ sub _EncodeLOB {
     my $Filename = shift;
 
     my $ContentEncoding = 'none';
+    my $note_args;
 
     #get the max attachment length from RT
     my $MaxSize = RT->Config->Get('MaxAttachmentSize');
@@ -794,11 +799,21 @@ sub _EncodeLOB {
     #if the attachment is larger than the maximum size
     if ( ($MaxSize) and ( $MaxSize < length($Body) ) ) {
 
+        my $size = length $Body;
         # if we're supposed to truncate large attachments
         if (RT->Config->Get('TruncateLongAttachments')) {
 
+            $RT::Logger->info("$self: Truncated an attachment of size $size");
+
             # truncate the attachment to that length.
             $Body = substr( $Body, 0, $MaxSize );
+            $note_args = {
+                Type           => 'AttachmentTruncate',
+                Data           => defined $Filename ? $Filename : 'Content',
+                OldValue       => $size,
+                NewValue       => $MaxSize,
+                ActivateScrips => 0,
+            };
 
         }
 
@@ -806,11 +821,17 @@ sub _EncodeLOB {
         elsif (RT->Config->Get('DropLongAttachments')) {
 
             # drop the attachment on the floor
-            $RT::Logger->info( "$self: Dropped an attachment of size "
-                               . length($Body));
+            $RT::Logger->info( "$self: Dropped an attachment of size $size" );
             $RT::Logger->info( "It started: " . substr( $Body, 0, 60 ) );
-            $Filename .= ".txt" if $Filename;
-            return ("none", "Large attachment dropped", "text/plain", $Filename );
+            $note_args = {
+                Type           => 'AttachmentDrop',
+                Data           => defined $Filename ? $Filename : 'Content',
+                OldValue       => $size,
+                NewValue       => $MaxSize,
+                ActivateScrips => 0,
+            };
+            $Filename .= ".txt" if $Filename && $Filename !~ /\.txt$/;
+            return ("none", "Large attachment dropped", "text/plain", $Filename, $note_args );
         }
     }
 
@@ -827,7 +848,7 @@ sub _EncodeLOB {
     }
 
 
-    return ($ContentEncoding, $Body, $MIMEType, $Filename );
+    return ($ContentEncoding, $Body, $MIMEType, $Filename, $note_args );
 }
 
 =head2 _DecodeLOB C<ContentType>, C<ContentEncoding>, C<Content>
diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index 68c7cd2..fdc66a8 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -841,6 +841,16 @@ sub _FormatUser {
         my $self = shift;
         return ("System error"); #loc()
     },
+    AttachmentTruncate => sub {
+        my $self = shift;
+        return ( "[_1] truncated because its size ([_2] bytes) exceeded configured maximum size setting ([_3] bytes).",
+            $self->Data // loc( "Content" ), $self->OldValue, $self->NewValue ); #loc()
+    },
+    AttachmentDrop => sub {
+        my $self = shift;
+        return ( "[_1] dropped because its size ([_2] bytes) exceeded configured maximum size setting ([_3] bytes).",
+            $self->Data // loc( "Content" ), $self->OldValue, $self->NewValue ); #loc()
+    },
     "Forward Transaction" => sub {
         my $self = shift;
         my $recipients = join ", ", map {

commit 216dedf2277d39d456162ba7116ff9f112d4995e
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Sun Dec 8 10:27:06 2013 +0800

    test attachment dropping/truncation

diff --git a/t/web/attachment_dropping.t b/t/web/attachment_dropping.t
new file mode 100644
index 0000000..c3b3a63
--- /dev/null
+++ b/t/web/attachment_dropping.t
@@ -0,0 +1,51 @@
+use warnings;
+use strict;
+
+use RT::Test tests => undef;
+use File::Temp 'tempfile';
+
+my $content = 'a' x 1000 . 'b' x 10;
+my ( $fh, $path ) = tempfile( UNLINK => 1, SUFFIX => '.txt' );
+print $fh $content;
+close $fh;
+
+my $name = ( File::Spec->splitpath($path) )[2];
+
+RT->Config->Set( 'MaxAttachmentSize', 1000 );
+RT->Config->Set( 'TruncateLongAttachments', '0' );
+RT->Config->Set( 'DropLongAttachments',     '1' );
+
+my $cf = RT::CustomField->new( RT->SystemUser );
+ok(
+    $cf->Create(
+        Name  => 'test truncation',
+        Queue => '0',
+        Type  => 'FreeformSingle',
+    ),
+);
+my $cfid = $cf->id;
+
+my ( $baseurl, $m ) = RT::Test->started_ok;
+ok $m->login, 'logged in';
+
+my $queue = RT::Test->load_or_create_queue( Name => 'General' );
+ok( $queue->id, "Loaded General queue" );
+$m->get_ok( $baseurl . '/Ticket/Create.html?Queue=' . $queue->id );
+$m->content_contains( "Create a new ticket", 'ticket create page' );
+
+$m->form_name('TicketCreate');
+$m->field( 'Subject', 'Attachments dropping test' );
+$m->field( 'Attach',  $path );
+$m->field( 'Content', 'Some content' );
+my $cf_content = 'cf' . 'a' x 998 . 'cfb';
+$m->field( "Object-RT::Ticket--CustomField-$cfid-Value", $cf_content );
+$m->submit;
+is( $m->status, 200, "request successful" );
+
+$m->content_contains( "$name dropped because its size (1010 bytes) exceeded configured maximum size setting (1000 bytes).", 'dropped message' );
+$m->content_lacks( 'cfaaaa', 'cf value was dropped' );
+$m->follow_link_ok( { text => "Download $name" } );
+is( $m->content, 'Large attachment dropped', 'dropped $name' );
+
+undef $m;
+done_testing;
diff --git a/t/web/attachment_truncation.t b/t/web/attachment_truncation.t
new file mode 100644
index 0000000..eedc91a
--- /dev/null
+++ b/t/web/attachment_truncation.t
@@ -0,0 +1,52 @@
+use warnings;
+use strict;
+
+use RT::Test tests => undef;
+use File::Temp 'tempfile';
+
+my $content = 'a' x 1000 . 'b' x 10;
+my ( $fh, $path ) = tempfile( UNLINK => 1, SUFFIX => '.txt' );
+print $fh $content;
+close $fh;
+my $name = ( File::Spec->splitpath($path) )[2];
+
+RT->Config->Set( 'MaxAttachmentSize', 1000 );
+RT->Config->Set( 'TruncateLongAttachments', '1' );
+
+my $queue = RT::Test->load_or_create_queue( Name => 'General' );
+ok( $queue->id, "Loaded General queue" );
+
+my $cf = RT::CustomField->new( RT->SystemUser );
+ok(
+    $cf->Create(
+        Name  => 'test truncation',
+        Queue => '0',
+        Type  => 'FreeformSingle',
+    ),
+);
+my $cfid = $cf->id;
+
+my ( $baseurl, $m ) = RT::Test->started_ok;
+ok $m->login, 'logged in';
+
+$m->get_ok( $baseurl . '/Ticket/Create.html?Queue=' . $queue->id );
+$m->content_contains( "Create a new ticket", 'ticket create page' );
+
+$m->form_name('TicketCreate');
+$m->field( 'Subject', 'Attachments test' );
+$m->field( 'Attach',  $path );
+$m->field( 'Content', 'Some content' );
+my $cf_content = 'cf' . 'a' x 998 . 'cfb';
+$m->field( "Object-RT::Ticket--CustomField-$cfid-Value", $cf_content );
+$m->submit;
+is( $m->status, 200, "request successful" );
+
+$m->content_contains( "$name truncated because its size (1010 bytes) exceeded configured maximum size setting (1000 bytes).", 'truncated message' );
+$m->content_contains( 'cf' . 'a' x 998, 'has the first 1000 cf chars' );
+$m->content_lacks( 'aaacfb', 'lacks cf chars after that' );
+$m->follow_link_ok( { text => "Download $name" } );
+$m->content_contains( 'a' x 1000, 'has the first 1000 chars' );
+$m->content_lacks( 'b', 'lacks chars after that' );
+
+undef $m;
+done_testing;

commit 8fc4cfff01edc6ae74810741d5acdb2caa89035a
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue Jul 15 03:31:47 2014 +0800

    make attachment truncate/drop and error txns more noticeable
    
    color scheme is from cloos++

diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index db2f7be..e0b2113 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -1795,6 +1795,8 @@ our %TRANSACTION_CLASSIFICATION = (
         ) ),
     },
     SystemError => 'error',
+    AttachmentTruncate => 'attachment-truncate',
+    AttachmentDrop => 'attachment-drop',
     __default => 'other',
 );
 
diff --git a/share/static/css/base/history.css b/share/static/css/base/history.css
index be16f31..4884fac 100644
--- a/share/static/css/base/history.css
+++ b/share/static/css/base/history.css
@@ -148,6 +148,10 @@ padding-right:0.25em;
 .transaction.reminders .type { background: #369; }
 .transaction.other .type { background: #abc; }
 .transaction.error .type { background: #abc; }
+.transaction.attachment-truncate .type, .transaction.attachment-drop .type { background-color: #abc; }
+
+.transaction.error { background-color: #fcc; }
+.transaction.attachment-truncate, .transaction.attachment-drop { background-color: #ffc; }
 
 
 .transaction .message-header-value.verify { font-weight: bold; }

commit a3aaa712f8905cd4620f6b7c5b9183e4a3c90dc8
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Tue Jul 15 04:19:05 2014 +0800

    refactor email sent failure system error txn to use the same tech as Attachment Truncate/Drop.
    
    it's more simple and secure: by specifying ActivateScrips to 0, it won't cause infinite-loop.

diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index 75669ce..3250a41 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -1771,9 +1771,10 @@ sub IsCorrectAction {
 sub _RecordSendEmailFailure {
     my $ticket = shift;
     if ($ticket) {
-        $ticket->_RecordNote(
-            NoteType => 'SystemError',
-            Content => "Sending the previous mail has failed.  Please contact your admin, they can find more details in the logs.",
+        $ticket->_NewTransaction(
+            Type => "SystemError",
+            Data => "Sending the previous mail has failed.  Please contact your admin, they can find more details in the logs.",
+            ActivateScrips => 0,
         );
         return 1;
     }
diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index fdc66a8..3293209 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -839,7 +839,7 @@ sub _FormatUser {
     },
     SystemError => sub {
         my $self = shift;
-        return ("System error"); #loc()
+        return $self->Data // ("System error"); #loc()
     },
     AttachmentTruncate => sub {
         my $self = shift;

-----------------------------------------------------------------------


More information about the rt-commit mailing list