Skip to content

Commit

Permalink
Fix unmactched routes catch
Browse files Browse the repository at this point in the history
  • Loading branch information
sfate committed Feb 16, 2017
1 parent 9d00f6c commit 87ae851
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 20 deletions.
26 changes: 20 additions & 6 deletions app/controllers/concerns/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,31 @@ module Errors
rescue_from AbstractController::ActionNotFound, with: -> { render_error(code: 404) }
end

# NOTE: Alexey Bobyrev 14 Jan 2017
# Raise error here to make it visible for application#rescue_from
# Ref: https://github.com/rails/rails/issues/671
def raise_routing_error
raise ActionController::RoutingError.new(params[:path])
def non_matching_request
path = params.fetch(:path, '')
format = params.fetch(:format, '')

if path.match(/\.php$/) || format.eql?('php')
head :not_acceptable, layout: false
elsif
# TODO: Alexey Bobyrev 14 Feb 2017
# Remove this condition as we have below block to cover it
data = request.env.select {|key,_| key.upcase == key }
NewRelic::Agent.record_custom_event('UndefinedURL', data)

raise ActionController::RoutingError.new(path)
else
# NOTE: Alexey Bobyrev 14 Jan 2017
# Raise error here to make it visible for application#rescue_from
# Ref: https://github.com/rails/rails/issues/671
raise ActionController::RoutingError.new(path)
end
end

protected

def render_error(code:)
if Rails.application.config.consider_all_requests_local
unless Rails.application.config.consider_all_requests_local
raise
else
respond_to do |format|
Expand Down
10 changes: 6 additions & 4 deletions config/routes.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
FameAndPartners::Application.routes.draw do

############################
# Devise Omniauth Workaround
############################
Expand Down Expand Up @@ -605,9 +604,12 @@
mount Revolution::Engine => '/'

mount WeddingAtelier::Engine, at: '/wedding-atelier'
end

match '*path', to: 'errors/invalid_format#capture_php', constraints: lambda { |request| request.path[/\.php$/] }
# NOTE: Alexey Bobyrev 14 Feb 2017
# Method append used here to handle all request directly right after defined ones (including engines)
FameAndPartners::Application.routes.append do
# NOTE: Alexey Bobyrev 14 Jan 2017
# Any other routes are handled here (as ActionDispatch prevents RoutingError from hitting ApplicationController::rescue_action)
match '*path', to: 'application#raise_routing_error'
# Any other routes are handled here (as ActionDispatch prevents RoutingError from hitting ApplicationController#rescue_action)
match '*path', to: 'application#non_matching_request', as: 'routing_error'
end
20 changes: 10 additions & 10 deletions spec/routing/not_found_routes_spec.rb
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
require 'spec_helper'

describe 'Not found requests', type: :routing do
describe 'Not found requests', type: :request do

describe 'with *.php' do
let(:controller) { {controller: 'errors/invalid_format', action: 'capture_php', path: 'abc', format: 'php'} }
it 'routes to application#non_matching_request' do
get '/abc.php'

it 'routes to errors/invalid_format#capture_php' do
expect(get: '/abc.php').to route_to(controller)
expect(post: '/abc.php').to route_to(controller)
expect(response).to render_template(nil)
expect(response).to have_http_status(406)
end
end

describe 'with any non-existent page' do
let(:controller) { {controller: 'application', action: 'raise_routing_error', path: 'abc'} }
it 'rotes to application#non_matching_request' do
get '/abc'

it 'rotes to application#raise_routing_error' do
expect(get: '/abc').to route_to(controller)
expect(post: '/abc').to route_to(controller)
expect(get: '/abc.js').to route_to(controller.merge(format: 'js'))
expect(response).to render_template('error/404')
expect(response).to have_http_status(404)
end
end

Expand Down

0 comments on commit 87ae851

Please sign in to comment.