Extract FoiAttachment::Dsn & restore functionality

app/models/foi_attachment.rb

@@ -157,76 +157,14 @@ def main_body_part?
     self == incoming_message.get_main_body_text_part
   end
 
-  # List of DSN codes taken from RFC 3463
-  # http://tools.ietf.org/html/rfc3463
-  DsnToMessage = {
-    'X.1.0' => 'Other address status',
-    'X.1.1' => 'Bad destination mailbox address',
-    'X.1.2' => 'Bad destination system address',
-    'X.1.3' => 'Bad destination mailbox address syntax',
-    'X.1.4' => 'Destination mailbox address ambiguous',
-    'X.1.5' => 'Destination mailbox address valid',
-    'X.1.6' => 'Mailbox has moved',
-    'X.1.7' => 'Bad sender\'s mailbox address syntax',
-    'X.1.8' => 'Bad sender\'s system address',
-    'X.2.0' => 'Other or undefined mailbox status',
-    'X.2.1' => 'Mailbox disabled, not accepting messages',
-    'X.2.2' => 'Mailbox full',
-    'X.2.3' => 'Message length exceeds administrative limit.',
-    'X.2.4' => 'Mailing list expansion problem',
-    'X.3.0' => 'Other or undefined mail system status',
-    'X.3.1' => 'Mail system full',
-    'X.3.2' => 'System not accepting network messages',
-    'X.3.3' => 'System not capable of selected features',
-    'X.3.4' => 'Message too big for system',
-    'X.4.0' => 'Other or undefined network or routing status',
-    'X.4.1' => 'No answer from host',
-    'X.4.2' => 'Bad connection',
-    'X.4.3' => 'Routing server failure',
-    'X.4.4' => 'Unable to route',
-    'X.4.5' => 'Network congestion',
-    'X.4.6' => 'Routing loop detected',
-    'X.4.7' => 'Delivery time expired',
-    'X.5.0' => 'Other or undefined protocol status',
-    'X.5.1' => 'Invalid command',
-    'X.5.2' => 'Syntax error',
-    'X.5.3' => 'Too many recipients',
-    'X.5.4' => 'Invalid command arguments',
-    'X.5.5' => 'Wrong protocol version',
-    'X.6.0' => 'Other or undefined media error',
-    'X.6.1' => 'Media not supported',
-    'X.6.2' => 'Conversion required and prohibited',
-    'X.6.3' => 'Conversion required but not supported',
-    'X.6.4' => 'Conversion with loss performed',
-    'X.6.5' => 'Conversion failed',
-    'X.7.0' => 'Other or undefined security status',
-    'X.7.1' => 'Delivery not authorized, message refused',
-    'X.7.2' => 'Mailing list expansion prohibited',
-    'X.7.3' => 'Security conversion required but not possible',
-    'X.7.4' => 'Security features not supported',
-    'X.7.5' => 'Cryptographic failure',
-    'X.7.6' => 'Cryptographic algorithm not supported',
-    'X.7.7' => 'Message integrity failure'
-  }
-
   # Returns HTML, of extra comment to put by attachment
   def extra_note
-    # For delivery status notification attachments, extract the status and
-    # look up what it means in the DSN table.
-    if @content_type == 'message/delivery-status'
-      return "" unless @body.match(/Status:\s+([0-9]+\.([0-9]+\.[0-9]+))\s+/)
-
-      dsn = $1
-      dsn_part = 'X.' + $2
+    return unless content_type == 'message/delivery-status'
 
-      dsn_message = ""
-      if DsnToMessage.include?(dsn_part)
-        dsn_message = " (" + DsnToMessage[dsn_part] + ")"
-      end
+    dsn = DeliveryStatusNotification.new(body)
+    return unless dsn.status && dsn.message
 
-      return "<br><em>DSN: " + dsn + dsn_message + "</em>"
-    end
-    ""
+    "DSN: #{dsn.status} #{dsn.message}"
   end
 
   # Called by controller so old filenames still work

app/models/foi_attachment/delivery_status_notification.rb

