Commit eddf3feb authored by blackst0ne's avatar blackst0ne

[Rails5] Add `safe_params` helper

Rails 5.0 requires to explicitly permit attributes when building a URL
using current `params` object.

The `safe_params` helper allows developers to just call `safe_params.merge(...)`
instead of manually adding `permit` to every call.

https://github.com/rails/rails/pull/20868
parent dd552d06
...@@ -5,6 +5,7 @@ class ApplicationController < ActionController::Base ...@@ -5,6 +5,7 @@ class ApplicationController < ActionController::Base
include Gitlab::GonHelper include Gitlab::GonHelper
include GitlabRoutingHelper include GitlabRoutingHelper
include PageLayoutHelper include PageLayoutHelper
include SafeParamsHelper
include SentryHelper include SentryHelper
include WorkhorseHelper include WorkhorseHelper
include EnforcesTwoFactorAuthentication include EnforcesTwoFactorAuthentication
......
...@@ -86,7 +86,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController ...@@ -86,7 +86,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
out_of_range = todos.current_page > total_pages out_of_range = todos.current_page > total_pages
if out_of_range if out_of_range
redirect_to url_for(params.merge(page: total_pages, only_path: true)) redirect_to url_for(safe_params.merge(page: total_pages, only_path: true))
end end
out_of_range out_of_range
......
...@@ -187,6 +187,6 @@ class GroupsController < Groups::ApplicationController ...@@ -187,6 +187,6 @@ class GroupsController < Groups::ApplicationController
params[:id] = group.to_param params[:id] = group.to_param
url_for(params) url_for(safe_params)
end end
end end
...@@ -404,7 +404,7 @@ class ProjectsController < Projects::ApplicationController ...@@ -404,7 +404,7 @@ class ProjectsController < Projects::ApplicationController
params[:namespace_id] = project.namespace.to_param params[:namespace_id] = project.namespace.to_param
params[:id] = project.to_param params[:id] = project.to_param
url_for(params) url_for(safe_params)
end end
def project_export_enabled def project_export_enabled
......
...@@ -146,6 +146,6 @@ class UsersController < ApplicationController ...@@ -146,6 +146,6 @@ class UsersController < ApplicationController
end end
def build_canonical_path(user) def build_canonical_path(user)
url_for(params.merge(username: user.to_param)) url_for(safe_params.merge(username: user.to_param))
end end
end end
...@@ -259,7 +259,7 @@ module BlobHelper ...@@ -259,7 +259,7 @@ module BlobHelper
options = [] options = []
if error == :collapsed if error == :collapsed
options << link_to('load it anyway', url_for(params.merge(viewer: viewer.type, expanded: true, format: nil))) options << link_to('load it anyway', url_for(safe_params.merge(viewer: viewer.type, expanded: true, format: nil)))
end end
# If the error is `:server_side_but_stored_externally`, the simple viewer will show the same error, # If the error is `:server_side_but_stored_externally`, the simple viewer will show the same error,
......
...@@ -180,7 +180,7 @@ module DiffHelper ...@@ -180,7 +180,7 @@ module DiffHelper
private private
def diff_btn(title, name, selected) def diff_btn(title, name, selected)
params_copy = params.dup params_copy = safe_params.dup
params_copy[:view] = name params_copy[:view] = name
# Always use HTML to handle case where JSON diff rendered this button # Always use HTML to handle case where JSON diff rendered this button
......
module SafeParamsHelper
# Rails 5.0 requires to permit parameters if used in url helpers.
# User this helper when generating links with `params.merge(...)`
def safe_params
if params.respond_to?(:permit!)
params.except(:host, :port, :protocol).permit!
else
params
end
end
end
xml.title "#{current_user.name} issues" xml.title "#{current_user.name} issues"
xml.link href: url_for(params), rel: "self", type: "application/atom+xml" xml.link href: url_for(safe_params), rel: "self", type: "application/atom+xml"
xml.link href: issues_dashboard_url, rel: "alternate", type: "text/html" xml.link href: issues_dashboard_url, rel: "alternate", type: "text/html"
xml.id issues_dashboard_url xml.id issues_dashboard_url
xml.updated @issues.first.updated_at.xmlschema if @issues.reorder(nil).any? xml.updated @issues.first.updated_at.xmlschema if @issues.reorder(nil).any?
......
...@@ -2,12 +2,12 @@ ...@@ -2,12 +2,12 @@
- page_title _("Issues") - page_title _("Issues")
- @breadcrumb_link = issues_dashboard_path(assignee_id: current_user.id) - @breadcrumb_link = issues_dashboard_path(assignee_id: current_user.id)
= content_for :meta_tags do = content_for :meta_tags do
= auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{current_user.name} issues") = auto_discovery_link_tag(:atom, safe_params.merge(rss_url_options).to_h, title: "#{current_user.name} issues")
.top-area .top-area
= render 'shared/issuable/nav', type: :issues, display_count: !@no_filters_set = render 'shared/issuable/nav', type: :issues, display_count: !@no_filters_set
.nav-controls .nav-controls
= link_to params.merge(rss_url_options), class: 'btn has-tooltip', data: { container: 'body' }, title: 'Subscribe' do = link_to safe_params.merge(rss_url_options), class: 'btn has-tooltip', data: { container: 'body' }, title: 'Subscribe' do
= icon('rss') = icon('rss')
= render 'shared/new_project_item_select', path: 'issues/new', label: "New issue", with_feature_enabled: 'issues', type: :issues = render 'shared/new_project_item_select', path: 'issues/new', label: "New issue", with_feature_enabled: 'issues', type: :issues
......
xml.title "#{@group.name} issues" xml.title "#{@group.name} issues"
xml.link href: url_for(params), rel: "self", type: "application/atom+xml" xml.link href: url_for(safe_params), rel: "self", type: "application/atom+xml"
xml.link href: issues_group_url, rel: "alternate", type: "text/html" xml.link href: issues_group_url, rel: "alternate", type: "text/html"
xml.id issues_group_url xml.id issues_group_url
xml.updated @issues.first.updated_at.xmlschema if @issues.reorder(nil).any? xml.updated @issues.first.updated_at.xmlschema if @issues.reorder(nil).any?
......
- page_title "Issues" - page_title "Issues"
= content_for :meta_tags do = content_for :meta_tags do
= auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{@group.name} issues") = auto_discovery_link_tag(:atom, safe_params.merge(rss_url_options).to_h, title: "#{@group.name} issues")
- if group_issues_count(state: 'all').zero? - if group_issues_count(state: 'all').zero?
= render 'shared/empty_states/issues', project_select_button: true = render 'shared/empty_states/issues', project_select_button: true
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
- rich_type = viewer.type == :rich ? viewer.partial_name : nil - rich_type = viewer.type == :rich ? viewer.partial_name : nil
- load_async = local_assigns.fetch(:load_async, viewer.load_async? && render_error.nil?) - load_async = local_assigns.fetch(:load_async, viewer.load_async? && render_error.nil?)
- viewer_url = local_assigns.fetch(:viewer_url) { url_for(params.merge(viewer: viewer.type, format: :json)) } if load_async - viewer_url = local_assigns.fetch(:viewer_url) { url_for(safe_params.merge(viewer: viewer.type, format: :json)) } if load_async
.blob-viewer{ data: { type: viewer.type, rich_type: rich_type, url: viewer_url }, class: ('hidden' if hidden) } .blob-viewer{ data: { type: viewer.type, rich_type: rich_type, url: viewer_url }, class: ('hidden' if hidden) }
- if render_error - if render_error
= render 'projects/blob/render_error', viewer: viewer = render 'projects/blob/render_error', viewer: viewer
......
- diff_file = viewer.diff_file - diff_file = viewer.diff_file
- url = url_for(params.merge(action: :diff_for_path, old_path: diff_file.old_path, new_path: diff_file.new_path, file_identifier: diff_file.file_identifier)) - url = url_for(safe_params.merge(action: :diff_for_path, old_path: diff_file.old_path, new_path: diff_file.new_path, file_identifier: diff_file.file_identifier))
.nothing-here-block.diff-collapsed{ data: { diff_for_path: url } } .nothing-here-block.diff-collapsed{ data: { diff_for_path: url } }
This diff is collapsed. This diff is collapsed.
%a.click-to-expand Click to expand it. %a.click-to-expand Click to expand it.
= link_to params.merge(rss_url_options), class: 'btn btn-default append-right-10 has-tooltip', title: 'Subscribe' do = link_to safe_params.merge(rss_url_options), class: 'btn btn-default append-right-10 has-tooltip', title: 'Subscribe' do
= icon('rss') = icon('rss')
- if @can_bulk_update - if @can_bulk_update
= button_tag "Edit issues", class: "btn btn-default append-right-10 js-bulk-update-toggle" = button_tag "Edit issues", class: "btn btn-default append-right-10 js-bulk-update-toggle"
......
xml.title "#{@project.name} issues" xml.title "#{@project.name} issues"
xml.link href: url_for(params), rel: "self", type: "application/atom+xml" xml.link href: url_for(safe_params), rel: "self", type: "application/atom+xml"
xml.link href: project_issues_url(@project), rel: "alternate", type: "text/html" xml.link href: project_issues_url(@project), rel: "alternate", type: "text/html"
xml.id project_issues_url(@project) xml.id project_issues_url(@project)
xml.updated @issues.first.updated_at.xmlschema if @issues.reorder(nil).any? xml.updated @issues.first.updated_at.xmlschema if @issues.reorder(nil).any?
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
- new_issue_email = @project.new_issuable_address(current_user, 'issue') - new_issue_email = @project.new_issuable_address(current_user, 'issue')
= content_for :meta_tags do = content_for :meta_tags do
= auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{@project.name} issues") = auto_discovery_link_tag(:atom, safe_params.merge(rss_url_options).to_h, title: "#{@project.name} issues")
- if project_issues(@project).exists? - if project_issues(@project).exists?
%div{ class: (container_class) } %div{ class: (container_class) }
......
...@@ -26,16 +26,16 @@ ...@@ -26,16 +26,16 @@
- else - else
%ul.merge-request-tabs.nav-links.no-top.no-bottom %ul.merge-request-tabs.nav-links.no-top.no-bottom
%li.commits-tab.active %li.commits-tab.active
= link_to url_for(params), data: {target: 'div#commits', action: 'new', toggle: 'tab'} do = link_to url_for(safe_params), data: {target: 'div#commits', action: 'new', toggle: 'tab'} do
Commits Commits
%span.badge= @commits.size %span.badge= @commits.size
- if @pipelines.any? - if @pipelines.any?
%li.builds-tab %li.builds-tab
= link_to url_for(params.merge(action: 'pipelines')), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tab'} do = link_to url_for(safe_params.merge(action: 'pipelines')), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tab'} do
Pipelines Pipelines
%span.badge= @pipelines.size %span.badge= @pipelines.size
%li.diffs-tab %li.diffs-tab
= link_to url_for(params.merge(action: 'diffs')), data: {target: 'div#diffs', action: 'diffs', toggle: 'tab'} do = link_to url_for(safe_params.merge(action: 'diffs')), data: {target: 'div#diffs', action: 'diffs', toggle: 'tab'} do
Changes Changes
%span.badge= @merge_request.diff_size %span.badge= @merge_request.diff_size
...@@ -46,7 +46,7 @@ ...@@ -46,7 +46,7 @@
-# This tab is always loaded via AJAX -# This tab is always loaded via AJAX
- if @pipelines.any? - if @pipelines.any?
#pipelines.pipelines.tab-pane #pipelines.pipelines.tab-pane
= render 'projects/merge_requests/pipelines', endpoint: url_for(params.merge(action: 'pipelines', format: :json)), disable_initialization: true = render 'projects/merge_requests/pipelines', endpoint: url_for(safe_params.merge(action: 'pipelines', format: :json)), disable_initialization: true
.mr-loading-status .mr-loading-status
= spinner = spinner
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