Commit 280529c7 authored by Adam Niedzielski's avatar Adam Niedzielski

Fix incorrect ETag cache key when relative instance URL is used

parent dddc54aa
---
title: Fix incorrect ETag cache key when relative instance URL is used
merge_request: 11964
author:
...@@ -6,12 +6,13 @@ module Gitlab ...@@ -6,12 +6,13 @@ module Gitlab
end end
def call(env) 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 return @app.call(env) unless route
track_event(:etag_caching_middleware_used, 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_none_match = env['HTTP_IF_NONE_MATCH']
if if_none_match == etag if if_none_match == etag
...@@ -27,8 +28,8 @@ module Gitlab ...@@ -27,8 +28,8 @@ module Gitlab
private private
def get_etag(env) def get_etag(request)
cache_key = env['PATH_INFO'] cache_key = request.path
store = Gitlab::EtagCaching::Store.new store = Gitlab::EtagCaching::Store.new
current_value = store.get(cache_key) current_value = store.get(cache_key)
cached_value_present = current_value.present? cached_value_present = current_value.present?
......
...@@ -53,8 +53,8 @@ module Gitlab ...@@ -53,8 +53,8 @@ module Gitlab
) )
].freeze ].freeze
def self.match(env) def self.match(request)
ROUTES.find { |route| route.regexp.match(env['PATH_INFO']) } ROUTES.find { |route| route.regexp.match(request.path_info) }
end end
end end
end end
......
...@@ -164,6 +164,25 @@ describe Gitlab::EtagCaching::Middleware do ...@@ -164,6 +164,25 @@ describe Gitlab::EtagCaching::Middleware do
end end
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 def mock_app_response
allow(app).to receive(:call).and_return([app_status_code, {}, ['body']]) allow(app).to receive(:call).and_return([app_status_code, {}, ['body']])
end end
......
...@@ -2,115 +2,115 @@ require 'spec_helper' ...@@ -2,115 +2,115 @@ require 'spec_helper'
describe Gitlab::EtagCaching::Router do describe Gitlab::EtagCaching::Router do
it 'matches issue notes endpoint' 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' '/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).to be_present
expect(result.name).to eq 'issue_notes' expect(result.name).to eq 'issue_notes'
end end
it 'matches issue title endpoint' do it 'matches issue title endpoint' do
env = build_env( request = build_request(
'/my-group/my-project/issues/123/realtime_changes' '/my-group/my-project/issues/123/realtime_changes'
) )
result = described_class.match(env) result = described_class.match(request)
expect(result).to be_present expect(result).to be_present
expect(result.name).to eq 'issue_title' expect(result.name).to eq 'issue_title'
end end
it 'matches project pipelines endpoint' do it 'matches project pipelines endpoint' do
env = build_env( request = build_request(
'/my-group/my-project/pipelines.json' '/my-group/my-project/pipelines.json'
) )
result = described_class.match(env) result = described_class.match(request)
expect(result).to be_present expect(result).to be_present
expect(result.name).to eq 'project_pipelines' expect(result.name).to eq 'project_pipelines'
end end
it 'matches commit pipelines endpoint' do it 'matches commit pipelines endpoint' do
env = build_env( request = build_request(
'/my-group/my-project/commit/aa8260d253a53f73f6c26c734c72fdd600f6e6d4/pipelines.json' '/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).to be_present
expect(result.name).to eq 'commit_pipelines' expect(result.name).to eq 'commit_pipelines'
end end
it 'matches new merge request pipelines endpoint' do it 'matches new merge request pipelines endpoint' do
env = build_env( request = build_request(
'/my-group/my-project/merge_requests/new.json' '/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).to be_present
expect(result.name).to eq 'new_merge_request_pipelines' expect(result.name).to eq 'new_merge_request_pipelines'
end end
it 'matches merge request pipelines endpoint' do it 'matches merge request pipelines endpoint' do
env = build_env( request = build_request(
'/my-group/my-project/merge_requests/234/pipelines.json' '/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).to be_present
expect(result.name).to eq 'merge_request_pipelines' expect(result.name).to eq 'merge_request_pipelines'
end end
it 'matches build endpoint' do it 'matches build endpoint' do
env = build_env( request = build_request(
'/my-group/my-project/builds/234.json' '/my-group/my-project/builds/234.json'
) )
result = described_class.match(env) result = described_class.match(request)
expect(result).to be_present expect(result).to be_present
expect(result.name).to eq 'project_build' expect(result.name).to eq 'project_build'
end end
it 'does not match blob with confusing name' do it 'does not match blob with confusing name' do
env = build_env( request = build_request(
'/my-group/my-project/blob/master/pipelines.json' '/my-group/my-project/blob/master/pipelines.json'
) )
result = described_class.match(env) result = described_class.match(request)
expect(result).to be_blank expect(result).to be_blank
end end
it 'matches the environments path' do it 'matches the environments path' do
env = build_env( request = build_request(
'/my-group/my-project/environments.json' '/my-group/my-project/environments.json'
) )
result = described_class.match(env) result = described_class.match(request)
expect(result).to be_present expect(result).to be_present
expect(result.name).to eq 'environments' expect(result.name).to eq 'environments'
end end
it 'matches pipeline#show endpoint' do it 'matches pipeline#show endpoint' do
env = build_env( request = build_request(
'/my-group/my-project/pipelines/2.json' '/my-group/my-project/pipelines/2.json'
) )
result = described_class.match(env) result = described_class.match(request)
expect(result).to be_present expect(result).to be_present
expect(result.name).to eq 'project_pipeline' expect(result.name).to eq 'project_pipeline'
end end
def build_env(path) def build_request(path)
{ 'PATH_INFO' => path } double(path_info: path)
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment