Set snowflake IDs for backdated statuses (#5260)

- Rename Mastodon::TimestampIds into Mastodon::Snowflake for clarity
- Skip for statuses coming from inbox, aka delivered in real-time
- Skip for statuses that claim to be from the future

diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb
index b06dd619460ae1069e66e6dc6063382b4cfbb875..9688f57a64b2cf0335c29bea1c56b83c169772c8 100644 (file)
--- a/app/lib/activitypub/activity.rb
+++ b/app/lib/activitypub/activity.rb
@@ -3,10 +3,11 @@
 class ActivityPub::Activity
   include JsonLdHelper
 
-  def initialize(json, account)
+  def initialize(json, account, options = {})
     @json    = json
     @account = account
     @object  = @json['object']
+    @options = options
   end
 
   def perform
@@ -14,9 +15,9 @@ class ActivityPub::Activity
   end
 
   class << self
-    def factory(json, account)
+    def factory(json, account, options = {})
       @json = json
-      klass&.new(json, account)
+      klass&.new(json, account, options)
     end
 
     private

diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb
index 1cf844281f65b290cf832ccaf9564cb328a09a88..b840989330ec3315a1ad79ce2e12ab416d10737c 100644 (file)
--- a/app/lib/activitypub/activity/announce.rb
+++ b/app/lib/activitypub/activity/announce.rb
@@ -15,8 +15,9 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity
       account: @account,
       reblog: original_status,
       uri: @json['id'],
-      created_at: @json['published'] || Time.now.utc
+      created_at: @options[:override_timestamps] ? nil : @json['published']
     )
+
     distribute(status)
     status
   end

diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb
index 9421a0aa7e9bf0ba8e7180f4be0bdece63a1c83d..d6e9bc1def1872f735fb1bfa89b87986325d50f6 100644 (file)
--- a/app/lib/activitypub/activity/create.rb
+++ b/app/lib/activitypub/activity/create.rb
@@ -43,7 +43,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
       text: text_from_content || '',
       language: language_from_content,
       spoiler_text: @object['summary'] || '',
-      created_at: @object['published'] || Time.now.utc,
+      created_at: @options[:override_timestamps] ? nil : @object['published'],
       reply: @object['inReplyTo'].present?,
       sensitive: @object['sensitive'] || false,
       visibility: visibility_from_audience,

diff --git a/app/lib/ostatus/activity/base.rb b/app/lib/ostatus/activity/base.rb
index 0393813979727aefb94d49e259a884c4b51d2686..8b27b124f68c0e9d35f6ef03ec279360968814ae 100644 (file)
--- a/app/lib/ostatus/activity/base.rb
+++ b/app/lib/ostatus/activity/base.rb
@@ -1,9 +1,10 @@
 # frozen_string_literal: true
 
 class OStatus::Activity::Base
-  def initialize(xml, account = nil)
-    @xml = xml
+  def initialize(xml, account = nil, options = {})
+    @xml     = xml
     @account = account
+    @options = options
   end
 
   def status?

diff --git a/app/lib/ostatus/activity/creation.rb b/app/lib/ostatus/activity/creation.rb
index 511c462d42f98f1bbed221c142f17118701e760d..a1ab522e23c3d9b0746bc492bc566efd61b8d2a7 100644 (file)
--- a/app/lib/ostatus/activity/creation.rb
+++ b/app/lib/ostatus/activity/creation.rb
@@ -34,7 +34,7 @@ class OStatus::Activity::Creation < OStatus::Activity::Base
         reblog: cached_reblog,
         text: content,
         spoiler_text: content_warning,
-        created_at: published,
+        created_at: @options[:override_timestamps] ? nil : published,
         reply: thread?,
         language: content_language,
         visibility: visibility_scope,

diff --git a/app/lib/ostatus/activity/general.rb b/app/lib/ostatus/activity/general.rb
index b3bef9861dca76f670ae40254aeada278558c030..8a6aabc3372edb36b0d68bf12f47cd0c2a53b7b9 100644 (file)
--- a/app/lib/ostatus/activity/general.rb
+++ b/app/lib/ostatus/activity/general.rb
@@ -2,7 +2,7 @@
 
 class OStatus::Activity::General < OStatus::Activity::Base
   def specialize
-    special_class&.new(@xml, @account)
+    special_class&.new(@xml, @account, @options)
   end
 
   private

diff --git a/app/models/status.rb b/app/models/status.rb
index ea4c097bfca8a4501e91ed8b384cb83bfe89ffd7..0d249244f3e746b18cda18626ecb952e8ec1b9ee 100644 (file)
--- a/app/models/status.rb
+++ b/app/models/status.rb
@@ -136,6 +136,8 @@ class Status < ApplicationRecord
 
   after_create :store_uri, if: :local?
 
