Merge branch '24415-restrict-wiki-page-titles' into 'master'

Limit length of wiki file/directory names

See merge request gitlab-org/gitlab!24364

app/models/wiki_page.rb

@@ -5,6 +5,9 @@ class WikiPage
   PageChangedError = Class.new(StandardError)
   PageRenameError = Class.new(StandardError)
 
+  MAX_TITLE_BYTES = 245
+  MAX_DIRECTORY_BYTES = 255
+
   include ActiveModel::Validations
   include ActiveModel::Conversion
   include StaticModel
@@ -51,6 +54,7 @@ class WikiPage
 
   validates :title, presence: true
   validates :content, presence: true
+  validate :validate_path_limits, if: :title_changed?
 
   # The GitLab ProjectWiki instance.
   attr_reader :wiki
@@ -262,7 +266,7 @@ class WikiPage
   end
 
   def title_changed?
-    title.present? && self.class.unhyphenize(@page.url_path) != title
+    title.present? && (@page.nil? || self.class.unhyphenize(@page.url_path) != title)
   end
 
   # Updates the current @attributes hash by merging a hash of params
@@ -324,4 +328,16 @@ class WikiPage
     set_attributes
     @persisted = errors.blank?
   end
+
+  def validate_path_limits
+    *dirnames, title = @attributes[:title].split('/')
+
+    if title.bytesize > MAX_TITLE_BYTES
+      errors.add(:title, _("exceeds the limit of %{bytes} bytes for page titles") % { bytes: MAX_TITLE_BYTES })
+    end
+
+    if dirnames.any? { |d| d.bytesize > MAX_DIRECTORY_BYTES }
+      errors.add(:title, _("exceeds the limit of %{bytes} bytes for directory names") % { bytes: MAX_DIRECTORY_BYTES })
+    end
+  end
 end

app/views/projects/wikis/_form.html.haml

@@ -17,9 +17,13 @@
         = icon('lightbulb-o')
         - if @page.persisted?
           = s_("WikiEditPageTip|Tip: You can move this page by adding the path to the beginning of the title.")
-          = link_to icon('question-circle'), help_page_path('user/project/wiki/index', anchor: 'moving-a-wiki-page'), target: '_blank'
+          = link_to icon('question-circle'), help_page_path('user/project/wiki/index', anchor: 'moving-a-wiki-page'),
+              target: '_blank', rel: 'noopener noreferrer'
         - else
           = s_("WikiNewPageTip|Tip: You can specify the full path for the new file. We will automatically create any missing directories.")
+        = succeed '.' do
+          = link_to _('Learn more'), help_page_path('user/project/wiki/index', anchor: 'creating-a-new-wiki-page'),
+              target: '_blank', rel: 'noopener noreferrer'
   .form-group.row
     .col-sm-12= f.label :format, class: 'control-label-full-width'
     .col-sm-12

changelogs/unreleased/24415-restrict-wiki-page-titles.yml

+---
+title: Limit length of wiki file/directory names
+merge_request: 24364
+author:
+type: changed

doc/administration/instance_limits.md

