From 4f28c71b5082d8194129f10084738775b36b8ed3 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Mon, 13 Nov 2017 15:36:45 +1100
Subject: [PATCH] FIX: error setting tombstone bucket when set to old version

---
 lib/s3_helper.rb                  | 14 ++++++++-
 spec/components/s3_helper_spec.rb | 52 +++++++++++++++++++++++++++++--
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/lib/s3_helper.rb b/lib/s3_helper.rb
index d1e5a0aa78f..9dadd71de3e 100644
--- a/lib/s3_helper.rb
+++ b/lib/s3_helper.rb
@@ -100,12 +100,24 @@ class S3Helper
       # skip trying to merge
     end
 
+    # in the past we has a rule that was called purge-tombstone vs purge_tombstone
+    # just go ahead and normalize for our bucket
     rules.delete_if do |r|
-      r.id == id
+      r.id.gsub('_', '-') == id.gsub('_', '-')
     end
 
     rules << rule
 
+    # normalize filter in rules, due to AWS library bug
+    rules = rules.map do |r|
+      r = r.to_h
+      prefix = r.delete(:prefix)
+      if prefix
+        r[:filter] = { prefix: prefix }
+      end
+      r
+    end
+
     s3_resource.client.put_bucket_lifecycle_configuration(
       bucket: @s3_bucket_name,
       lifecycle_configuration: {
diff --git a/spec/components/s3_helper_spec.rb b/spec/components/s3_helper_spec.rb
index 0ee326b65b2..412f0805e25 100644
--- a/spec/components/s3_helper_spec.rb
+++ b/spec/components/s3_helper_spec.rb
@@ -3,6 +3,54 @@ require "rails_helper"
 
 describe "S3Helper" do
 
-  # we should test something here, perhaps in a smoke test that uploads a real image
-  # to s3
+  it "can correctly set the purge policy" do
+
+    stub_request(:get, "http://169.254.169.254/latest/meta-data/iam/security-credentials/").
+      to_return(status: 404, body: "", headers: {})
+
+    lifecycle = <<~XML
+      <?xml version="1.0" encoding="UTF-8"?>
+      <LifecycleConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
+        <Rule>
+            <ID>old_rule</ID>
+            <Prefix>projectdocs/</Prefix>
+            <Status>Enabled</Status>
+            <Expiration>
+               <Days>3650</Days>
+            </Expiration>
+        </Rule>
+        <Rule>
+            <ID>purge-tombstone</ID>
+            <Prefix>test/</Prefix>
+            <Status>Enabled</Status>
+            <Expiration>
+               <Days>3650</Days>
+            </Expiration>
+        </Rule>
+      </LifecycleConfiguration>
+    XML
+
+    stub_request(:get, "https://bob.s3.amazonaws.com/?lifecycle").
+      to_return(status: 200, body: lifecycle, headers: {})
+
+    stub_request(:put, "https://bob.s3.amazonaws.com/?lifecycle").
+      with do |req|
+
+      hash = Hash.from_xml(req.body.to_s)
+      rules = hash["LifecycleConfiguration"]["Rule"]
+
+      expect(rules.length).to eq(2)
+
+      # fixes the bad filter
+      expect(rules[0]["Filter"]["Prefix"]).to eq("projectdocs/")
+    end.to_return(status: 200, body: "", headers: {})
+
+    SiteSetting.enable_s3_uploads = true
+    SiteSetting.s3_access_key_id = "abc"
+    SiteSetting.s3_secret_access_key = "def"
+
+    helper = S3Helper.new('bob', 'tomb')
+    helper.update_tombstone_lifecycle(100)
+  end
+
 end