summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTimothy Andrew <mail@timothyandrew.net>2017-06-12 09:11:30 +0000
committerTimothy Andrew <mail@timothyandrew.net>2017-06-12 09:11:30 +0000
commit1e24ed3ae38e00a1cf898bf0109ad9502aee0796 (patch)
tree5f8d8bdcc9c13c06603abf89c69abc90dcae45d1
parent09420c6d8fe4f428d0a7aaebadf15ddd4cfc36fc (diff)
parent0d0d0297a1e90fc7d50deb8fa7c3f966036480f4 (diff)
downloadgitlab-ce-1e24ed3ae38e00a1cf898bf0109ad9502aee0796.tar.gz
Merge remote-tracking branch 'origin/fix-9-2-stable-conflicts-for-mr-11964' into 9-2-stable
-rw-r--r--changelogs/unreleased/32995-issue-contents-dynamically-replaced-with-stale-version-after-saving-or-refreshing-relative-external_url-only.yml4
-rw-r--r--lib/gitlab/etag_caching/middleware.rb9
-rw-r--r--lib/gitlab/etag_caching/router.rb4
-rw-r--r--spec/lib/gitlab/etag_caching/middleware_spec.rb19
-rw-r--r--spec/lib/gitlab/etag_caching/router_spec.rb36
5 files changed, 48 insertions, 24 deletions
diff --git a/changelogs/unreleased/32995-issue-contents-dynamically-replaced-with-stale-version-after-saving-or-refreshing-relative-external_url-only.yml b/changelogs/unreleased/32995-issue-contents-dynamically-replaced-with-stale-version-after-saving-or-refreshing-relative-external_url-only.yml
new file mode 100644
index 00000000000..5cd36a4e3e2
--- /dev/null
+++ b/changelogs/unreleased/32995-issue-contents-dynamically-replaced-with-stale-version-after-saving-or-refreshing-relative-external_url-only.yml
@@ -0,0 +1,4 @@
+---
+title: Fix incorrect ETag cache key when relative instance URL is used
+merge_request: 11964
+author:
diff --git a/lib/gitlab/etag_caching/middleware.rb b/lib/gitlab/etag_caching/middleware.rb
index 270d67dd50c..7f884183bb1 100644
--- a/lib/gitlab/etag_caching/middleware.rb
+++ b/lib/gitlab/etag_caching/middleware.rb
@@ -6,12 +6,13 @@ module Gitlab
end
def call(env)
- route = Gitlab::EtagCaching::Router.match(env)
+ request = Rack::Request.new(env)
+ route = Gitlab::EtagCaching::Router.match(request)
return @app.call(env) unless route
track_event(:etag_caching_middleware_used, route)
- etag, cached_value_present = get_etag(env)
+ etag, cached_value_present = get_etag(request)
if_none_match = env['HTTP_IF_NONE_MATCH']
if if_none_match == etag
@@ -27,8 +28,8 @@ module Gitlab
private
- def get_etag(env)
- cache_key = env['PATH_INFO']
+ def get_etag(request)
+ cache_key = request.path
store = Gitlab::EtagCaching::Store.new
current_value = store.get(cache_key)
cached_value_present = current_value.present?
diff --git a/lib/gitlab/etag_caching/router.rb b/lib/gitlab/etag_caching/router.rb
index d74e31af5c6..597ccb58bfc 100644
--- a/lib/gitlab/etag_caching/router.rb
+++ b/lib/gitlab/etag_caching/router.rb
@@ -43,8 +43,8 @@ module Gitlab
),
].freeze
- def self.match(env)
- ROUTES.find { |route| route.regexp.match(env['PATH_INFO']) }
+ def self.match(request)
+ ROUTES.find { |route| route.regexp.match(request.path_info) }
end
end
end
diff --git a/spec/lib/gitlab/etag_caching/middleware_spec.rb b/spec/lib/gitlab/etag_caching/middleware_spec.rb
index 24df04e985a..3c6ef7c7ccb 100644
--- a/spec/lib/gitlab/etag_caching/middleware_spec.rb
+++ b/spec/lib/gitlab/etag_caching/middleware_spec.rb
@@ -164,6 +164,25 @@ describe Gitlab::EtagCaching::Middleware do
end
end
+ context 'when GitLab instance is using a relative URL' do
+ before do
+ mock_app_response
+ end
+
+ it 'uses full path as cache key' do
+ env = {
+ 'PATH_INFO' => enabled_path,
+ 'SCRIPT_NAME' => '/relative-gitlab'
+ }
+
+ expect_any_instance_of(Gitlab::EtagCaching::Store)
+ .to receive(:get).with("/relative-gitlab#{enabled_path}")
+ .and_return(nil)
+
+ middleware.call(env)
+ end
+ end
+
def mock_app_response
allow(app).to receive(:call).and_return([app_status_code, {}, ['body']])
end
diff --git a/spec/lib/gitlab/etag_caching/router_spec.rb b/spec/lib/gitlab/etag_caching/router_spec.rb
index 410df116a3a..5eafce9cb8f 100644
--- a/spec/lib/gitlab/etag_caching/router_spec.rb
+++ b/spec/lib/gitlab/etag_caching/router_spec.rb
@@ -2,93 +2,93 @@ require 'spec_helper'
describe Gitlab::EtagCaching::Router do
it 'matches issue notes endpoint' do
- env = build_env(
+ request = build_request(
'/my-group/and-subgroup/here-comes-the-project/noteable/issue/1/notes'
)
- result = described_class.match(env)
+ result = described_class.match(request)
expect(result).to be_present
expect(result.name).to eq 'issue_notes'
end
it 'matches issue title endpoint' do
- env = build_env(
+ request = build_request(
'/my-group/my-project/issues/123/rendered_title'
)
- result = described_class.match(env)
+ result = described_class.match(request)
expect(result).to be_present
expect(result.name).to eq 'issue_title'
end
it 'matches project pipelines endpoint' do
- env = build_env(
+ request = build_request(
'/my-group/my-project/pipelines.json'
)
- result = described_class.match(env)
+ result = described_class.match(request)
expect(result).to be_present
expect(result.name).to eq 'project_pipelines'
end
it 'matches commit pipelines endpoint' do
- env = build_env(
+ request = build_request(
'/my-group/my-project/commit/aa8260d253a53f73f6c26c734c72fdd600f6e6d4/pipelines.json'
)
- result = described_class.match(env)
+ result = described_class.match(request)
expect(result).to be_present
expect(result.name).to eq 'commit_pipelines'
end
it 'matches new merge request pipelines endpoint' do
- env = build_env(
+ request = build_request(
'/my-group/my-project/merge_requests/new.json'
)
- result = described_class.match(env)
+ result = described_class.match(request)
expect(result).to be_present
expect(result.name).to eq 'new_merge_request_pipelines'
end
it 'matches merge request pipelines endpoint' do
- env = build_env(
+ request = build_request(
'/my-group/my-project/merge_requests/234/pipelines.json'
)
- result = described_class.match(env)
+ result = described_class.match(request)
expect(result).to be_present
expect(result.name).to eq 'merge_request_pipelines'
end
it 'does not match blob with confusing name' do
- env = build_env(
+ request = build_request(
'/my-group/my-project/blob/master/pipelines.json'
)
- result = described_class.match(env)
+ result = described_class.match(request)
expect(result).to be_blank
end
it 'matches pipeline#show endpoint' do
- env = build_env(
+ request = build_request(
'/my-group/my-project/pipelines/2.json'
)
- result = described_class.match(env)
+ result = described_class.match(request)
expect(result).to be_present
expect(result.name).to eq 'project_pipeline'
end
- def build_env(path)
- { 'PATH_INFO' => path }
+ def build_request(path)
+ double(path_info: path)
end
end