@@ -119,3 +119,7 @@ This limit can be configured for self hosted installations when [enabling
 Elasticsearch](../integration/elasticsearch.md#enabling-elasticsearch).
 
 NOTE: **Note:** Set the limit to `0` to disable it.
+
+## Wiki limits
+
+- [Length restrictions for file and directory names](../user/project/wiki/index.md#length-restrictions-for-file-and-directory-names).

doc/user/project/wiki/index.md

@@ -50,6 +50,8 @@ When you're ready, click the **Create page** and the new page will be created.
 
 ![New page](img/wiki_create_new_page.png)
 
+### Attachment storage
+
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/issues/33475) in GitLab 11.3.
 
 Starting with GitLab 11.3, any file that is uploaded to the wiki via GitLab's
@@ -58,6 +60,22 @@ if you clone the wiki repository locally. All uploaded files prior to GitLab
 11.3 are stored in GitLab itself. If you want them to be part of the wiki's Git
 repository, you will have to upload them again.
 
+### Length restrictions for file and directory names
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24364) in GitLab 12.8.
+
+Many common file systems have a [limit of 255 bytes for file and directory names](https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits), and while Git and GitLab both support paths exceeding those limits, the presence of them makes it impossible for users on those file systems to checkout a wiki repository locally.
+
+To avoid this situation, these limits are enforced when editing pages through the GitLab web interface and API:
+
+- 245 bytes for page titles (reserving 10 bytes for the file extension).
+- 255 bytes for directory names.
+
+Please note that:
+
+- Non-ASCII characters take up more than one byte.
+- It's still possible to create files and directories exceeding those limits locally through Git, but this might break on other people's machines.
+
 ## Editing a wiki page
 
 NOTE: **Note:**

locale/gitlab.pot

@@ -22772,6 +22772,12 @@ msgstr ""
 msgid "estimateCommand|%{slash_command} will update the estimated time with the latest command."
 msgstr ""
 
+msgid "exceeds the limit of %{bytes} bytes for directory names"
+msgstr ""
+
+msgid "exceeds the limit of %{bytes} bytes for page titles"
+msgstr ""
+
 msgid "expired on %{milestone_due_date}"
 msgstr ""
 

spec/models/wiki_page_spec.rb

@@ -7,7 +7,18 @@ describe WikiPage do
   let(:user) { project.owner }
   let(:wiki) { ProjectWiki.new(project, user) }
 
-  subject { described_class.new(wiki) }
+  let(:new_page) do
+    described_class.new(wiki).tap do |page|
+      page.attributes = { title: 'test page', content: 'test content' }
+    end
+  end
+
+  let(:existing_page) do
+    create_page('test page', 'test content')
+    wiki.find_page('test page')
+  end
+
+  subject { new_page }
 
   describe '.group_by_directory' do
     context 'when there are no pages' do
@@ -100,56 +111,134 @@ describe WikiPage do
 
   describe "#initialize" do
     context "when initialized with an existing page" do
-      before do
-        create_page("test page", "test content")
-        @page = wiki.wiki.page(title: "test page")
-        @wiki_page = described_class.new(wiki, @page, true)
-      end
+      subject { existing_page }
 
       it "sets the slug attribute" do
-        expect(@wiki_page.slug).to eq("test-page")
+        expect(subject.slug).to eq("test-page")
       end
 
       it "sets the title attribute" do
-        expect(@wiki_page.title).to eq("test page")
+        expect(subject.title).to eq("test page")
       end
 
       it "sets the formatted content attribute" do
-        expect(@wiki_page.content).to eq("test content")
+        expect(subject.content).to eq("test content")
       end
 
       it "sets the format attribute" do
-        expect(@wiki_page.format).to eq(:markdown)
+        expect(subject.format).to eq(:markdown)
       end
 
       it "sets the message attribute" do
-        expect(@wiki_page.message).to eq("test commit")
+        expect(subject.message).to eq("test commit")
       end
 
       it "sets the version attribute" do
-        expect(@wiki_page.version).to be_a Gitlab::Git::WikiPageVersion
+        expect(subject.version).to be_a Gitlab::Git::WikiPageVersion
       end
     end
   end
 
   describe "validations" do
-    before do
-      subject.attributes = { title: 'title', content: 'content' }
-    end
-
     it "validates presence of title" do
       subject.attributes.delete(:title)
-      expect(subject.valid?).to be_falsey
+
+      expect(subject).not_to be_valid
+      expect(subject.errors.keys).to contain_exactly(:title)
     end
 
     it "validates presence of content" do
       subject.attributes.delete(:content)
-      expect(subject.valid?).to be_falsey
+
+      expect(subject).not_to be_valid
+      expect(subject.errors.keys).to contain_exactly(:content)
+    end
+
+    describe '#validate_path_limits' do
+      let(:max_title) { described_class::MAX_TITLE_BYTES }
+      let(:max_directory) { described_class::MAX_DIRECTORY_BYTES }
+
+      where(:character) do
+        ['a', 'ä', '🙈']
+      end
+
+      with_them do
+        let(:size) { character.bytesize.to_f }
+        let(:valid_title) { character * (max_title / size).floor }
+        let(:valid_directory) { character * (max_directory / size).floor }
+        let(:invalid_title) { character * ((max_title + 1) / size).ceil }
+        let(:invalid_directory) { character * ((max_directory + 1) / size).ceil }
+
+        it 'accepts page titles below the limit' do
+          subject.title = valid_title
+
+          expect(subject).to be_valid
+        end
+
+        it 'accepts directories below the limit' do
+          subject.title = valid_directory + '/foo'
+
+          expect(subject).to be_valid
+        end
+
+        it 'accepts a path with page title and directory below the limit' do
+          subject.title = "#{valid_directory}/#{valid_title}"
+
+          expect(subject).to be_valid
+        end
+
+        it 'rejects page titles exceeding the limit' do
+          subject.title = invalid_title
+
+          expect(subject).not_to be_valid
+          expect(subject.errors[:title]).to contain_exactly(
+            "exceeds the limit of #{max_title} bytes for page titles"
+          )
+        end
+
+        it 'rejects directories exceeding the limit' do
+          subject.title = invalid_directory + '/foo'
+
+          expect(subject).not_to be_valid
+          expect(subject.errors[:title]).to contain_exactly(
+            "exceeds the limit of #{max_directory} bytes for directory names"
+          )
+        end
+
+        it 'rejects a page with both title and directory exceeding the limit' do
+          subject.title = "#{invalid_directory}/#{invalid_title}"
+
+          expect(subject).not_to be_valid
+          expect(subject.errors[:title]).to contain_exactly(
+            "exceeds the limit of #{max_title} bytes for page titles",
+            "exceeds the limit of #{max_directory} bytes for directory names"
+          )
+        end
+      end
+
+      context 'with an existing page title exceeding the limit' do
+        subject do
+          title = 'a' * (max_title + 1)
+          create_page(title, 'content')
+          wiki.find_page(title)
+        end
+
+        it 'accepts the exceeding title length when unchanged' do
+          expect(subject).to be_valid
+        end
+
+        it 'rejects the exceeding title length when changed' do
+          subject.title = 'b' * (max_title + 1)
+
+          expect(subject).not_to be_valid
+          expect(subject.errors).to include(:title)
+        end
+      end
     end
   end
 
   describe "#create" do
-    let(:wiki_attr) do
+    let(:attributes) do
       {
         title: "Index",
         content: "Home Page",
@@ -158,22 +247,19 @@ describe WikiPage do
       }
     end
 
-    after do
-      destroy_page("Index")
-    end
-
     context "with valid attributes" do
       it "saves the wiki page" do
-        subject.create(wiki_attr)
+        subject.create(attributes)
+
         expect(wiki.find_page("Index")).not_to be_nil
       end
 
       it "returns true" do
-        expect(subject.create(wiki_attr)).to eq(true)
+        expect(subject.create(attributes)).to eq(true)
       end
 
       it 'saves the wiki page with message' do
-        subject.create(wiki_attr)
+        subject.create(attributes)
 
         expect(wiki.find_page("Index").message).to eq 'Custom Commit Message'
       end
@@ -183,40 +269,37 @@ describe WikiPage do
   describe "dot in the title" do
     let(:title) { 'Index v1.2.3' }
 
-    before do
-      @wiki_attr = { title: title, content: "Home Page", format: "markdown" }
-    end
-
     describe "#create" do
-      after do
-        destroy_page(title)
-      end
+      let(:attributes) { { title: title, content: "Home Page", format: "markdown" } }
 
       context "with valid attributes" do
         it "saves the wiki page" do