+  around_create Mastodon::Snowflake::Callbacks
+
   before_validation :prepare_contents, if: :local?
   before_validation :set_reblog
   before_validation :set_visibility

diff --git a/app/services/activitypub/process_collection_service.rb b/app/services/activitypub/process_collection_service.rb
index 59cb65c65e886fcd36571d2bcb226ce3350fe590..db4d1b4bc032d04e2e7fbbc27ad2d7e838aacef3 100644 (file)
--- a/app/services/activitypub/process_collection_service.rb
+++ b/app/services/activitypub/process_collection_service.rb
@@ -3,9 +3,10 @@
 class ActivityPub::ProcessCollectionService < BaseService
   include JsonLdHelper
 
-  def call(body, account)
+  def call(body, account, options = {})
     @account = account
     @json    = Oj.load(body, mode: :strict)
+    @options = options
 
     return unless supported_context?
     return if different_actor? && verify_account!.nil?
@@ -38,7 +39,7 @@ class ActivityPub::ProcessCollectionService < BaseService
   end
 
   def process_item(item)
-    activity = ActivityPub::Activity.factory(item, @account)
+    activity = ActivityPub::Activity.factory(item, @account, @options)
     activity&.perform
   end
 

diff --git a/app/services/process_feed_service.rb b/app/services/process_feed_service.rb
index 2a5f1e2bc4f576b26976dcd9b70588ac156c3423..60eff135e986becea2a5d69a3924c40b4a5f7f06 100644 (file)
--- a/app/services/process_feed_service.rb
+++ b/app/services/process_feed_service.rb
@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
 class ProcessFeedService < BaseService
-  def call(body, account)
+  def call(body, account, options = {})
+    @options = options
+
     xml = Nokogiri::XML(body)
     xml.encoding = 'utf-8'
 
@@ -20,7 +22,7 @@ class ProcessFeedService < BaseService
   end
 
   def process_entry(xml, account)
-    activity = OStatus::Activity::General.new(xml, account)
+    activity = OStatus::Activity::General.new(xml, account, @options)
     activity.specialize&.perform if activity.status?
   rescue ActiveRecord::RecordInvalid => e
     Rails.logger.debug "Nothing was saved for #{activity.id} because: #{e}"

diff --git a/app/workers/activitypub/processing_worker.rb b/app/workers/activitypub/processing_worker.rb
index bb9adf64bde9cccb11c293900b68d7acf1a5eb8e..0e2e0edddba98b56db5ab48e7056e5e147d672ab 100644 (file)
--- a/app/workers/activitypub/processing_worker.rb
+++ b/app/workers/activitypub/processing_worker.rb
@@ -6,6 +6,6 @@ class ActivityPub::ProcessingWorker
   sidekiq_options backtrace: true
 
   def perform(account_id, body)
-    ActivityPub::ProcessCollectionService.new.call(body, Account.find(account_id))
+    ActivityPub::ProcessCollectionService.new.call(body, Account.find(account_id), override_timestamps: true)
   end
 end

diff --git a/app/workers/processing_worker.rb b/app/workers/processing_worker.rb
index 5df404bcc9b7bb3e653bbfb20d22482b9457c398..978c3aba26f2d0238fe0266868ca91ff6fc84d54 100644 (file)
--- a/app/workers/processing_worker.rb
+++ b/app/workers/processing_worker.rb
@@ -6,6 +6,6 @@ class ProcessingWorker
   sidekiq_options backtrace: true
 
   def perform(account_id, body)
-    ProcessFeedService.new.call(body, Account.find(account_id))
+    ProcessFeedService.new.call(body, Account.find(account_id), override_timestamps: true)
   end
 end

diff --git a/config/application.rb b/config/application.rb
index b6ce7414775a407d4226eaeaa5c696c4f0b1b75b..4860a08a1e8d4725d54b2869b4c21099e950ea76 100644 (file)
--- a/config/application.rb
+++ b/config/application.rb
@@ -9,6 +9,7 @@ Bundler.require(*Rails.groups)
 require_relative '../app/lib/exceptions'
 require_relative '../lib/paperclip/gif_transcoder'
 require_relative '../lib/paperclip/video_transcoder'
+require_relative '../lib/mastodon/snowflake'
 require_relative '../lib/mastodon/version'
 
 Dotenv::Railtie.load

diff --git a/config/brakeman.ignore b/config/brakeman.ignore
index 2a1bc1997c3880b1ffd38101943290fc2d0a2c2a..f198eebac2bedca95740814019ff52b9c226369d 100644 (file)
--- a/config/brakeman.ignore
+++ b/config/brakeman.ignore
@@ -57,26 +57,6 @@
       "confidence": "Weak",
       "note": ""
     },
-    {
-      "warning_type": "SQL Injection",
-      "warning_code": 0,
-      "fingerprint": "34efc76883080f8b1110a30c34ec4f903946ee56651aae46c62477f45d4fc412",
-      "check_name": "SQL",
-      "message": "Possible SQL injection",
-      "file": "lib/mastodon/timestamp_ids.rb",
-      "line": 63,
-      "link": "http://brakemanscanner.org/docs/warning_types/sql_injection/",
-      "code": "connection.execute(\"        CREATE OR REPLACE FUNCTION timestamp_id(table_name text)\\n        RETURNS bigint AS\\n        $$\\n          DECLARE\\n            time_part bigint;\\n            sequence_base bigint;\\n            tail bigint;\\n          BEGIN\\n            time_part := (\\n              -- Get the time in milliseconds\\n              ((date_part('epoch', now()) * 1000))::bigint\\n              -- And shift it over two bytes\\n              << 16);\\n\\n            sequence_base := (\\n              'x' ||\\n              -- Take the first two bytes (four hex characters)\\n              substr(\\n                -- Of the MD5 hash of the data we documented\\n                md5(table_name ||\\n                  '#{SecureRandom.hex(16)}' ||\\n                  time_part::text\\n                ),\\n                1, 4\\n              )\\n            -- And turn it into a bigint\\n            )::bit(16)::bigint;\\n\\n            -- Finally, add our sequence number to our base, and chop\\n            -- it to the last two bytes\\n            tail := (\\n              (sequence_base + nextval(table_name || '_id_seq'))\\n              & 65535);\\n\\n            -- Return the time part and the sequence part. OR appears\\n            -- faster here than addition, but they're equivalent:\\n            -- time_part has no trailing two bytes, and tail is only\\n            -- the last two bytes.\\n            RETURN time_part | tail;\\n          END\\n        $$ LANGUAGE plpgsql VOLATILE;\\n\")",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "Mastodon::TimestampIds",
-        "method": "define_timestamp_id"
-      },
-      "user_input": "SecureRandom.hex(16)",
-      "confidence": "Medium",
-      "note": ""
-    },
     {
       "warning_type": "Dynamic Render Path",
       "warning_code": 15,
@@ -106,7 +86,7 @@
       "line": 3,
       "link": "http://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
       "code": "render(action => \"stream_entries/#{Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase}\", { Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase.to_sym => Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity, :centered => true })",
-      "render_path": [{"type":"controller","class":"StatusesController","method":"embed","line":35,"file":"app/controllers/statuses_controller.rb"}],
+      "render_path": [{"type":"controller","class":"StatusesController","method":"embed","line":41,"file":"app/controllers/statuses_controller.rb"}],
       "location": {
         "type": "template",
         "template": "stream_entries/embed"
@@ -153,6 +133,26 @@
       "confidence": "Weak",
       "note": ""
     },
+    {
+      "warning_type": "SQL Injection",
+      "warning_code": 0,
+      "fingerprint": "9ccb9ba6a6947400e187d515e0bf719d22993d37cfc123c824d7fafa6caa9ac3",
+      "check_name": "SQL",
+      "message": "Possible SQL injection",
+      "file": "lib/mastodon/snowflake.rb",
+      "line": 86,
+      "link": "http://brakemanscanner.org/docs/warning_types/sql_injection/",
+      "code": "connection.execute(\"        CREATE OR REPLACE FUNCTION timestamp_id(table_name text)\\n        RETURNS bigint AS\\n        $$\\n          DECLARE\\n            time_part bigint;\\n            sequence_base bigint;\\n            tail bigint;\\n          BEGIN\\n            time_part := (\\n              -- Get the time in milliseconds\\n              ((date_part('epoch', now()) * 1000))::bigint\\n              -- And shift it over two bytes\\n              << 16);\\n\\n            sequence_base := (\\n              'x' ||\\n              -- Take the first two bytes (four hex characters)\\n              substr(\\n                -- Of the MD5 hash of the data we documented\\n                md5(table_name ||\\n                  '#{SecureRandom.hex(16)}' ||\\n                  time_part::text\\n                ),\\n                1, 4\\n              )\\n            -- And turn it into a bigint\\n            )::bit(16)::bigint;\\n\\n            -- Finally, add our sequence number to our base, and chop\\n            -- it to the last two bytes\\n            tail := (\\n              (sequence_base + nextval(table_name || '_id_seq'))\\n              & 65535);\\n\\n            -- Return the time part and the sequence part. OR appears\\n            -- faster here than addition, but they're equivalent:\\n            -- time_part has no trailing two bytes, and tail is only\\n            -- the last two bytes.\\n            RETURN time_part | tail;\\n          END\\n        $$ LANGUAGE plpgsql VOLATILE;\\n\")",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Mastodon::Snowflake",
+        "method": "define_timestamp_id"
+      },
+      "user_input": "SecureRandom.hex(16)",
+      "confidence": "Medium",
+      "note": ""
+    },
     {
       "warning_type": "Dynamic Render Path",
       "warning_code": 15,
@@ -269,6 +269,6 @@
       "note": ""
     }
   ],
