FIX: never attempt to log invalid post numbers

Previously in some cases we would queue logging of invalid post numbers

The impact would be we would miss logging an incoming link and would leak
an error.
This commit is contained in:
Sam 2018-11-21 11:58:47 +11:00
parent 86255faa08
commit 20268385a5
4 changed files with 23 additions and 3 deletions

View File

@ -806,7 +806,7 @@ class TopicsController < ApplicationController
host: request.host, host: request.host,
current_user: current_user, current_user: current_user,
topic_id: @topic_view.topic.id, topic_id: @topic_view.topic.id,
post_number: params[:post_number], post_number: @topic_view.current_post_number,
username: request['u'], username: request['u'],
ip_address: request.remote_ip ip_address: request.remote_ip
} }

View File

@ -174,11 +174,11 @@ class TopicViewSerializer < ApplicationSerializer
end end
def current_post_number def current_post_number
[object.post_number, object.highest_post_number].min object.current_post_number
end end
def include_current_post_number? def include_current_post_number?
object.highest_post_number.present? object.current_post_number.present?
end end
def highest_post_number def highest_post_number

View File

@ -481,6 +481,14 @@ class TopicView
@filtered_posts.order(sort_order: :desc).limit(1).pluck(:id).first @filtered_posts.order(sort_order: :desc).limit(1).pluck(:id).first
end end
def current_post_number
if highest_post_number.present?
post_number > highest_post_number ? highest_post_number : post_number
else
nil
end
end
protected protected
def read_posts_set def read_posts_set

View File

@ -1095,6 +1095,18 @@ RSpec.describe TopicsController do
end.to change(TopicViewItem, :count).by(1) end.to change(TopicViewItem, :count).by(1)
end end
it 'records a view to invalid post_number' do
user = Fabricate(:user)
expect do
get "/t/#{topic.slug}/#{topic.id}/#{256**4}", params: {
u: user.username
}
expect(response.status).to eq(200)
end.to change { IncomingLink.count }.by(1)
end
it 'records incoming links' do it 'records incoming links' do
user = Fabricate(:user) user = Fabricate(:user)