-          subject.create(@wiki_attr)
+          subject.create(attributes)
+
           expect(wiki.find_page(title)).not_to be_nil
         end
 
         it "returns true" do
-          expect(subject.create(@wiki_attr)).to eq(true)
+          expect(subject.create(attributes)).to eq(true)
         end
       end
     end
 
     describe "#update" do
-      before do
+      subject do
         create_page(title, "content")
-        @page = wiki.find_page(title)
+        wiki.find_page(title)
       end
 
       it "updates the content of the page" do
-        @page.update(content: "new content")
-        @page = wiki.find_page(title)
+        subject.update(content: "new content")
+        page = wiki.find_page(title)
+
+        expect(page.content).to eq('new content')
       end
 
       it "returns true" do
-        expect(@page.update(content: "more content")).to be_truthy
+        expect(subject.update(content: "more content")).to be_truthy
       end
     end
   end
@@ -226,66 +309,55 @@ describe WikiPage do
       it 'raises an error if a page with the same path already exists' do
         create_page('New Page', 'content')
         create_page('foo/bar', 'content')
+
         expect { create_page('New Page', 'other content') }.to raise_error Gitlab::Git::Wiki::DuplicatePageError
         expect { create_page('foo/bar', 'other content') }.to raise_error Gitlab::Git::Wiki::DuplicatePageError
-
-        destroy_page('New Page')
-        destroy_page('bar', 'foo')
       end
 
       it 'if the title is preceded by a / it is removed' do
         create_page('/New Page', 'content')
 
         expect(wiki.find_page('New Page')).not_to be_nil
-
-        destroy_page('New Page')
       end
     end
   end
 
   describe "#update" do
-    before do
-      create_page("Update", "content")
-      @page = wiki.find_page("Update")
-    end
-
-    after do
-      destroy_page(@page.title, @page.directory)
-    end
+    subject { existing_page }
 
     context "with valid attributes" do
       it "updates the content of the page" do
         new_content = "new content"
 
-        @page.update(content: new_content)
-        @page = wiki.find_page("Update")
+        subject.update(content: new_content)
+        page = wiki.find_page('test page')
 
-        expect(@page.content).to eq("new content")
+        expect(page.content).to eq("new content")
       end
 
       it "updates the title of the page" do
         new_title = "Index v.1.2.4"
 
-        @page.update(title: new_title)
-        @page = wiki.find_page(new_title)
+        subject.update(title: new_title)
+        page = wiki.find_page(new_title)
 
-        expect(@page.title).to eq(new_title)
+        expect(page.title).to eq(new_title)
       end
 
       it "returns true" do
-        expect(@page.update(content: "more content")).to be_truthy
+        expect(subject.update(content: "more content")).to be_truthy
       end
     end
 
     context 'with same last commit sha' do
       it 'returns true' do
-        expect(@page.update(content: 'more content', last_commit_sha: @page.last_commit_sha)).to be_truthy
+        expect(subject.update(content: 'more content', last_commit_sha: subject.last_commit_sha)).to be_truthy
       end
     end
 
     context 'with different last commit sha' do
       it 'raises exception' do
-        expect { @page.update(content: 'more content', last_commit_sha: 'xxx') }.to raise_error(WikiPage::PageChangedError)
+        expect { subject.update(content: 'more content', last_commit_sha: 'xxx') }.to raise_error(WikiPage::PageChangedError)
       end
     end
 
@@ -293,23 +365,21 @@ describe WikiPage do
       it 'raises an error if the page already exists' do
         create_page('Existing Page', 'content')
 
-        expect { @page.update(title: 'Existing Page', content: 'new_content') }.to raise_error(WikiPage::PageRenameError)
-        expect(@page.title).to eq 'Update'
-        expect(@page.content).to eq 'new_content'
-
-        destroy_page('Existing Page')
+        expect { subject.update(title: 'Existing Page', content: 'new_content') }.to raise_error(WikiPage::PageRenameError)
+        expect(subject.title).to eq 'test page'
+        expect(subject.content).to eq 'new_content'
       end
 
       it 'updates the content and rename the file' do
         new_title = 'Renamed Page'
         new_content = 'updated content'
 
