Commit 05c988419f438ec72556f3a52227c62943c67845

Authored by Peter Fern
1 parent 4ab27017

Expand refs constraints to include valid characters

Fixes #4831, #4865, #4932
config/routes.rb
... ... @@ -222,14 +222,14 @@ Gitlab::Application.routes.draw do
222 222 end
223 223 end
224 224  
225   - resources :branches, only: [:index, :new, :create, :destroy], constraints: { id: /[a-zA-Z.\/0-9_\-#%+]+/ } do
  225 + resources :branches, only: [:index, :new, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex } do
226 226 collection do
227   - get :recent
  227 + get :recent, constraints: { id: Gitlab::Regex.git_reference_regex }
228 228 end
229 229 end
230 230  
231   - resources :tags, only: [:index, :new, :create, :destroy], constraints: { id: /[a-zA-Z.\/0-9_\-#%+]+/ }
232   - resources :protected_branches, only: [:index, :create, :destroy], constraints: { id: /[a-zA-Z.\/0-9_\-#%+]+/ }
  231 + resources :tags, only: [:index, :new, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex }
  232 + resources :protected_branches, only: [:index, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex }
233 233  
234 234 resources :refs, only: [] do
235 235 collection do
... ... @@ -238,11 +238,11 @@ Gitlab::Application.routes.draw do
238 238  
239 239 member do
240 240 # tree viewer logs
241   - get "logs_tree", constraints: { id: /[a-zA-Z.\/0-9_\-#%+]+/ }
  241 + get "logs_tree", constraints: { id: Gitlab::Regex.git_reference_regex }
242 242 get "logs_tree/:path" => "refs#logs_tree",
243 243 as: :logs_file,
244 244 constraints: {
245   - id: /[a-zA-Z.0-9\/_\-#%+]+/,
  245 + id: Gitlab::Regex.git_reference_regex,
246 246 path: /.*/
247 247 }
248 248 end
... ...
lib/gitlab/regex.rb
... ... @@ -18,6 +18,29 @@ module Gitlab
18 18 default_regex
19 19 end
20 20  
  21 + def git_reference_regex
  22 + # Valid git ref regex, see:
  23 + # https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html
  24 +
  25 + %r{
  26 + (?!
  27 + # doesn't begins with
  28 + \/| # (rule #6)
  29 + # doesn't contain
  30 + .*(?:
  31 + [\/.]\.| # (rule #1,3)
  32 + \/\/| # (rule #6)
  33 + @\{| # (rule #8)
  34 + \\ # (rule #9)
  35 + )
  36 + )
  37 + [^\000-\040\177~^:?*\[]+ # (rule #4-5)
  38 + # doesn't end with
  39 + (?<!\.lock) # (rule #1)
  40 + (?<![\/.]) # (rule #6-7)
  41 + }x
  42 + end
  43 +
21 44 protected
22 45  
23 46 def default_regex
... ...
spec/routing/project_routing_spec.rb
... ... @@ -138,12 +138,24 @@ end
138 138 describe Projects::BranchesController, "routing" do
139 139 it "to #branches" do
140 140 get("/gitlab/gitlabhq/branches").should route_to('projects/branches#index', project_id: 'gitlab/gitlabhq')
  141 + delete("/gitlab/gitlabhq/branches/feature%2345").should route_to('projects/branches#destroy', project_id: 'gitlab/gitlabhq', id: 'feature#45')
  142 + delete("/gitlab/gitlabhq/branches/feature%2B45").should route_to('projects/branches#destroy', project_id: 'gitlab/gitlabhq', id: 'feature+45')
  143 + delete("/gitlab/gitlabhq/branches/feature@45").should route_to('projects/branches#destroy', project_id: 'gitlab/gitlabhq', id: 'feature@45')
  144 + delete("/gitlab/gitlabhq/branches/feature%2345/foo/bar/baz").should route_to('projects/branches#destroy', project_id: 'gitlab/gitlabhq', id: 'feature#45/foo/bar/baz')
  145 + delete("/gitlab/gitlabhq/branches/feature%2B45/foo/bar/baz").should route_to('projects/branches#destroy', project_id: 'gitlab/gitlabhq', id: 'feature+45/foo/bar/baz')
  146 + delete("/gitlab/gitlabhq/branches/feature@45/foo/bar/baz").should route_to('projects/branches#destroy', project_id: 'gitlab/gitlabhq', id: 'feature@45/foo/bar/baz')
141 147 end
142 148 end
143 149  
144 150 describe Projects::TagsController, "routing" do
145 151 it "to #tags" do
146 152 get("/gitlab/gitlabhq/tags").should route_to('projects/tags#index', project_id: 'gitlab/gitlabhq')
  153 + delete("/gitlab/gitlabhq/tags/feature%2345").should route_to('projects/tags#destroy', project_id: 'gitlab/gitlabhq', id: 'feature#45')
  154 + delete("/gitlab/gitlabhq/tags/feature%2B45").should route_to('projects/tags#destroy', project_id: 'gitlab/gitlabhq', id: 'feature+45')
  155 + delete("/gitlab/gitlabhq/tags/feature@45").should route_to('projects/tags#destroy', project_id: 'gitlab/gitlabhq', id: 'feature@45')
  156 + delete("/gitlab/gitlabhq/tags/feature%2345/foo/bar/baz").should route_to('projects/tags#destroy', project_id: 'gitlab/gitlabhq', id: 'feature#45/foo/bar/baz')
  157 + delete("/gitlab/gitlabhq/tags/feature%2B45/foo/bar/baz").should route_to('projects/tags#destroy', project_id: 'gitlab/gitlabhq', id: 'feature+45/foo/bar/baz')
  158 + delete("/gitlab/gitlabhq/tags/feature@45/foo/bar/baz").should route_to('projects/tags#destroy', project_id: 'gitlab/gitlabhq', id: 'feature@45/foo/bar/baz')
147 159 end
148 160 end
149 161  
... ... @@ -183,9 +195,11 @@ describe Projects::RefsController, &quot;routing&quot; do
183 195 get("/gitlab/gitlabhq/refs/stable/logs_tree").should route_to('projects/refs#logs_tree', project_id: 'gitlab/gitlabhq', id: 'stable')
184 196 get("/gitlab/gitlabhq/refs/feature%2345/logs_tree").should route_to('projects/refs#logs_tree', project_id: 'gitlab/gitlabhq', id: 'feature#45')
185 197 get("/gitlab/gitlabhq/refs/feature%2B45/logs_tree").should route_to('projects/refs#logs_tree', project_id: 'gitlab/gitlabhq', id: 'feature+45')
  198 + get("/gitlab/gitlabhq/refs/feature@45/logs_tree").should route_to('projects/refs#logs_tree', project_id: 'gitlab/gitlabhq', id: 'feature@45')
186 199 get("/gitlab/gitlabhq/refs/stable/logs_tree/foo/bar/baz").should route_to('projects/refs#logs_tree', project_id: 'gitlab/gitlabhq', id: 'stable', path: 'foo/bar/baz')
187 200 get("/gitlab/gitlabhq/refs/feature%2345/logs_tree/foo/bar/baz").should route_to('projects/refs#logs_tree', project_id: 'gitlab/gitlabhq', id: 'feature#45', path: 'foo/bar/baz')
188 201 get("/gitlab/gitlabhq/refs/feature%2B45/logs_tree/foo/bar/baz").should route_to('projects/refs#logs_tree', project_id: 'gitlab/gitlabhq', id: 'feature+45', path: 'foo/bar/baz')
  202 + get("/gitlab/gitlabhq/refs/feature@45/logs_tree/foo/bar/baz").should route_to('projects/refs#logs_tree', project_id: 'gitlab/gitlabhq', id: 'feature@45', path: 'foo/bar/baz')
189 203 get("/gitlab/gitlabhq/refs/stable/logs_tree/files.scss").should route_to('projects/refs#logs_tree', project_id: 'gitlab/gitlabhq', id: 'stable', path: 'files.scss')
190 204 end
191 205 end
... ...