@@ -0,0 +1,84 @@
+# Parse DSN status code to provide a human readable summary message
+class FoiAttachment::DeliveryStatusNotification
+  # List of DSN codes taken from RFC 3463
+  # http://tools.ietf.org/html/rfc3463
+  STATUS_TO_MESSAGE = {
+    'X.1.0' => 'Other address status',
+    'X.1.1' => 'Bad destination mailbox address',
+    'X.1.2' => 'Bad destination system address',
+    'X.1.3' => 'Bad destination mailbox address syntax',
+    'X.1.4' => 'Destination mailbox address ambiguous',
+    'X.1.5' => 'Destination mailbox address valid',
+    'X.1.6' => 'Mailbox has moved',
+    'X.1.7' => 'Bad sender\'s mailbox address syntax',
+    'X.1.8' => 'Bad sender\'s system address',
+    'X.2.0' => 'Other or undefined mailbox status',
+    'X.2.1' => 'Mailbox disabled, not accepting messages',
+    'X.2.2' => 'Mailbox full',
+    'X.2.3' => 'Message length exceeds administrative limit.',
+    'X.2.4' => 'Mailing list expansion problem',
+    'X.3.0' => 'Other or undefined mail system status',
+    'X.3.1' => 'Mail system full',
+    'X.3.2' => 'System not accepting network messages',
+    'X.3.3' => 'System not capable of selected features',
+    'X.3.4' => 'Message too big for system',
+    'X.4.0' => 'Other or undefined network or routing status',
+    'X.4.1' => 'No answer from host',
+    'X.4.2' => 'Bad connection',
+    'X.4.3' => 'Routing server failure',
+    'X.4.4' => 'Unable to route',
+    'X.4.5' => 'Network congestion',
+    'X.4.6' => 'Routing loop detected',
+    'X.4.7' => 'Delivery time expired',
+    'X.5.0' => 'Other or undefined protocol status',
+    'X.5.1' => 'Invalid command',
+    'X.5.2' => 'Syntax error',
+    'X.5.3' => 'Too many recipients',
+    'X.5.4' => 'Invalid command arguments',
+    'X.5.5' => 'Wrong protocol version',
+    'X.6.0' => 'Other or undefined media error',
+    'X.6.1' => 'Media not supported',
+    'X.6.2' => 'Conversion required and prohibited',
+    'X.6.3' => 'Conversion required but not supported',
+    'X.6.4' => 'Conversion with loss performed',
+    'X.6.5' => 'Conversion failed',
+    'X.7.0' => 'Other or undefined security status',
+    'X.7.1' => 'Delivery not authorized, message refused',
+    'X.7.2' => 'Mailing list expansion prohibited',
+    'X.7.3' => 'Security conversion required but not possible',
+    'X.7.4' => 'Security features not supported',
+    'X.7.5' => 'Cryptographic failure',
+    'X.7.6' => 'Cryptographic algorithm not supported',
+    'X.7.7' => 'Message integrity failure'
+  }.freeze
+
+  def initialize(body)
+    @body = body
+  end
+
+  def status
+    @status ||= status!
+  end
+
+  def message
+    STATUS_TO_MESSAGE[status_part]
+  end
+
+  private
+
+  attr_reader :body
+
+  def status!
+    @status = match ? match[1] : nil
+  end
+
+  def status_part
+    return '' unless match
+
+    'X.' + match[2]
+  end
+
+  def match
+    body.match(/Status:\s+([0-9]+\.([0-9]+\.[0-9]+))\s+/)
+  end
+end

app/views/request/_attachments.html.erb

@@ -32,7 +32,10 @@
               <% if a.has_body_as_html? && incoming_message.info_request.prominence(:decorate => true).is_public? %>
                 <%= link_to "View as HTML", attachment_path(a, :html => true) %>
               <% end %>
-              <%= a.extra_note %>
+
+              <% if note = a.extra_note %>
+                <em><%= note %></em>
+              <% end %>
             </p>
           <% end %>
         <% end %>

doc/CHANGES.md

@@ -2,6 +2,7 @@
 
 ## Highlighted Features
 