-        expect(@page.update(title: new_title, content: new_content)).to be_truthy
+        expect(subject.update(title: new_title, content: new_content)).to be_truthy
 
-        @page = wiki.find_page(new_title)
+        page = wiki.find_page(new_title)
 
-        expect(@page).not_to be_nil
-        expect(@page.content).to eq new_content
+        expect(page).not_to be_nil
+        expect(page.content).to eq new_content
       end
     end
 
@@ -317,18 +387,16 @@ describe WikiPage do
       it 'raises an error if the page already exists' do
         create_page('foo/Existing Page', 'content')
 
-        expect { @page.update(title: 'foo/Existing Page', content: 'new_content') }.to raise_error(WikiPage::PageRenameError)
-        expect(@page.title).to eq 'Update'
-        expect(@page.content).to eq 'new_content'
-
-        destroy_page('Existing Page', 'foo')
+        expect { subject.update(title: 'foo/Existing Page', content: 'new_content') }.to raise_error(WikiPage::PageRenameError)
+        expect(subject.title).to eq 'test page'
+        expect(subject.content).to eq 'new_content'
       end
 
       it 'updates the content and moves the file' do
         new_title = 'foo/Other Page'
         new_content = 'new_content'
 
-        expect(@page.update(title: new_title, content: new_content)).to be_truthy
+        expect(subject.update(title: new_title, content: new_content)).to be_truthy
 
         page = wiki.find_page(new_title)
 
@@ -337,120 +405,101 @@ describe WikiPage do
       end
 
       context 'in subdir' do
-        before do
+        subject do
           create_page('foo/Existing Page', 'content')
-          @page = wiki.find_page('foo/Existing Page')
+          wiki.find_page('foo/Existing Page')
         end
 
         it 'moves the page to the root folder if the title is preceded by /' do
-          expect(@page.slug).to eq 'foo/Existing-Page'
-          expect(@page.update(title: '/Existing Page', content: 'new_content')).to be_truthy
-          expect(@page.slug).to eq 'Existing-Page'
+          expect(subject.slug).to eq 'foo/Existing-Page'
+          expect(subject.update(title: '/Existing Page', content: 'new_content')).to be_truthy
+          expect(subject.slug).to eq 'Existing-Page'
         end
 
         it 'does nothing if it has the same title' do
-          original_path = @page.slug
+          original_path = subject.slug
 
-          expect(@page.update(title: 'Existing Page', content: 'new_content')).to be_truthy
-          expect(@page.slug).to eq original_path
+          expect(subject.update(title: 'Existing Page', content: 'new_content')).to be_truthy
+          expect(subject.slug).to eq original_path
         end
       end
 
       context 'in root dir' do
         it 'does nothing if the title is preceded by /' do
-          original_path = @page.slug
+          original_path = subject.slug
 
-          expect(@page.update(title: '/Update', content: 'new_content')).to be_truthy
-          expect(@page.slug).to eq original_path
+          expect(subject.update(title: '/test page', content: 'new_content')).to be_truthy
+          expect(subject.slug).to eq original_path
         end
       end
     end
 
     context "with invalid attributes" do
       it 'aborts update if title blank' do
-        expect(@page.update(title: '', content: 'new_content')).to be_falsey
-        expect(@page.content).to eq 'new_content'
+        expect(subject.update(title: '', content: 'new_content')).to be_falsey
+        expect(subject.content).to eq 'new_content'
 
-        page = wiki.find_page('Update')
-        expect(page.content).to eq 'content'
+        page = wiki.find_page('test page')
 
-        @page.title = 'Update'
+        expect(page.content).to eq 'test content'
       end
     end
   end
 
   describe "#destroy" do
-    before do
-      create_page("Delete Page", "content")
-      @page = wiki.find_page("Delete Page")
-    end
+    subject { existing_page }
 
     it "deletes the page" do
-      @page.delete
+      subject.delete
+
       expect(wiki.list_pages).to be_empty
     end
 
     it "returns true" do
-      expect(@page.delete).to eq(true)
+      expect(subject.delete).to eq(true)
     end
   end
 
   describe "#versions" do
-    let(:page) { wiki.find_page("Update") }
-
-    before do
-      create_page("Update", "content")
-    end
-
-    after do
-      destroy_page("Update")
-    end
+    subject { existing_page }
 
     it "returns an array of all commits for the page" do
-      3.times { |i| page.update(content: "content #{i}") }
+      3.times { |i| subject.update(content: "content #{i}") }
 
-      expect(page.versions.count).to eq(4)
+      expect(subject.versions.count).to eq(4)
     end
 
     it 'returns instances of WikiPageVersion' do
-      expect(page.versions).to all( be_a(Gitlab::Git::WikiPageVersion) )
+      expect(subject.versions).to all( be_a(Gitlab::Git::WikiPageVersion) )
     end
   end
 
   describe "#title" do
-    before do
-      create_page("Title", "content")
-      @page = wiki.find_page("Title")
-    end
-
-    after do
-      destroy_page("Title")
-    end
-
     it "replaces a hyphen to a space" do
-      @page.title = "Import-existing-repositories-into-GitLab"
-      expect(@page.title).to eq("Import existing repositories into GitLab")
+      subject.title = "Import-existing-repositories-into-GitLab"
+
+      expect(subject.title).to eq("Import existing repositories into GitLab")
     end
 
     it 'unescapes html' do
-      @page.title = 'foo & bar'
+      subject.title = 'foo & bar'
 
-      expect(@page.title).to eq('foo & bar')
+      expect(subject.title).to eq('foo & bar')
     end
   end
 
   describe '#path' do
     let(:path) { 'mypath.md' }
-    let(:wiki_page) { instance_double('Gitlab::Git::WikiPage', path: path).as_null_object }
+    let(:git_page) { instance_double('Gitlab::Git::WikiPage', path: path).as_null_object }
 
     it 'returns the path when persisted' do
-      page = described_class.new(wiki, wiki_page, true)
+      page = described_class.new(wiki, git_page, true)
 
       expect(page.path).to eq(path)
     end
 
     it 'returns nil when not persisted' do
-      page = described_class.new(wiki, wiki_page, false)
+      page = described_class.new(wiki, git_page, false)
 
       expect(page.path).to be_nil
     end
@@ -458,39 +507,38 @@ describe WikiPage do
 
   describe '#directory' do
     context 'when the page is at the root directory' do
-      it 'returns an empty string' do
+      subject do
         create_page('file', 'content')
-        page = wiki.find_page('file')
+        wiki.find_page('file')
+      end
 
-        expect(page.directory).to eq('')
+      it 'returns an empty string' do
+        expect(subject.directory).to eq('')
       end
     end
 
     context 'when the page is inside an actual directory' do
-      it 'returns the full directory hierarchy' do
+      subject do
         create_page('dir_1/dir_1_1/file', 'content')
-        page = wiki.find_page('dir_1/dir_1_1/file')
+        wiki.find_page('dir_1/dir_1_1/file')
+      end
 
-        expect(page.directory).to eq('dir_1/dir_1_1')
+      it 'returns the full directory hierarchy' do
+        expect(subject.directory).to eq('dir_1/dir_1_1')
       end
     end
   end
 
   describe '#historical?' do
-    let(:page) { wiki.find_page('Update') }
-    let(:old_version) { page.versions.last.id }
-    let(:old_page) { wiki.find_page('Update', old_version) }
-    let(:latest_version) { page.versions.first.id }
-    let(:latest_page) { wiki.find_page('Update', latest_version) }
+    subject { existing_page }
 
