From d4a9da0db57a2e9193651628bfa3305090fc6873 Mon Sep 17 00:00:00 2001 From: Bart Kamphorst Date: Fri, 16 Nov 2018 14:24:21 +0100 Subject: [PATCH 1/4] Implement infrastructure for detecting and handling of simultaneous edits. --- lib/gollum/app.rb | 8 +++++- .../public/gollum/javascript/gollum.js.erb | 28 +++++++++++++++++++ lib/gollum/templates/editor.mustache | 5 ++-- lib/gollum/views/edit.rb | 3 ++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/lib/gollum/app.rb b/lib/gollum/app.rb index 6c03fcc5..1be48a43 100644 --- a/lib/gollum/app.rb +++ b/lib/gollum/app.rb @@ -176,6 +176,7 @@ module Precious @page = page @page.version = wiki.repo.log(wiki.ref, @page.path).first @content = page.text_data + @etag = Digest::SHA1.hexdigest(@content) mustache :edit else redirect_to("/create/#{encodeURIComponent(@name)}") @@ -278,11 +279,17 @@ module Precious end post '/edit/*' do + etag = params[:etag] path = "/#{clean_url(sanitize_empty_params(params[:path]))}" page_name = CGI.unescape(params[:page]) wiki = wiki_new page = wiki.paged(page_name, path, exact = true) + return if page.nil? + if etag != Digest::SHA1.hexdigest(page.text_data) + halt 412, 'For future use: some helpful data for resolving the conflict here.' + end + committer = Gollum::Committer.new(wiki, commit_message) commit = { :committer => committer } @@ -292,7 +299,6 @@ module Precious update_wiki_page(wiki, page.sidebar, params[:sidebar], commit) if params[:sidebar] committer.commit - redirect to("/#{page.escaped_url_path}") unless page.nil? end diff --git a/lib/gollum/public/gollum/javascript/gollum.js.erb b/lib/gollum/public/gollum/javascript/gollum.js.erb index 4d17c20a..e87278a7 100755 --- a/lib/gollum/public/gollum/javascript/gollum.js.erb +++ b/lib/gollum/public/gollum/javascript/gollum.js.erb @@ -367,6 +367,34 @@ $(document).ready(function() { window.onbeforeunload = function(){ return "Leaving will discard all edits!" }; }); $.GollumEditor(); + $("#gollum-editor-submit").click( function(e) { + e.preventDefault(); + // Prevent button from being clicked again + $(this).attr('disabled', true); + + var formData = new FormData($('#gollum-editor-form').get(0)); + var endpoint = $('#gollum-editor-form').attr("action"); + + $.ajax({ + url: endpoint, + type: 'POST', + data: formData, + processData: false, + contentType: false, + success: function(data) { + window.location = window.location.href.replace(/gollum\/edit\//, '') + }, + error: function(data, textStatus, errorThrown) { + if (data.status == 412) { + $('#gollum-editor-submit').attr('disabled', false) + alert('Someone else has modified this page while you were editing it. Please store your version on disk outside of the browser, reload this page and reapply your modifications.'); + } else { + alert('Error updating page: ' + textStatus + ' ' + errorThrown); + } + } + }); + + }); } if( $("#last-edit").length ) { diff --git a/lib/gollum/templates/editor.mustache b/lib/gollum/templates/editor.mustache index fbb4e0e7..c4345ff7 100644 --- a/lib/gollum/templates/editor.mustache +++ b/lib/gollum/templates/editor.mustache @@ -1,9 +1,9 @@
{{#is_create_page}} -
+ {{/is_create_page}} {{#is_edit_page}} - + {{/is_edit_page}}
{{#is_create_page}} @@ -17,6 +17,7 @@ {{/is_create_page}} {{#is_edit_page}} + {{/is_edit_page}}
diff --git a/lib/gollum/views/edit.rb b/lib/gollum/views/edit.rb index 8c67fe51..f8cf50b2 100755 --- a/lib/gollum/views/edit.rb +++ b/lib/gollum/views/edit.rb @@ -60,6 +60,9 @@ module Precious true end + def etag + @etag + end def allow_uploads @allow_uploads From efd1d3992d093589ba70a82b04f9d4d845e5050f Mon Sep 17 00:00:00 2001 From: Bart Kamphorst Date: Mon, 19 Nov 2018 12:58:30 +0100 Subject: [PATCH 2/4] Use page.sha instead of a content-based hash for collision detection. --- lib/gollum/app.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gollum/app.rb b/lib/gollum/app.rb index 1be48a43..e68443ad 100644 --- a/lib/gollum/app.rb +++ b/lib/gollum/app.rb @@ -176,7 +176,7 @@ module Precious @page = page @page.version = wiki.repo.log(wiki.ref, @page.path).first @content = page.text_data - @etag = Digest::SHA1.hexdigest(@content) + @etag = page.sha mustache :edit else redirect_to("/create/#{encodeURIComponent(@name)}") @@ -279,14 +279,14 @@ module Precious end post '/edit/*' do - etag = params[:etag] + etag = params[:etag] path = "/#{clean_url(sanitize_empty_params(params[:path]))}" page_name = CGI.unescape(params[:page]) wiki = wiki_new page = wiki.paged(page_name, path, exact = true) return if page.nil? - if etag != Digest::SHA1.hexdigest(page.text_data) + if etag != page.sha halt 412, 'For future use: some helpful data for resolving the conflict here.' end From 4eb7fd211cbb92b452bdfe6dbd8e062829739550 Mon Sep 17 00:00:00 2001 From: Bart Kamphorst Date: Tue, 20 Nov 2018 14:53:27 +0100 Subject: [PATCH 3/4] Fix tests (do not expect redirect after edit) and add test for edit collisions. --- test/test_app.rb | 50 ++++++++++++++++++++++++++++---------------- test/test_unicode.rb | 6 ++---- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/test/test_app.rb b/test/test_app.rb index ca7d22ba..27e0ecf2 100644 --- a/test/test_app.rb +++ b/test/test_app.rb @@ -80,8 +80,7 @@ context "Frontend" do test "edits page" do page_1 = @wiki.page('A') post "/gollum/edit/A", :content => 'abc', :page => 'A', - :format => page_1.format, :message => 'def' - follow_redirect! + :format => page_1.format, :message => 'def', :etag => page_1.sha assert last_response.ok? @wiki.clear_cache @@ -90,12 +89,28 @@ context "Frontend" do assert_equal 'def', page_2.version.message assert_not_equal page_1.version.sha, page_2.version.sha end + + test "edit page fails when page is outdated (edit collision)" do + page = @wiki.page('A') + old_sha = page.sha + post "/gollum/edit/A", :content => 'abc', :page => 'A', + :format => page.format, :message => 'def', :etag => old_sha + assert last_response.ok? + + @wiki.clear_cache + page = @wiki.page('A') + new_sha = page.sha + assert_not_equal old_sha, new_sha + + post "/gollum/edit/A", :content => 'def', :page => 'A', + :format => page.format, :message => 'def', :etag => old_sha + assert_equal last_response.status, 412 + end test "edit page with empty message" do page_1 = @wiki.page('A') post "/gollum/edit/A", :content => 'abc', :page => 'A', - :format => page_1.format - follow_redirect! + :format => page_1.format, :etag => page_1.sha assert last_response.ok? @wiki.clear_cache @@ -108,8 +123,7 @@ context "Frontend" do test "edit page with slash" do page_1 = @wiki.page('A') post "/gollum/edit/A", :content => 'abc', :page => 'A', :path => '/////', - :format => page_1.format, :message => 'def' - follow_redirect! + :format => page_1.format, :message => 'def', :etag => page_1.sha assert last_response.ok? end @@ -121,9 +135,7 @@ context "Frontend" do side_1 = page_1.sidebar post "/gollum/edit/A", :header => 'header', - :footer => 'footer', :page => "A", :sidebar => 'sidebar', :message => 'def' - follow_redirect! - assert_equal "/A.md", last_request.fullpath + :footer => 'footer', :page => "A", :sidebar => 'sidebar', :message => 'def', :etag => page_1.sha assert last_response.ok? @wiki.clear_cache @@ -325,7 +337,7 @@ context "Frontend" do page = 'not-real-page' path = '/' post '/gollum/edit/', :content => 'edit_msg', - :page => page, :path => path, :message => '' + :page => page, :path => path, :message => '' page_e = @wiki.paged(page, path) assert_equal nil, page_e end @@ -335,7 +347,7 @@ context "Frontend" do path = 'a/b/' # path must end with / post '/gollum/create', :content => 'create_msg', :page => page, - :path => path, :format => 'markdown', :message => '' + :path => path, :format => 'markdown', :message => '' page_c = @wiki.paged(page, path) assert_equal 'create_msg', page_c.raw_data @@ -344,7 +356,7 @@ context "Frontend" do # post '/edit' fails. post '/edit/' works. post '/gollum/edit/', :content => 'edit_msg', - :page => page, :path => path, :message => '' + :page => page, :path => path, :message => '', :etag => page_c.sha page_e = @wiki.paged(page, path) assert_equal 'edit_msg', page_e.raw_data @@ -469,8 +481,7 @@ context "Frontend" do gollum_author = { :name => 'ghi', :email => 'jkl' } session = { 'gollum.author' => gollum_author } - post "/gollum/edit/A", { :content => 'abc', :page => 'A', :format => page1.format, :message => 'def' }, { 'rack.session' => session } - follow_redirect! + post "/gollum/edit/A", { :content => 'abc', :page => 'A', :format => page1.format, :message => 'def', :etag => page1.sha }, { 'rack.session' => session } assert last_response.ok? @wiki.clear_cache @@ -548,9 +559,11 @@ context "Frontend" do :content => 'りんご', :page => 'Multibyte', :format => :markdown, :message => 'mesg' + page = @wiki.paged('Multibyte') + post "/gollum/edit/Multibyte", :content => 'りんご', :header => 'みかん', :footer => 'バナナ', :sidebar => 'スイカ', - :page => 'Multibyte', :format => :markdown, :message => 'mesg' + :page => 'Multibyte', :format => :markdown, :message => 'mesg', :etag => page.sha get "/gollum/edit/Multibyte" @@ -695,13 +708,14 @@ context "Frontend with lotr" do test "edit pages within sub-directories" do post "/gollum/create", :content => 'big smelly creatures', :page => 'Orc', - :path => 'Mordor', :format => 'markdown', :message => 'oooh, scary' + :path => 'Mordor', :format => 'markdown', :message => 'oooh, scary' assert_equal 'http://example.org/Mordor/Orc.md', last_response.headers['Location'] + page = @wiki.paged('Orc', 'Mordor') post "/gollum/edit/Mordor/Orc", :content => 'not so big smelly creatures', - :page => 'Orc', :path => 'Mordor', :message => 'minor edit' - assert_equal 'http://example.org/Mordor/Orc.md', last_response.headers['Location'] + :page => 'Orc', :path => 'Mordor', :message => 'minor edit', :etag => page.sha + assert last_response.ok? get "/Mordor/Orc" assert_match /not so big smelly creatures/, last_response.body diff --git a/test/test_unicode.rb b/test/test_unicode.rb index e2afd71b..902d03e4 100644 --- a/test/test_unicode.rb +++ b/test/test_unicode.rb @@ -57,8 +57,7 @@ context "Frontend Unicode support" do page = @wiki.page('PG') assert_equal '다른 text', utf8(page.raw_data) - post '/gollum/edit/PG', :page => 'PG', :content => '바뀐 text', :message => 'ghi' - follow_redirect! + post '/gollum/edit/PG', :page => 'PG', :content => '바뀐 text', :message => 'ghi', :etag => page.sha assert last_response.ok? @wiki = Gollum::Wiki.new(@path) @@ -79,8 +78,7 @@ context "Frontend Unicode support" do assert_equal '다른 text', utf8(page.raw_data) post '/gollum/edit/' + CGI.escape('한글'), :page => 'k', :content => '바뀐 text', - :format => 'markdown', :message => 'ghi' - follow_redirect! + :format => 'markdown', :message => 'ghi', :etag => page.sha assert last_response.ok? @wiki = Gollum::Wiki.new(@path) From f4838d12c6a4b1d8475b774bb42a51e1fbec59be Mon Sep 17 00:00:00 2001 From: Bart Kamphorst Date: Tue, 20 Nov 2018 15:04:32 +0100 Subject: [PATCH 4/4] Return the page's most recent version instead of random string. --- lib/gollum/app.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/gollum/app.rb b/lib/gollum/app.rb index e68443ad..ae477226 100644 --- a/lib/gollum/app.rb +++ b/lib/gollum/app.rb @@ -287,7 +287,8 @@ module Precious return if page.nil? if etag != page.sha - halt 412, 'For future use: some helpful data for resolving the conflict here.' + # Signal edit collision and return the page's most recent version + halt 412, {etag: page.sha, text_data: page.text_data}.to_json end committer = Gollum::Committer.new(wiki, commit_message)