-  "updated": "2017-10-06 03:27:46 +0200",
+  "updated": "2017-10-07 19:24:02 +0200",
   "brakeman_version": "4.0.1"
 }

diff --git a/lib/mastodon/timestamp_ids.rb b/lib/mastodon/snowflake.rb
similarity index 86%
rename from lib/mastodon/timestamp_ids.rb
rename to lib/mastodon/snowflake.rb
index 3b048a50ce63fa4e0f04c1500f3e7d1b3f18c498..219e323d487b88d07c737734613ac724c0cb8b3a 100644 (file)
--- a/lib/mastodon/timestamp_ids.rb
+++ b/lib/mastodon/snowflake.rb
@@ -1,8 +1,32 @@
 # frozen_string_literal: true
 
-module Mastodon::TimestampIds
+module Mastodon::Snowflake
   DEFAULT_REGEX = /timestamp_id\('(?<seq_prefix>\w+)'/
 
+  class Callbacks
+    def self.around_create(record)
+      now = Time.now.utc
+
+      if record.created_at.nil? || record.created_at >= now || record.created_at == record.updated_at
+        yield
+      else
+        record.id = Mastodon::Snowflake.id_at(record.created_at)
+        tries     = 0
+
+        begin
+          yield
+        rescue ActiveRecord::RecordNotUnique
+          raise if tries > 100
+
+          tries     += 1
+          record.id += rand(100)
+
+          retry
+        end
+      end
+    end
+  end
+
   class << self
     # Our ID will be composed of the following:
     # 6 bytes (48 bits) of millisecond-level timestamp
@@ -114,6 +138,13 @@ module Mastodon::TimestampIds
       end
     end
 
+    def id_at(timestamp)
+      id  = timestamp.to_i * 1000 + rand(1000)
+      id  = id << 16
+      id += rand(2**16)
+      id
+    end
+
     private
 
     def already_defined?

diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake
index 6af6bb6fb7e6028f83374d69e96b34d652b3234b..32039c31d1b4b2a6318e7e6fef33879ad49f9327 100644 (file)
--- a/lib/tasks/db.rake
+++ b/lib/tasks/db.rake
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-require Rails.root.join('lib', 'mastodon', 'timestamp_ids')
+require_relative '../mastodon/snowflake'
 
 def each_schema_load_environment
   # If we're in development, also run this for the test environment.
@@ -63,13 +63,13 @@ namespace :db do
 
   task :define_timestamp_id do
     each_schema_load_environment do
-      Mastodon::TimestampIds.define_timestamp_id
+      Mastodon::Snowflake.define_timestamp_id
     end
   end
 
   task :ensure_id_sequences_exist do
     each_schema_load_environment do
-      Mastodon::TimestampIds.ensure_id_sequences_exist
+      Mastodon::Snowflake.ensure_id_sequences_exist
     end
   end
 end

diff --git a/spec/services/activitypub/process_collection_service_spec.rb b/spec/services/activitypub/process_collection_service_spec.rb
index c1cc22523c3adc292cb366729cd6162e7cf9e18c..3cea970cfad94841875daf7dfb0c58e11b29c919 100644 (file)
--- a/spec/services/activitypub/process_collection_service_spec.rb
+++ b/spec/services/activitypub/process_collection_service_spec.rb
@@ -28,7 +28,7 @@ RSpec.describe ActivityPub::ProcessCollectionService do
 
       it 'processes payload with sender if no signature exists' do
         expect_any_instance_of(ActivityPub::LinkedDataSignature).not_to receive(:verify_account!)
-        expect(ActivityPub::Activity).to receive(:factory).with(instance_of(Hash), forwarder)
+        expect(ActivityPub::Activity).to receive(:factory).with(instance_of(Hash), forwarder, instance_of(Hash))
 
         subject.call(json, forwarder)
       end
@@ -37,7 +37,7 @@ RSpec.describe ActivityPub::ProcessCollectionService do
         payload['signature'] = {'type' => 'RsaSignature2017'}
 
         expect_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_account!).and_return(actor)
-        expect(ActivityPub::Activity).to receive(:factory).with(instance_of(Hash), actor)
+        expect(ActivityPub::Activity).to receive(:factory).with(instance_of(Hash), actor, instance_of(Hash))
 
         subject.call(json, forwarder)
       end