Commit 3bab1bd4c162239dcf582a82360cf94d4953ef69
1 parent
33eae334
Exists in
master
and in
4 other branches
Improve consistency: use file_path for API create/update/delete files
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Showing
6 changed files
with
18 additions
and
18 deletions
Show diff stats
app/contexts/files/create_context.rb
| @@ -15,18 +15,13 @@ module Files | @@ -15,18 +15,13 @@ module Files | ||
| 15 | return error("You can only create files if you are on top of a branch") | 15 | return error("You can only create files if you are on top of a branch") |
| 16 | end | 16 | end |
| 17 | 17 | ||
| 18 | - file_name = params[:file_name] | 18 | + file_name = File.basename(path) |
| 19 | + file_path = path | ||
| 19 | 20 | ||
| 20 | unless file_name =~ Gitlab::Regex.path_regex | 21 | unless file_name =~ Gitlab::Regex.path_regex |
| 21 | return error("Your changes could not be commited, because file name contains not allowed characters") | 22 | return error("Your changes could not be commited, because file name contains not allowed characters") |
| 22 | end | 23 | end |
| 23 | 24 | ||
| 24 | - file_path = if path.blank? | ||
| 25 | - file_name | ||
| 26 | - else | ||
| 27 | - File.join(path, file_name) | ||
| 28 | - end | ||
| 29 | - | ||
| 30 | blob = repository.blob_at(ref, file_path) | 25 | blob = repository.blob_at(ref, file_path) |
| 31 | 26 | ||
| 32 | if blob | 27 | if blob |
app/controllers/projects/new_tree_controller.rb
| @@ -5,11 +5,12 @@ class Projects::NewTreeController < Projects::BaseTreeController | @@ -5,11 +5,12 @@ class Projects::NewTreeController < Projects::BaseTreeController | ||
| 5 | end | 5 | end |
| 6 | 6 | ||
| 7 | def update | 7 | def update |
| 8 | - result = Files::CreateContext.new(@project, current_user, params, @ref, @path).execute | 8 | + file_path = File.join(@path, File.basename(params[:file_name])) |
| 9 | + result = Files::CreateContext.new(@project, current_user, params, @ref, file_path).execute | ||
| 9 | 10 | ||
| 10 | if result[:status] == :success | 11 | if result[:status] == :success |
| 11 | flash[:notice] = "Your changes have been successfully commited" | 12 | flash[:notice] = "Your changes have been successfully commited" |
| 12 | - redirect_to project_blob_path(@project, File.join(@id, params[:file_name])) | 13 | + redirect_to project_blob_path(@project, File.join(@ref, file_path)) |
| 13 | else | 14 | else |
| 14 | flash[:alert] = result[:error] | 15 | flash[:alert] = result[:error] |
| 15 | render :show | 16 | render :show |
doc/api/repositories.md
| @@ -379,8 +379,7 @@ POST /projects/:id/repository/files | @@ -379,8 +379,7 @@ POST /projects/:id/repository/files | ||
| 379 | 379 | ||
| 380 | Parameters: | 380 | Parameters: |
| 381 | 381 | ||
| 382 | -+ `file_name` (required) - The name of new file. Ex. class.rb | ||
| 383 | -+ `file_path` (optional) - The path to new file. Ex. lib/ | 382 | ++ `file_path` (optional) - Full path to new file. Ex. lib/class.rb |
| 384 | + `branch_name` (required) - The name of branch | 383 | + `branch_name` (required) - The name of branch |
| 385 | + `content` (required) - File content | 384 | + `content` (required) - File content |
| 386 | + `commit_message` (required) - Commit message | 385 | + `commit_message` (required) - Commit message |
lib/api/files.rb
| @@ -8,8 +8,7 @@ module API | @@ -8,8 +8,7 @@ module API | ||
| 8 | # Create new file in repository | 8 | # Create new file in repository |
| 9 | # | 9 | # |
| 10 | # Parameters: | 10 | # Parameters: |
| 11 | - # file_name (required) - The name of new file. Ex. class.rb | ||
| 12 | - # file_path (optional) - The path to new file. Ex. lib/ | 11 | + # file_path (optional) - The path to new file. Ex. lib/class.rb |
| 13 | # branch_name (required) - The name of branch | 12 | # branch_name (required) - The name of branch |
| 14 | # content (required) - File content | 13 | # content (required) - File content |
| 15 | # commit_message (required) - Commit message | 14 | # commit_message (required) - Commit message |
| @@ -18,8 +17,8 @@ module API | @@ -18,8 +17,8 @@ module API | ||
| 18 | # POST /projects/:id/repository/files | 17 | # POST /projects/:id/repository/files |
| 19 | # | 18 | # |
| 20 | post ":id/repository/files" do | 19 | post ":id/repository/files" do |
| 21 | - required_attributes! [:file_name, :branch_name, :content, :commit_message] | ||
| 22 | - attrs = attributes_for_keys [:file_name, :file_path, :branch_name, :content, :commit_message] | 20 | + required_attributes! [:file_path, :branch_name, :content, :commit_message] |
| 21 | + attrs = attributes_for_keys [:file_path, :branch_name, :content, :commit_message] | ||
| 23 | branch_name = attrs.delete(:branch_name) | 22 | branch_name = attrs.delete(:branch_name) |
| 24 | file_path = attrs.delete(:file_path) | 23 | file_path = attrs.delete(:file_path) |
| 25 | result = ::Files::CreateContext.new(user_project, current_user, attrs, branch_name, file_path).execute | 24 | result = ::Files::CreateContext.new(user_project, current_user, attrs, branch_name, file_path).execute |
| @@ -28,7 +27,6 @@ module API | @@ -28,7 +27,6 @@ module API | ||
| 28 | status(201) | 27 | status(201) |
| 29 | 28 | ||
| 30 | { | 29 | { |
| 31 | - file_name: attrs[:file_name], | ||
| 32 | file_path: file_path, | 30 | file_path: file_path, |
| 33 | branch_name: branch_name | 31 | branch_name: branch_name |
| 34 | } | 32 | } |
lib/gitlab/satellite/files/new_file_action.rb
| @@ -18,6 +18,13 @@ module Gitlab | @@ -18,6 +18,13 @@ module Gitlab | ||
| 18 | 18 | ||
| 19 | # update the file in the satellite's working dir | 19 | # update the file in the satellite's working dir |
| 20 | file_path_in_satellite = File.join(repo.working_dir, file_path) | 20 | file_path_in_satellite = File.join(repo.working_dir, file_path) |
| 21 | + | ||
| 22 | + # Prevent relative links | ||
| 23 | + unless File.absolute_path(file_path_in_satellite) == file_path_in_satellite | ||
| 24 | + Gitlab::GitLogger.error("NewFileAction: Relative path not allowed") | ||
| 25 | + return false | ||
| 26 | + end | ||
| 27 | + | ||
| 21 | File.open(file_path_in_satellite, 'w') { |f| f.write(content) } | 28 | File.open(file_path_in_satellite, 'w') { |f| f.write(content) } |
| 22 | 29 | ||
| 23 | # add new file | 30 | # add new file |
spec/requests/api/files_spec.rb
| @@ -12,7 +12,7 @@ describe API::API do | @@ -12,7 +12,7 @@ describe API::API do | ||
| 12 | describe "POST /projects/:id/repository/files" do | 12 | describe "POST /projects/:id/repository/files" do |
| 13 | let(:valid_params) { | 13 | let(:valid_params) { |
| 14 | { | 14 | { |
| 15 | - file_name: 'newfile.rb', | 15 | + file_path: 'newfile.rb', |
| 16 | branch_name: 'master', | 16 | branch_name: 'master', |
| 17 | content: 'puts 8', | 17 | content: 'puts 8', |
| 18 | commit_message: 'Added newfile' | 18 | commit_message: 'Added newfile' |
| @@ -26,7 +26,7 @@ describe API::API do | @@ -26,7 +26,7 @@ describe API::API do | ||
| 26 | 26 | ||
| 27 | post api("/projects/#{project.id}/repository/files", user), valid_params | 27 | post api("/projects/#{project.id}/repository/files", user), valid_params |
| 28 | response.status.should == 201 | 28 | response.status.should == 201 |
| 29 | - json_response['file_name'].should == 'newfile.rb' | 29 | + json_response['file_path'].should == 'newfile.rb' |
| 30 | end | 30 | end |
| 31 | 31 | ||
| 32 | it "should return a 400 bad request if no params given" do | 32 | it "should return a 400 bad request if no params given" do |