-    before do
-      create_page('Update', 'content')
-      @page = wiki.find_page('Update')
-      3.times { |i| @page.update(content: "content #{i}") }
-    end
+    let(:old_version) { subject.versions.last.id }
+    let(:old_page) { wiki.find_page(subject.title, old_version) }
+    let(:latest_version) { subject.versions.first.id }
+    let(:latest_page) { wiki.find_page(subject.title, latest_version) }
 
-    after do
-      destroy_page('Update')
+    before do
+      3.times { |i| subject.update(content: "content #{i}") }
     end
 
     it 'returns true when requesting an old version' do
@@ -520,56 +568,48 @@ describe WikiPage do
 
   describe '#to_partial_path' do
     it 'returns the relative path to the partial to be used' do
-      page = build(:wiki_page)
-
-      expect(page.to_partial_path).to eq('projects/wikis/wiki_page')
+      expect(subject.to_partial_path).to eq('projects/wikis/wiki_page')
     end
   end
 
   describe '#==' do
-    let(:original_wiki_page) { create(:wiki_page) }
+    subject { existing_page }
 
     it 'returns true for identical wiki page' do
-      expect(original_wiki_page).to eq(original_wiki_page)
+      expect(subject).to eq(subject)
     end
 
     it 'returns false for updated wiki page' do
-      updated_wiki_page = original_wiki_page.update(content: "Updated content")
-      expect(original_wiki_page).not_to eq(updated_wiki_page)
+      subject.update(content: "Updated content")
+      updated_page = wiki.find_page('test page')
+
+      expect(updated_page).not_to be_nil
+      expect(updated_page).not_to eq(subject)
     end
   end
 
   describe '#last_commit_sha' do
-    before do
-      create_page("Update", "content")
-      @page = wiki.find_page("Update")
-    end
-
-    after do
-      destroy_page("Update")
-    end
+    subject { existing_page }
 
     it 'returns commit sha' do
-      expect(@page.last_commit_sha).to eq @page.last_version.sha
+      expect(subject.last_commit_sha).to eq subject.last_version.sha
     end
 
     it 'is changed after page updated' do
-      last_commit_sha_before_update = @page.last_commit_sha
+      last_commit_sha_before_update = subject.last_commit_sha
 
-      @page.update(content: "new content")
-      @page = wiki.find_page("Update")
+      subject.update(content: "new content")
+      page = wiki.find_page('test page')
 
-      expect(@page.last_commit_sha).not_to eq last_commit_sha_before_update
+      expect(page.last_commit_sha).not_to eq last_commit_sha_before_update
     end
   end
 
   describe '#hook_attrs' do
     it 'adds absolute urls for images in the content' do
-      create_page("test page", "test![WikiPage_Image](/uploads/abc/WikiPage_Image.png)")
-      page = wiki.wiki.page(title: "test page")
-      wiki_page = described_class.new(wiki, page, true)
+      subject.attributes[:content] = 'test![WikiPage_Image](/uploads/abc/WikiPage_Image.png)'
 
-      expect(wiki_page.hook_attrs['content']).to eq("test![WikiPage_Image](#{Settings.gitlab.url}/uploads/abc/WikiPage_Image.png)")
+      expect(subject.hook_attrs['content']).to eq("test![WikiPage_Image](#{Settings.gitlab.url}/uploads/abc/WikiPage_Image.png)")
     end
   end
 
@@ -587,11 +627,6 @@ describe WikiPage do
     wiki.wiki.write_page(name, :markdown, content, commit_details)
   end
 
-  def destroy_page(title, dir = '')
-    page = wiki.wiki.page(title: title, dir: dir)
-    wiki.delete_page(page, "test commit")
-  end
-
   def get_slugs(page_or_dir)
     if page_or_dir.is_a? WikiPage
       [page_or_dir.slug]