Commit 6afe9191 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'fix-leading-slash-in-redirects-plus-rubocop' into 'master'

Fix leading slash in redirects and add cop

Closes #37423 and #50645

See merge request gitlab-org/gitlab-ce!21828
parents f9320089 0896d694
---
title: Fix leading slash in redirects and add rubocop cop
merge_request: 21828
author: Sanad Liaquat
type: fixed
# frozen_string_literal: true # frozen_string_literal: true
namespace :instance_statistics do namespace :instance_statistics do
root to: redirect('/-/instance_statistics/conversational_development_index') root to: redirect('-/instance_statistics/conversational_development_index')
resources :cohorts, only: :index resources :cohorts, only: :index
resources :conversational_development_index, only: :index resources :conversational_development_index, only: :index
......
...@@ -2,7 +2,7 @@ scope(controller: :wikis) do ...@@ -2,7 +2,7 @@ scope(controller: :wikis) do
scope(path: 'wikis', as: :wikis) do scope(path: 'wikis', as: :wikis) do
get :git_access get :git_access
get :pages get :pages
get '/', to: redirect('/%{namespace_id}/%{project_id}/wikis/home') get '/', to: redirect('%{namespace_id}/%{project_id}/wikis/home')
post '/', to: 'wikis#create' post '/', to: 'wikis#create'
end end
......
# frozen_string_literal: true
module RuboCop
module Cop
# Checks for a leading '/' in route redirects
# For more information see: https://gitlab.com/gitlab-org/gitlab-ce/issues/50645
#
# @example
# # bad
# root to: redirect('/-/instance/statistics/conversational_development_index')
#
# # good
# root to: redirect('-/instance/statistics/conversational_development_index')
#
class AvoidRouteRedirectLeadingSlash < RuboCop::Cop::Cop
MSG = 'Do not use a leading "/" in route redirects'
def_node_matcher :leading_slash_in_redirect?, <<~PATTERN
(send nil? :redirect (str #has_leading_slash?))
PATTERN
def on_send(node)
return unless in_routes?(node)
return unless leading_slash_in_redirect?(node)
add_offense(node)
end
def has_leading_slash?(str)
str.start_with?("/")
end
def in_routes?(node)
path = node.location.expression.source_buffer.name
dirname = File.dirname(path)
filename = File.basename(path)
dirname.end_with?('config/routes') || filename.end_with?('routes.rb')
end
def autocorrect(node)
lambda do |corrector|
corrector.replace(node.loc.expression, remove_leading_slash(node))
end
end
def remove_leading_slash(node)
node.source.sub('/', '')
end
end
end
end
...@@ -7,6 +7,7 @@ require_relative 'cop/gitlab/union' ...@@ -7,6 +7,7 @@ require_relative 'cop/gitlab/union'
require_relative 'cop/include_sidekiq_worker' require_relative 'cop/include_sidekiq_worker'
require_relative 'cop/avoid_return_from_blocks' require_relative 'cop/avoid_return_from_blocks'
require_relative 'cop/avoid_break_from_strong_memoize' require_relative 'cop/avoid_break_from_strong_memoize'
require_relative 'cop/avoid_route_redirect_leading_slash'
require_relative 'cop/line_break_around_conditional_block' require_relative 'cop/line_break_around_conditional_block'
require_relative 'cop/prefer_class_methods_over_module' require_relative 'cop/prefer_class_methods_over_module'
require_relative 'cop/migration/add_column' require_relative 'cop/migration/add_column'
......
# frozen_string_literal: true
require 'spec_helper'
require 'rubocop'
require_relative '../../../rubocop/cop/avoid_route_redirect_leading_slash'
describe RuboCop::Cop::AvoidRouteRedirectLeadingSlash do
include CopHelper
subject(:cop) { described_class.new }
before do
allow(cop).to receive(:in_routes?).and_return(true)
end
it 'registers an offense when redirect has a leading slash' do
expect_offense(<<~PATTERN.strip_indent)
root to: redirect("/-/route")
^^^^^^^^^^^^^^^^^^^^ Do not use a leading "/" in route redirects
PATTERN
end
it 'does not register an offense when redirect does not have a leading slash' do
expect_no_offenses(<<~PATTERN.strip_indent)
root to: redirect("-/route")
PATTERN
end
it 'autocorrect `/-/route` to `-/route`' do
expect(autocorrect_source('redirect("/-/route")')).to eq('redirect("-/route")')
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment