#442

backend/app/controllers/post_versions_controller.rb

@@ -14,8 +14,9 @@ class PostVersionsController < ApplicationController
 
     offset = (page - 1) * limit
 
-    q = HistoryAccess.visible(version_scope)
+    q = HistoryAccess.visible(PostVersion.all)
     q = q.where('post_versions.post_id = ?', post_id) if post_id
+    q = q.with_previous_version if tag_id || external_tag_id
     if external_tag_id || (tag_id && !(Tag.exists?(id: tag_id)))
       q = q.where('JSON_CONTAINS(post_versions.tags_json,' +
                   "JSON_OBJECT('external_tag_id', #{ external_tag_id || tag_id })) " +
@@ -28,9 +29,12 @@ class PostVersionsController < ApplicationController
 
     count = q.except(:select, :order, :limit, :offset).count
 
-    versions = q.order(Arel.sql('post_versions.created_at DESC, post_versions.id DESC'))
+    ids = q.order(Arel.sql('post_versions.created_at DESC, post_versions.id DESC'))
                 .limit(limit)
                 .offset(offset)
+                .pluck('post_versions.id')
+    versions = HistoryAccess.visible(version_scope.where(id: ids))
+        .order(Arel.sql('post_versions.created_at DESC, post_versions.id DESC'))
 
     render json: { versions: serialise_versions(versions, locale.language_code), count:, population_count: HistoryAccess.population_count(PostVersion) }
   end

backend/app/controllers/posts_controller.rb

@@ -39,18 +39,16 @@ class PostsController < ApplicationController
 
     pt_max_sql =
         HistoryAccess.visible(PostVersion.all)
-        .select('post_id, MAX(created_at) AS max_updated_at')
-        .group('post_id')
+        .where('post_versions.post_id = posts.id')
+        .select('MAX(post_versions.created_at)')
         .to_sql
 
     updated_at_all_sql =
         'GREATEST(posts.updated_at,' +
-        'COALESCE(pt_max.max_updated_at, posts.updated_at))'
+        "COALESCE((#{ pt_max_sql }), posts.updated_at))"
 
     q =
         filtered_posts(locale)
-        .joins("LEFT JOIN (#{ pt_max_sql }) pt_max ON pt_max.post_id = posts.id")
-        .reselect('posts.*', Arel.sql("#{ updated_at_all_sql } AS updated_at_all"))
         .preload(:external_tags, :uploaded_user, :parents, :children,
                  post_tags: [:sections, { tag: [:deerjikists, :materials,
                                                 :tag_names, :tag_descriptions] }])
@@ -88,12 +86,15 @@ class PostsController < ApplicationController
              .limit(limit)
              .offset(offset)
              .to_a
+    latest_version_times = HistoryAccess.visible(PostVersion.where(post_id: posts.map(&:id)))
+        .group(:post_id).maximum(:created_at)
 
     q = q.except(:select, :order)
 
     render json: { posts: posts.map { |post|
       PostRepr.base(post, language_code: locale.language_code)
-              .merge(updated_at: post.updated_at_all).tap do |json|
+              .merge(updated_at: [post.updated_at, latest_version_times[post.id]].compact.max)
+              .tap do |json|
         json['thumbnail'] =
             if post.thumbnail.attached?
               rails_storage_proxy_url(post.thumbnail, only_path: false)

backend/app/models/history_access.rb

@@ -109,11 +109,15 @@ class HistoryAccess < ActiveSupport::CurrentAttributes
       # conservatively; never expose a private linked name through an external tag.
       names = "JSON_MERGE_PRESERVE(JSON_ARRAY(private_tag.name), " +
           "COALESCE(JSON_EXTRACT(private_tag.names, '$**.name'), JSON_ARRAY()))"
-      ["NOT EXISTS (SELECT 1 FROM tag_versions private_tag " +
+      # DISTINCT prevents merging this independent name set into the correlated scan.
+      # Keep byte-distinct names even when the column collation considers them equal.
+      private_names = 'SELECT DISTINCT private_name.name, ' +
+          'CAST(private_name.name AS BINARY) AS name_bytes FROM tag_versions private_tag ' +
           "JOIN (#{ discarded_keys('tag_versions', 'tag_id') }) blocked " +
           "ON blocked.tag_id = private_tag.tag_id " +
           "JOIN JSON_TABLE(#{ names }, '$[*]' COLUMNS (name TEXT PATH '$')) private_name " +
-          "WHERE private_name.name <> '' AND " +
+          "WHERE private_name.name <> ''"
+      ["NOT EXISTS (SELECT 1 FROM (#{ private_names }) private_name WHERE " +
           "LOCATE(CONCAT(' ', private_name.name, ' '), " +
           "CONCAT(' ', COALESCE(#{ row }.linked_tags, ''), ' ')) > 0)"]
     when 'tag_versions'

backend/spec/requests/post_history_privacy_spec.rb

@@ -0,0 +1,94 @@
+require 'rails_helper'
+
+RSpec.describe 'Post history privacy', type: :request do
+  let(:origin) { Time.utc(2020, 1, 1) }
+  let!(:post_record) do
+    create(:post, title: 'current', created_at: origin, updated_at: origin)
+  end
+  let!(:other_post) do
+    create(:post, created_at: origin, updated_at: origin + 3.days + 1.hour)
+  end
+  let!(:tag) { create(:tag, primary_name: 'private-history-tag') }
+  let!(:external) { create(:external_tag, name: 'external-history-tag') }
+
+  before do
+    ['create', 'discard'].each_with_index do |event, index|
+      TagVersion.create!(tag:, version_no: index + 1, event_type: event,
+          name: 'private-history-tag', category: 'general', aliases: '', parent_tag_ids: '',
+          names: { ja: { name: 'private-history-tag', script_code: 'Jpan',
+                         auto_generated: false, aliases: [] } },
+          created_at: origin + index.hours)
+    end
+    NicoTagVersion.create!(external_tag: external, version_no: 1, event_type: 'create',
+        name: 'nico:external-history-tag', linked_tags: 'private-history-tag',
+        created_at: origin)
+    ['create', 'discard', 'restore', 'update'].each_with_index do |event, index|
+      snapshots =
+          if index < 2
+            [{ tag_id: tag.id, version_no: 1, name: 'private-history-tag', sections: [] }]
+          elsif index == 3
+            [{ external_tag_id: external.id }]
+          else
+            []
+          end
+      PostVersion.create!(post: post_record, version_no: index + 1, event_type: event,
+          title: "version #{ index + 1 }", url: post_record.url, tags: '',
+          tags_json: snapshots, parent_post_ids: '', created_at: origin + (index + 1).days)
+    end
+  end
+
+  ['anonymous', 'member', 'admin'].each do |role|
+    context "as #{ role }" do
+      before do
+        sign_in_as(create(:user, role.to_sym)) unless role == 'anonymous'
+      end
+
+      it 'uses only authorised history timestamps for display' do
+        get '/posts'
+        expect(response).to have_http_status(:ok)
+        row = response.parsed_body.fetch('posts').find { _1.fetch('id') == post_record.id }
+        expected = origin + (role == 'admin' ? 4 : 3).days
+        expect(Time.zone.parse(row.fetch('updated_at'))).to eq(expected)
+      end
+
+      it 'uses the same visibility for date bounds and count' do
+        get '/posts', params: { updated_from: (origin + 4.days).iso8601,
+                                updated_to: (origin + 4.days).iso8601 }
+        expect(response).to have_http_status(:ok)
+        ids = response.parsed_body.fetch('posts').pluck('id')
+        expect(ids).to eq(role == 'admin' ? [post_record.id] : [])
+        expect(response.parsed_body.fetch('count')).to eq(ids.length)
+      end
+
+      it 'sorts using authorised timestamps in both directions' do
+        ['asc', 'desc'].each do |direction|
+          get '/posts', params: { order: "updated_at:#{ direction }" }
+          expect(response).to have_http_status(:ok)
+          expected = role == 'admin' ? [other_post.id, post_record.id] :
+              [post_record.id, other_post.id]
+          expected.reverse! if direction == 'desc'
+          expect(response.parsed_body.fetch('posts').pluck('id')).to eq(expected)
+        end
+      end
+
+      it 'counts only visible versions and masks a private previous version' do
+        get '/posts/versions', params: { post: post_record.id }
+        expect(response).to have_http_status(:ok)
+        body = response.parsed_body
+        expected = role == 'admin' ? [4, 3, 2, 1] : [3]
+        expect(body.fetch('count')).to eq(expected.length)
+        expect(body.fetch('versions').pluck('version_no')).to eq(expected)
+        restored = body.fetch('versions').find { _1.fetch('version_no') == 3 }
+        expect(restored.fetch('title').fetch('prev')).to eq(role == 'admin' ? 'version 2' : nil)
+      end
+
+      it 'uses only an authorised previous version for tag filtering' do
+        get '/posts/versions', params: { post: post_record.id, tag: tag.id }
+        expect(response).to have_http_status(:ok)
+        expected = role == 'admin' ? [3, 2, 1] : []
+        expect(response.parsed_body.fetch('versions').pluck('version_no')).to eq(expected)
+        expect(response.parsed_body.fetch('count')).to eq(expected.length)
+      end
+    end
+  end
+end