+* Restore delivery status notification attachment note (Gareth Rees)
 * Allow `InfoRequest` to be categorised (Graeme Porteous)
 * Replace public body categories with generalised categories (Graeme Porteous)
 * Add admin links to and from batch request show action (Graeme Porteous)

spec/factories/foi_attchments.rb

@@ -29,16 +29,19 @@
       body { 'hereisthetext' }
       filename { 'attachment.txt' }
     end
+
     factory :pdf_attachment do
       content_type { 'application/pdf' }
       filename { 'interesting.pdf' }
       body { load_file_fixture('interesting.pdf') }
     end
+
     factory :rtf_attachment do
       content_type { 'application/rtf' }
       filename { 'interesting.rtf' }
       body { load_file_fixture('interesting.rtf') }
     end
+
     factory :html_attachment do
       content_type { 'text/html' }
       filename { 'interesting.html' }
@@ -51,11 +54,19 @@
         Mail::Utilities.to_crlf(load_file_fixture('interesting.html'))
       }
     end
+
     factory :jpeg_attachment do
       content_type { 'image/jpeg' }
       filename { 'interesting.jpg' }
       body { 'someimage' }
     end
+
+    factory :delivery_status_notification_attachment do
+      content_type { 'message/delivery-status' }
+      filename { 'attachment.delivery_status' }
+      body { load_file_fixture('attachment.delivery_status') }
+    end
+
     factory :unknown_attachment do
       content_type { 'application/unknown' }
       filename { 'interesting.spc' }

spec/fixtures/files/attachment.delivery_status

@@ -0,0 +1,10 @@
+X-Symantec-Messaging-Gateway-Queue-ID: EE/CD-06721-9C109DE4
+X-Symantec-Messaging-Gateway-Sender: rfc822; [email protected]
+Reporting-MTA: dns; gov.example.com
+Arrival-Date: Sat, 03 Dec 2011 04:50:17 +0000
+
+Final-Recipient: rfc822; [email protected]
+Status: 4.4.0
+Action: delayed
+Last-Attempt-Date: Sat, 03 Dec 2011 04:50:17 +0000
+Diagnostic-Code: smtp; 421 4.4.0 [internal] no MXs for this domain could be reached at this time

spec/models/foi_attachment/delivery_status_notification_spec.rb

@@ -0,0 +1,47 @@
+require 'spec_helper'
+
+RSpec.describe FoiAttachment::DeliveryStatusNotification do
+  let(:valid_status_body) do
+    <<~EOF
+    Action: failed
+    Status: 5.4.1
+    EOF
+  end
+
+  let(:invalid_status_body) do
+    <<~EOF
+    Action: failed
+    Status: invalid
+    EOF
+  end
+
+  describe '#status' do
+    subject { dsn.status }
+    let(:dsn) { described_class.new(body) }
+
+    context 'with a valid status' do
+      let(:body) { valid_status_body }
+      it { is_expected.to eq('5.4.1') }
+    end
+
+    context 'with an invalid status' do
+      let(:body) { invalid_status_body }
+      it { is_expected.to be_nil }
+    end
+  end
+
+  describe '#message' do
+    subject { dsn.message }
+    let(:dsn) { described_class.new(body) }
+
+    context 'with a valid status' do
+      let(:body) { valid_status_body }
+      it { is_expected.to eq('No answer from host') }
+    end
+
+    context 'with an invalid status' do
+      let(:body) { invalid_status_body }
+      it { is_expected.to be_nil }
+    end
+  end
+end

spec/models/foi_attachment_spec.rb

@@ -374,4 +374,25 @@
       it { is_expected.to be_nil }
     end
   end
+
+  describe '#extra_note' do
+    subject { foi_attachment.extra_note }
+
+    context 'with a delivery status notification' do
+      let(:foi_attachment) do
+        FactoryBot.create(:delivery_status_notification_attachment)
+      end
+
+      let(:note) do
+        'DSN: 4.4.0 Other or undefined network or routing status'
+      end
+
+      it { is_expected.to eq(note) }
+    end
+
+    context 'with any other content type' do
+      let(:foi_attachment) { FactoryBot.build(:rtf_attachment) }
+      it { is_expected.to be_nil }
+    end
+  end
 end