Commit 551145bc authored by Robert Schilling's avatar Robert Schilling

Validate branch-names and references in WebUI, API

Add specs for GitRefValidator
parent 7a0e1c72
......@@ -17,9 +17,17 @@ class Projects::BranchesController < Projects::ApplicationController
end
def create
@branch = CreateBranchService.new.execute(project, params[:branch_name], params[:ref], current_user)
result = CreateBranchService.new.execute(project,
params[:branch_name],
params[:ref],
current_user)
if result[:status] == :success
@branch = result[:branch]
redirect_to project_tree_path(@project, @branch.name)
else
@error = result[:message]
render action: 'new'
end
end
def destroy
......
class CreateBranchService
def execute(project, branch_name, ref, current_user)
valid_branch = Gitlab::GitRefValidator.validate(branch_name)
if valid_branch == false
return error('Branch name invalid')
end
repository = project.repository
existing_branch = repository.find_branch(branch_name)
if existing_branch
return error('Branch already exists')
end
repository.add_branch(branch_name, ref)
new_branch = repository.find_branch(branch_name)
if new_branch
Event.create_ref_event(project, current_user, new_branch, 'add')
return success(new_branch)
else
return error('Invalid reference name')
end
end
def error(message)
{
message: message,
status: :error
}
end
new_branch
def success(branch)
{
branch: branch,
status: :success
}
end
end
......@@ -5,21 +5,21 @@ class DeleteBranchService
# No such branch
unless branch
return error('No such branch')
return error('No such branch', 404)
end
if branch_name == repository.root_ref
return error('Cannot remove HEAD branch')
return error('Cannot remove HEAD branch', 405)
end
# Dont allow remove of protected branch
if project.protected_branch?(branch_name)
return error('Protected branch cant be removed')
return error('Protected branch cant be removed', 405)
end
# Dont allow user to remove branch if he is not allowed to push
unless current_user.can?(:push_code, project)
return error('You dont have push access to repo')
return error('You dont have push access to repo', 405)
end
if repository.rm_branch(branch_name)
......@@ -30,9 +30,10 @@ class DeleteBranchService
end
end
def error(message)
def error(message, return_code = 400)
{
message: message,
return_code: return_code,
state: :error
}
end
......
- if @error
.alert.alert-danger
%button{ type: "button", class: "close", "data-dismiss" => "alert"} &times;
= @error
%h3.page-title
%i.icon-code-fork
New branch
......@@ -5,11 +9,11 @@
.form-group
= label_tag :branch_name, 'Name for new branch', class: 'control-label'
.col-sm-10
= text_field_tag :branch_name, nil, placeholder: 'enter new branch name', required: true, tabindex: 1, class: 'form-control'
= text_field_tag :branch_name, params[:branch_name], placeholder: 'enter new branch name', required: true, tabindex: 1, class: 'form-control'
.form-group
= label_tag :ref, 'Create from', class: 'control-label'
.col-sm-10
= text_field_tag :ref, nil, placeholder: 'existing branch name, tag or commit SHA', required: true, tabindex: 2, class: 'form-control'
= text_field_tag :ref, params[:ref], placeholder: 'existing branch name, tag or commit SHA', required: true, tabindex: 2, class: 'form-control'
.form-actions
= submit_tag 'Create branch', class: 'btn btn-create', tabindex: 3
= link_to 'Cancel', project_branches_path(@project), class: 'btn btn-cancel'
......
......@@ -196,6 +196,8 @@ Parameters:
}
```
It return 200 if succeed or 400 if failed with error message explaining reason.
## Delete repository branch
```
......@@ -207,4 +209,5 @@ Parameters:
- `id` (required) - The ID of a project
- `branch` (required) - The name of the branch
It return 200 if succeed or 405 if failed with error message explaining reason.
It return 200 if succeed, 404 if the branch to be deleted does not exist
or 400 for other reasons. In case of an error, an explaining message is provided.
......@@ -23,3 +23,21 @@ Feature: Project Browse branches
Given I visit project branches page
And I click branch 'improve/awesome' delete link
Then I should not see branch 'improve/awesome'
Scenario: I create a branch with invalid name
Given I visit project branches page
And I click new branch link
When I submit new branch form with invalid name
Then I should see new an error that branch is invalid
Scenario: I create a branch with invalid reference
Given I visit project branches page
And I click new branch link
When I submit new branch form with invalid reference
Then I should see new an error that ref is invalid
Scenario: I create a branch that already exists
Given I visit project branches page
And I click new branch link
When I submit new branch form with branch that already exists
Then I should see new an error that branch already exists
......@@ -38,10 +38,38 @@ class ProjectBrowseBranches < Spinach::FeatureSteps
click_button 'Create branch'
end
step 'I submit new branch form with invalid name' do
fill_in 'branch_name', with: '1.0 stable'
fill_in 'ref', with: 'master'
click_button 'Create branch'
end
step 'I submit new branch form with invalid reference' do
fill_in 'branch_name', with: 'foo'
fill_in 'ref', with: 'foo'
click_button 'Create branch'
end
step 'I submit new branch form with branch that already exists' do
fill_in 'branch_name', with: 'master'
fill_in 'ref', with: 'master'
click_button 'Create branch'
end
step 'I should see new branch created' do
within '.tree-ref-holder' do
page.should have_content 'deploy_keys'
end
step 'I should see new an error that branch is invalid' do
page.should have_content 'Branch name invalid'
end
step 'I should see new an error that ref is invalid' do
page.should have_content 'Invalid reference name'
end
step 'I should see new an error that branch already exists' do
page.should have_content 'Branch already exists'
end
step "I click branch 'improve/awesome' delete link" do
......
......@@ -80,9 +80,17 @@ module API
# POST /projects/:id/repository/branches
post ":id/repository/branches" do
authorize_push_project
@branch = CreateBranchService.new.execute(user_project, params[:branch_name], params[:ref], current_user)
present @branch, with: Entities::RepoObject, project: user_project
result = CreateBranchService.new.execute(user_project,
params[:branch_name],
params[:ref],
current_user)
if result[:status] == :success
present result[:branch],
with: Entities::RepoObject,
project: user_project
else
render_api_error!(result[:message], 400)
end
end
# Delete branch
......@@ -99,7 +107,7 @@ module API
if result[:state] == :success
true
else
render_api_error!(result[:message], 405)
render_api_error!(result[:message], result[:return_code])
end
end
end
......
module Gitlab
module GitRefValidator
extend self
# Validates a given name against the git reference specification
#
# Returns true for a valid reference name, false otherwise
def validate(ref_name)
system *%W(git check-ref-format refs/#{ref_name})
end
end
end
require 'spec_helper'
describe Gitlab::GitRefValidator do
it { Gitlab::GitRefValidator.validate('feature/new').should be_true }
it { Gitlab::GitRefValidator.validate('implement_@all').should be_true }
it { Gitlab::GitRefValidator.validate('my_new_feature').should be_true }
it { Gitlab::GitRefValidator.validate('#1').should be_true }
it { Gitlab::GitRefValidator.validate('feature/~new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/^new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/:new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/?new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/*new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/[new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/new.').should be_false }
it { Gitlab::GitRefValidator.validate('feature\@{').should be_false }
it { Gitlab::GitRefValidator.validate('feature\new').should be_false }
it { Gitlab::GitRefValidator.validate('feature//new').should be_false }
it { Gitlab::GitRefValidator.validate('feature new').should be_false }
end
......@@ -94,12 +94,12 @@ describe API::API, api: true do
describe "POST /projects/:id/repository/branches" do
it "should create a new branch" do
post api("/projects/#{project.id}/repository/branches", user),
branch_name: branch_name,
branch_name: 'feature1',
ref: branch_sha
response.status.should == 201
json_response['name'].should == branch_name
json_response['name'].should == 'feature1'
json_response['commit']['id'].should == branch_sha
end
......@@ -107,9 +107,37 @@ describe API::API, api: true do
post api("/projects/#{project.id}/repository/branches", user2),
branch_name: branch_name,
ref: branch_sha
response.status.should == 403
end
it 'should return 400 if branch name is invalid' do
post api("/projects/#{project.id}/repository/branches", user),
branch_name: 'new design',
ref: branch_sha
response.status.should == 400
json_response['message'].should == 'Branch name invalid'
end
it 'should return 400 if branch already exists' do
post api("/projects/#{project.id}/repository/branches", user),
branch_name: 'new_design1',
ref: branch_sha
response.status.should == 201
post api("/projects/#{project.id}/repository/branches", user),
branch_name: 'new_design1',
ref: branch_sha
response.status.should == 400
json_response['message'].should == 'Branch already exists'
end
it 'should return 400 if ref name is invalid' do
post api("/projects/#{project.id}/repository/branches", user),
branch_name: 'new_design3',
ref: 'foo'
response.status.should == 400
json_response['message'].should == 'Invalid reference name'
end
end
describe "DELETE /projects/:id/repository/branches/:branch" do
......@@ -120,6 +148,11 @@ describe API::API, api: true do
response.status.should == 200
end
it 'should return 404 if branch not exists' do
delete api("/projects/#{project.id}/repository/branches/foobar", user)
response.status.should == 404
end
it "should remove protected branch" do
project.protected_branches.create(name: branch_name)
delete api("/projects/#{project.id}/repository/branches/#{branch_name}", user)
......
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