IntenseWebs/discourse
3
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-02-25 18:55:32 -06:00
Code Issues Packages Projects Releases Wiki Activity

FEATURE: Add site setting to show more detailed 404 errors. (#8014)

Browse Source 
If the setting is turned on, then the user will receive information
about the subject: if it was deleted or requires some special access to
a group (only if the group is public). Otherwise, the user will receive
a generic #404 error message. For now, this change affects only the
topics and categories controller.

This commit also tries to refactor some of the code related to error
handling. To make error pages more consistent (design-wise), the actual
error page will be rendered server-side.
This commit is contained in:
Dan Ungureanu
2019-10-08 14:15:08 +03:00
committed by GitHub
parent d2bceff133
commit fdb1d3404c
23 changed files with 769 additions and 660 deletions
Download Patch File Download Diff File Expand all files Collapse all files

144
app/controllers/application_controller.rb
View File

@@ -98,15 +98,59 @@ class ApplicationController < ActionController::Base
use_crawler_layout? ? 'crawler' : 'application'
end
# Some exceptions
class RenderEmpty < StandardError; end
class PluginDisabled < StandardError; end
# Render nothing
rescue_from RenderEmpty do
render 'default/empty'
end
def render_rate_limit_error(e)
rescue_from ArgumentError do |e|
if e.message == "string contains null byte"
raise Discourse::InvalidParameters, e.message
else
raise e
end
end
rescue_from PG::ReadOnlySqlTransaction do |e|
Discourse.received_postgres_readonly!
Rails.logger.error("#{e.class} #{e.message}: #{e.backtrace.join("\n")}")
raise Discourse::ReadOnly
end
rescue_from ActionController::ParameterMissing do |e|
render_json_error e.message, status: 400
end
rescue_from ActionController::RoutingError, PluginDisabled do
rescue_discourse_actions(:not_found, 404)
end
# Handles requests for giant IDs that throw pg exceptions
rescue_from ActiveModel::RangeError do |e|
if e.message =~ /ActiveModel::Type::Integer/
rescue_discourse_actions(:not_found, 404)
else
raise e
end
end
rescue_from ActiveRecord::RecordInvalid do |e|
if request.format && request.format.json?
render_json_error e, type: :record_invalid, status: 422
else
raise e
end
end
rescue_from ActiveRecord::StatementInvalid do |e|
Discourse.reset_active_record_cache_if_needed(e)
raise e
end
# If they hit the rate limiter
rescue_from RateLimiter::LimitExceeded do |e|
retry_time_in_seconds = e&.available_in
render_json_error(
@@ -118,25 +162,6 @@ class ApplicationController < ActionController::Base
)
end
rescue_from ActiveRecord::RecordInvalid do |e|
if request.format && request.format.json?
render_json_error e, type: :record_invalid, status: 422
else
raise e
end
end
# If they hit the rate limiter
rescue_from RateLimiter::LimitExceeded do |e|
render_rate_limit_error(e)
end
rescue_from PG::ReadOnlySqlTransaction do |e|
Discourse.received_postgres_readonly!
Rails.logger.error("#{e.class} #{e.message}: #{e.backtrace.join("\n")}")
raise Discourse::ReadOnly
end
rescue_from Discourse::NotLoggedIn do |e|
if (request.format && request.format.json?) || request.xhr? || !request.get?
rescue_discourse_actions(:not_logged_in, 403, include_ember: true)
@@ -145,14 +170,6 @@ class ApplicationController < ActionController::Base
end
end
rescue_from ArgumentError do |e|
if e.message == "string contains null byte"
raise Discourse::InvalidParameters, e.message
else
raise e
end
end
rescue_from Discourse::InvalidParameters do |e|
message = I18n.t('invalid_params', message: e.message)
if (request.format && request.format.json?) || request.xhr? || !request.get?
@@ -162,45 +179,27 @@ class ApplicationController < ActionController::Base
end
end
rescue_from ActiveRecord::StatementInvalid do |e|
Discourse.reset_active_record_cache_if_needed(e)
raise e
end
class PluginDisabled < StandardError; end
# Handles requests for giant IDs that throw pg exceptions
rescue_from ActiveModel::RangeError do |e|
if e.message =~ /ActiveModel::Type::Integer/
rescue_discourse_actions(:not_found, 404)
else
raise e
end
end
rescue_from Discourse::NotFound do |e|
rescue_discourse_actions(
:not_found,
e.status,
check_permalinks: e.check_permalinks,
original_path: e.original_path
original_path: e.original_path,
custom_message: e.custom_message
)
end
rescue_from PluginDisabled, ActionController::RoutingError do
rescue_discourse_actions(:not_found, 404)
end
rescue_from Discourse::InvalidAccess do |e|
if e.opts[:delete_cookie].present?
cookies.delete(e.opts[:delete_cookie])
end
rescue_discourse_actions(
:invalid_access,
403,
include_ember: true,
custom_message: e.custom_message
custom_message: e.custom_message,
group: e.group
)
end
@@ -208,10 +207,6 @@ class ApplicationController < ActionController::Base
render_json_error I18n.t('read_only_mode_enabled'), type: :read_only, status: 503
end
rescue_from ActionController::ParameterMissing do |e|
render_json_error e.message, status: 400
end
def redirect_with_client_support(url, options)
if request.xhr?
response.headers['Discourse-Xhr-Redirect'] = 'true'
@@ -241,18 +236,21 @@ class ApplicationController < ActionController::Base
end
message = opts[:custom_message_translated] || I18n.t(opts[:custom_message] || type)
error_page_opts = {
title: opts[:custom_message_translated] || I18n.t(opts[:custom_message] || "page_not_found.title"),
status: status_code,
group: opts[:group]
}
if show_json_errors
# HACK: do not use render_json_error for topics#show
if request.params[:controller] == 'topics' && request.params[:action] == 'show'
return render(
status: status_code,
layout: false,
plain: (status_code == 404 || status_code == 410) ? build_not_found_page(status_code) : message
)
opts = { type: type, status: status_code }
# Include error in HTML format for topics#show.
if (request.params[:controller] == 'topics' && request.params[:action] == 'show') || (request.params[:controller] == 'categories' && request.params[:action] == 'find_by_slug')
opts[:extras] = { html: build_not_found_page(error_page_opts) }
end
render_json_error message, type: type, status: status_code
render_json_error message, opts
else
begin
# 404 pages won't have the session and theme_keys without these:
@@ -262,7 +260,8 @@ class ApplicationController < ActionController::Base
return render plain: message, status: status_code
end
render html: build_not_found_page(status_code, opts[:include_ember] ? 'application' : 'no_ember')
error_page_opts[:layout] = opts[:include_ember] ? 'application' : 'no_ember'
render html: build_not_found_page(error_page_opts)
end
end
@@ -754,13 +753,13 @@ class ApplicationController < ActionController::Base
raise Discourse::ReadOnly.new if !(request.get? || request.head?) && @readonly_mode
end
def build_not_found_page(status = 404, layout = false)
def build_not_found_page(opts = {})
if SiteSetting.bootstrap_error_pages?
preload_json
layout = 'application' if layout == 'no_ember'
opts[:layout] = 'application' if opts[:layout] == 'no_ember'
end
if !SiteSetting.login_required? || current_user
if !SiteSetting.login_required? || (current_user rescue false)
key = "page_not_found_topics"
if @topics_partial = $redis.get(key)
@topics_partial = @topics_partial.html_safe
@@ -774,9 +773,12 @@ class ApplicationController < ActionController::Base
end
@container_class = "wrap not-found-container"
@slug = (params[:slug].presence || params[:id].presence || "").tr('-', '')
@title = opts[:title] || I18n.t("page_not_found.title")
@group = opts[:group]
@hide_search = true if SiteSetting.login_required
render_to_string status: status, layout: layout, formats: [:html], template: '/exceptions/not_found'
@slug = (params[:slug].presence || params[:id].presence || "").tr('-', ' ')
render_to_string status: opts[:status], layout: opts[:layout], formats: [:html], template: '/exceptions/not_found'
end
def is_asset_path

13
app/controllers/categories_controller.rb
View File

@@ -206,7 +206,18 @@ class CategoriesController < ApplicationController
def find_by_slug
params.require(:category_slug)
@category = Category.find_by_slug(params[:category_slug], params[:parent_category_slug])
guardian.ensure_can_see!(@category)
if !guardian.can_see?(@category)
if SiteSetting.detailed_404 && group = @category.access_category_via_group
raise Discourse::InvalidAccess.new(
'not in group',
@category,
custom_message: 'not_in_group.title_category',
group: group
)
else
raise Discourse::NotFound
end
end
@category.permission = CategoryGroup.permission_types[:full] if Category.topic_create_allowed(guardian).where(id: @category.id).exists?
render_serialized(@category, CategorySerializer)

9
app/controllers/exceptions_controller.rb
View File

@@ -2,7 +2,6 @@
class ExceptionsController < ApplicationController
skip_before_action :check_xhr, :preload_json
before_action :hide_search
def not_found
# centralize all rendering of 404 into app controller
@@ -11,13 +10,7 @@ class ExceptionsController < ApplicationController
# Give us an endpoint to use for 404 content in the ember app
def not_found_body
render html: build_not_found_page(200, false)
end
private
def hide_search
@hide_search = true if SiteSetting.login_required
render html: build_not_found_page(status: 200)
end
end

8
app/controllers/list_controller.rb
View File

@@ -368,7 +368,13 @@ class ListController < ApplicationController
raise Discourse::NotFound.new("category not found", check_permalinks: true) if !@category
@description_meta = @category.description_text
raise Discourse::NotFound unless guardian.can_see?(@category)
if !guardian.can_see?(@category)
if SiteSetting.detailed_404
raise Discourse::InvalidAccess
else
raise Discourse::NotFound
end
end
if use_crawler_layout?
@subcategories = @category.subcategories.select { |c| guardian.can_see?(c) }

80
app/controllers/topics_controller.rb
View File

@@ -79,12 +79,54 @@ class TopicsController < ApplicationController
begin
@topic_view = TopicView.new(params[:id] || params[:topic_id], current_user, opts)
rescue Discourse::NotFound
rescue Discourse::NotFound => ex
if params[:id]
topic = Topic.find_by(slug: params[:id].downcase)
return redirect_to_correct_topic(topic, opts[:post_number]) if topic
end
raise Discourse::NotFound
raise ex
rescue Discourse::NotLoggedIn => ex
if !SiteSetting.detailed_404
raise Discourse::NotFound
else
raise ex
end
rescue Discourse::InvalidAccess => ex
# If the user can't see the topic, clean up notifications for it.
Notification.remove_for(current_user.id, params[:topic_id]) if current_user
deleted = guardian.can_see_topic?(ex.obj, false) ||
(!guardian.can_see_topic?(ex.obj) &&
ex.obj&.access_topic_via_group &&
ex.obj.deleted_at)
if SiteSetting.detailed_404
if deleted
raise Discourse::NotFound.new(
'deleted topic',
custom_message: 'deleted_topic',
status: 410,
check_permalinks: true,
original_path: ex.obj.relative_url
)
elsif !guardian.can_see_topic?(ex.obj) && group = ex.obj&.access_topic_via_group
raise Discourse::InvalidAccess.new(
'not in group',
ex.obj,
custom_message: 'not_in_group.title_topic',
group: group
)
end
raise ex
else
raise Discourse::NotFound.new(
nil,
check_permalinks: deleted,
original_path: ex.obj.relative_url
)
end
end
page = params[:page]
@@ -120,27 +162,6 @@ class TopicsController < ApplicationController
end
perform_show_response
rescue Discourse::InvalidAccess => ex
if !guardian.can_see_topic?(ex.obj) && guardian.can_get_access_to_topic?(ex.obj)
return perform_hidden_topic_show_response(ex.obj)
end
if current_user
# If the user can't see the topic, clean up notifications for it.
Notification.remove_for(current_user.id, params[:topic_id])
end
if ex.obj && Topic === ex.obj && guardian.can_see_topic_if_not_deleted?(ex.obj)
raise Discourse::NotFound.new(
"topic was deleted",
status: 410,
check_permalinks: true,
original_path: ex.obj.relative_url
)
end
raise ex
end
def publish
@@ -960,19 +981,6 @@ class TopicsController < ApplicationController
end
end
def perform_hidden_topic_show_response(topic)
respond_to do |format|
format.html do
@topic_view = nil
render :show
end
format.json do
render_serialized(topic, HiddenTopicViewSerializer, root: false)
end
end
end
def render_topic_changes(dest_topic)
if dest_topic.present?
render json: { success: true, url: dest_topic.relative_url }

2
app/controllers/users_controller.rb
View File

@@ -606,8 +606,6 @@ class UsersController < ApplicationController
end
end
end
rescue RateLimiter::LimitExceeded => e
render_rate_limit_error(e)
rescue ::Webauthn::SecurityKeyError => err
render json: {
message: err.message,