From f5c8af990482138ca8da7b430b023c073a0ed391 Mon Sep 17 00:00:00 2001
From: benjamin wil <bw@benjaminwil.info>
Date: Sun, 2 Jan 2022 13:54:52 -0800
Subject: [PATCH] Tweak setting initialization

I could not see a meaningful reason behind calling `forbid` so early in
the settings initialization block. But moving the `forbid` call until
later resolved this issue.

In the real world, I don't see how this would cause issues. But I found
that calling `forbid` when `wiki_options[:allow_editing]` was set to
false caused order-dependent test errors where Sprockets would end up
being badly configured and left without an initialized
`Sprockets::Environment` object, which is required by the
`sprockets-helpers` gem to resolve asset paths.

The test that would cause errors is also in this commit diff. I've
updated it to be a bit more readable.

I also took this opportunity to review and clean up the `@allow_editing`
assignment, which seemed a bit obfuscated.
---
 lib/gollum/app.rb          |  7 +++----
 test/test_allow_editing.rb | 25 ++++++++++++++++---------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/lib/gollum/app.rb b/lib/gollum/app.rb
index 28e0fc39..4323b38a 100644
--- a/lib/gollum/app.rb
+++ b/lib/gollum/app.rb
@@ -103,8 +103,7 @@ module Precious
     end
 
     before do
-      settings.wiki_options[:allow_editing] = settings.wiki_options.fetch(:allow_editing, true)
-      @allow_editing = settings.wiki_options[:allow_editing]
+      @allow_editing = settings.wiki_options.fetch(:allow_editing, true)
       @critic_markup = settings.wiki_options[:critic_markup]
       @redirects_enabled = settings.wiki_options.fetch(:redirects_enabled, true)
       @per_page_uploads = settings.wiki_options[:per_page_uploads]
@@ -112,8 +111,6 @@ module Precious
       
       @wiki_title = settings.wiki_options.fetch(:title, 'Gollum Wiki')
 
-      forbid unless @allow_editing || request.request_method == 'GET'
-
       if settings.wiki_options[:template_dir]
         Precious::Views::Layout.extend Precious::Views::TemplateCascade
         Precious::Views::Layout.template_priority_path = settings.wiki_options[:template_dir]
@@ -143,6 +140,8 @@ module Precious
           config.manifest = Sprockets::Manifest.new(settings.sprockets, @static_assets_path)
         end
       end
+
+      forbid unless @allow_editing || request.request_method == 'GET'
     end
 
     get '/' do
diff --git a/test/test_allow_editing.rb b/test/test_allow_editing.rb
index 104e0332..44ff2a9b 100644
--- a/test/test_allow_editing.rb
+++ b/test/test_allow_editing.rb
@@ -3,8 +3,9 @@ require File.expand_path(File.join(File.dirname(__FILE__), 'helper'))
 
 context "Precious::Views::Editing" do
   include Rack::Test::Methods
+
   setup do
-    @path    = cloned_testpath('examples/revert.git')
+    @path = cloned_testpath('examples/revert.git')
     Precious::App.set(:gollum_path, @path)
     @wiki = Gollum::Wiki.new(@path)
   end
@@ -13,14 +14,20 @@ context "Precious::Views::Editing" do
     FileUtils.rm_rf(@path)
   end
 
-  test "creating page is blocked" do
-    Precious::App.set(:wiki_options, { allow_editing: false})
-    post "/gollum/create", :content => 'abc', :page => "D",
-         :format             => 'markdown', :message => 'def'
-    assert !last_response.ok?
+  test 'creating pages is blocked' do
+    Precious::App.set(:wiki_options, {allow_editing: false})
 
-    page = @wiki.page('D')
-    assert page.nil?
+    post '/gollum/create',
+      content: 'abc',
+      format: 'markdown',
+      message: 'def',
+      page: 'D'
+
+    assert last_response.body.include? 'Forbidden. This wiki is set to no-edit mode.'
+
+    refute last_response.ok?
+
+    assert_nil @wiki.page('D')
   end
 
   test ".redirects.gollum file should not be accessible" do
@@ -28,7 +35,7 @@ context "Precious::Views::Editing" do
     get '/.redirects.gollum'
     assert_match /Accessing this resource is not allowed/, last_response.body
   end
-  
+
   test ".redirects.gollum file should not be editable" do
     Precious::App.set(:wiki_options, { allow_editing: true, allow_uploads: true })
     get '/gollum/edit/.redirects.gollum'