Commit ed6de53e authored by Peter Leitzen's avatar Peter Leitzen

Merge branch '217553-permanent-links-to-release-assets-not-found-404' into 'master'

Fix release assets link redirection

See merge request gitlab-org/gitlab!35381
parents c22d72d1 bdd3d2b3
...@@ -54,7 +54,7 @@ class Projects::ReleasesController < Projects::ApplicationController ...@@ -54,7 +54,7 @@ class Projects::ReleasesController < Projects::ApplicationController
end end
def sanitized_filepath def sanitized_filepath
CGI.unescape(params[:filepath]) "/#{CGI.unescape(params[:filepath])}"
end end
def sanitized_tag_name def sanitized_tag_name
......
...@@ -24,10 +24,8 @@ module Types ...@@ -24,10 +24,8 @@ module Types
def direct_asset_url def direct_asset_url
return object.url unless object.filepath return object.url unless object.filepath
release = object.release release = object.release.present
project = release.project release.download_url(object.filepath)
Gitlab::Routing.url_helpers.project_release_url(project, release) << object.filepath
end end
end end
end end
...@@ -71,6 +71,12 @@ class ReleasePresenter < Gitlab::View::Presenter::Delegated ...@@ -71,6 +71,12 @@ class ReleasePresenter < Gitlab::View::Presenter::Delegated
can_download_code? ? release.name : "Release-#{release.id}" can_download_code? ? release.name : "Release-#{release.id}"
end end
def download_url(filepath)
filepath = filepath.sub(%r{^/}, '') if filepath.start_with?('/')
downloads_project_release_url(project, release, filepath)
end
private private
def can_download_code? def can_download_code?
......
---
title: Fix release assets link redirection
merge_request: 35381
author:
type: fixed
...@@ -14,10 +14,8 @@ module API ...@@ -14,10 +14,8 @@ module API
def direct_asset_url def direct_asset_url
return object.url unless object.filepath return object.url unless object.filepath
release = object.release release = object.release.present
project = release.project release.download_url(object.filepath)
Gitlab::Routing.url_helpers.project_release_url(project, release) << object.filepath
end end
end end
end end
......
...@@ -201,102 +201,7 @@ RSpec.describe Projects::ReleasesController do ...@@ -201,102 +201,7 @@ RSpec.describe Projects::ReleasesController do
end end
end end
context 'GET #downloads' do # `GET #downloads` is addressed in spec/requests/projects/releases_controller_spec.rb
subject do
get :downloads, params: { namespace_id: project.namespace, project_id: project, tag: tag, filepath: filepath }
end
before do
sign_in(user)
end
let(:release) { create(:release, project: project, tag: tag ) }
let!(:link) { create(:release_link, release: release, name: 'linux-amd64 binaries', filepath: '/binaries/linux-amd64', url: 'https://downloads.example.com/bin/gitlab-linux-amd64') }
let(:tag) { 'v11.9.0-rc2' }
context 'valid filepath' do
let(:filepath) { CGI.escape('/binaries/linux-amd64') }
it 'redirects to the asset direct link' do
subject
expect(response).to redirect_to('https://downloads.example.com/bin/gitlab-linux-amd64')
end
it 'redirects with a status of 302' do
subject
expect(response).to have_gitlab_http_status(:redirect)
end
end
context 'invalid filepath' do
let(:filepath) { CGI.escape('/binaries/win32') }
it 'is not found' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
context 'GET #downloads' do
subject do
get :downloads, params: {
namespace_id: project.namespace,
project_id: project,
tag: tag,
filepath: filepath
}
end
before do
sign_in(user)
end
let(:release) { create(:release, project: project, tag: tag ) }
let(:tag) { 'v11.9.0-rc2' }
let(:db_filepath) { '/binaries/linux-amd64' }
let!(:link) do
create :release_link,
release: release,
name: 'linux-amd64 binaries',
filepath: db_filepath,
url: 'https://downloads.example.com/bin/gitlab-linux-amd64'
end
context 'valid filepath' do
let(:filepath) { CGI.escape('/binaries/linux-amd64') }
it 'redirects to the asset direct link' do
subject
expect(response).to redirect_to(link.url)
end
end
context 'invalid filepath' do
let(:filepath) { CGI.escape('/binaries/win32') }
it 'is not found' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'ignores filepath extension' do
let(:db_filepath) { '/binaries/linux-amd64.json' }
let(:filepath) { CGI.escape(db_filepath) }
it 'redirects to the asset direct link' do
subject
expect(response).to redirect_to(link.url)
end
end
end
private private
......
...@@ -38,7 +38,7 @@ RSpec.describe 'User views releases', :js do ...@@ -38,7 +38,7 @@ RSpec.describe 'User views releases', :js do
context 'when there is a link as an asset' do context 'when there is a link as an asset' do
let!(:release_link) { create(:release_link, release: release_v1, url: url ) } let!(:release_link) { create(:release_link, release: release_v1, url: url ) }
let(:url) { "#{project.web_url}/-/jobs/1/artifacts/download" } let(:url) { "#{project.web_url}/-/jobs/1/artifacts/download" }
let(:direct_asset_link) { Gitlab::Routing.url_helpers.project_release_url(project, release_v1) << release_link.filepath } let(:direct_asset_link) { Gitlab::Routing.url_helpers.project_release_url(project, release_v1) << "/downloads#{release_link.filepath}" }
it 'sees the link' do it 'sees the link' do
page.within("##{release_v1.tag} .js-assets-list") do page.within("##{release_v1.tag} .js-assets-list") do
......
...@@ -18,7 +18,7 @@ Object { ...@@ -18,7 +18,7 @@ Object {
"count": 8, "count": 8,
"links": Array [ "links": Array [
Object { Object {
"directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/binaries/awesome-app-3", "directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/downloads/binaries/awesome-app-3",
"external": true, "external": true,
"id": "gid://gitlab/Releases::Link/13", "id": "gid://gitlab/Releases::Link/13",
"linkType": "image", "linkType": "image",
...@@ -26,7 +26,7 @@ Object { ...@@ -26,7 +26,7 @@ Object {
"url": "https://example.com/image", "url": "https://example.com/image",
}, },
Object { Object {
"directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/binaries/awesome-app-2", "directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/downloads/binaries/awesome-app-2",
"external": true, "external": true,
"id": "gid://gitlab/Releases::Link/12", "id": "gid://gitlab/Releases::Link/12",
"linkType": "package", "linkType": "package",
...@@ -34,7 +34,7 @@ Object { ...@@ -34,7 +34,7 @@ Object {
"url": "https://example.com/package", "url": "https://example.com/package",
}, },
Object { Object {
"directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/binaries/awesome-app-1", "directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/downloads/binaries/awesome-app-1",
"external": false, "external": false,
"id": "gid://gitlab/Releases::Link/11", "id": "gid://gitlab/Releases::Link/11",
"linkType": "runbook", "linkType": "runbook",
...@@ -42,7 +42,7 @@ Object { ...@@ -42,7 +42,7 @@ Object {
"url": "http://localhost/releases-namespace/releases-project/runbook", "url": "http://localhost/releases-namespace/releases-project/runbook",
}, },
Object { Object {
"directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/binaries/linux-amd64", "directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/downloads/binaries/linux-amd64",
"external": true, "external": true,
"id": "gid://gitlab/Releases::Link/10", "id": "gid://gitlab/Releases::Link/10",
"linkType": "other", "linkType": "other",
...@@ -146,7 +146,7 @@ Object { ...@@ -146,7 +146,7 @@ Object {
"count": 8, "count": 8,
"links": Array [ "links": Array [
Object { Object {
"directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/binaries/awesome-app-3", "directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/downloads/binaries/awesome-app-3",
"external": true, "external": true,
"id": "gid://gitlab/Releases::Link/13", "id": "gid://gitlab/Releases::Link/13",
"linkType": "image", "linkType": "image",
...@@ -154,7 +154,7 @@ Object { ...@@ -154,7 +154,7 @@ Object {
"url": "https://example.com/image", "url": "https://example.com/image",
}, },
Object { Object {
"directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/binaries/awesome-app-2", "directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/downloads/binaries/awesome-app-2",
"external": true, "external": true,
"id": "gid://gitlab/Releases::Link/12", "id": "gid://gitlab/Releases::Link/12",
"linkType": "package", "linkType": "package",
...@@ -162,7 +162,7 @@ Object { ...@@ -162,7 +162,7 @@ Object {
"url": "https://example.com/package", "url": "https://example.com/package",
}, },
Object { Object {
"directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/binaries/awesome-app-1", "directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/downloads/binaries/awesome-app-1",
"external": false, "external": false,
"id": "gid://gitlab/Releases::Link/11", "id": "gid://gitlab/Releases::Link/11",
"linkType": "runbook", "linkType": "runbook",
...@@ -170,7 +170,7 @@ Object { ...@@ -170,7 +170,7 @@ Object {
"url": "http://localhost/releases-namespace/releases-project/runbook", "url": "http://localhost/releases-namespace/releases-project/runbook",
}, },
Object { Object {
"directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/binaries/linux-amd64", "directAssetUrl": "http://localhost/releases-namespace/releases-project/-/releases/v1.1/downloads/binaries/linux-amd64",
"external": true, "external": true,
"id": "gid://gitlab/Releases::Link/10", "id": "gid://gitlab/Releases::Link/10",
"linkType": "other", "linkType": "other",
......
...@@ -121,7 +121,7 @@ RSpec.describe 'Creation of a new release' do ...@@ -121,7 +121,7 @@ RSpec.describe 'Creation of a new release' do
create_release create_release
release = mutation_response[:release] release = mutation_response[:release]
expected_direct_asset_url = Gitlab::Routing.url_helpers.project_release_url(project, Release.find_by(tag: tag_name)) << asset_link[:directAssetPath] expected_direct_asset_url = Gitlab::Routing.url_helpers.project_release_url(project, Release.find_by(tag: tag_name)) << "/downloads#{asset_link[:directAssetPath]}"
expected_attributes = { expected_attributes = {
tagName: tag_name, tagName: tag_name,
......
...@@ -147,7 +147,7 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do ...@@ -147,7 +147,7 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do
'name' => link.name, 'name' => link.name,
'url' => link.url, 'url' => link.url,
'external' => link.external?, 'external' => link.external?,
'directAssetUrl' => link.filepath ? Gitlab::Routing.url_helpers.project_release_url(project, release) << link.filepath : link.url 'directAssetUrl' => link.filepath ? Gitlab::Routing.url_helpers.project_release_url(project, release) << "/downloads#{link.filepath}" : link.url
} }
end end
......
...@@ -146,7 +146,7 @@ RSpec.describe API::Release::Links do ...@@ -146,7 +146,7 @@ RSpec.describe API::Release::Links do
specify do specify do
get api("/projects/#{project.id}/releases/v0.1/assets/links/#{link.id}", maintainer) get api("/projects/#{project.id}/releases/v0.1/assets/links/#{link.id}", maintainer)
expect(json_response['direct_asset_url']).to eq("http://localhost/#{project.namespace.path}/#{project.name}/-/releases/#{release.tag}/bin/bigfile.exe") expect(json_response['direct_asset_url']).to eq("http://localhost/#{project.namespace.path}/#{project.name}/-/releases/#{release.tag}/downloads/bin/bigfile.exe")
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Projects::ReleasesController' do
let_it_be(:project) { create(:project, :repository) }
let_it_be(:user) { create(:user) }
before do
project.add_developer(user)
login_as(user)
end
# Added as a request spec because of https://gitlab.com/gitlab-org/gitlab/-/issues/232386
describe 'GET #downloads' do
context 'filepath redirection' do
let_it_be(:release) { create(:release, project: project, tag: 'v11.9.0-rc2' ) }
let!(:link) { create(:release_link, release: release, name: 'linux-amd64 binaries', filepath: filepath, url: 'https://aws.example.com/s3/project/bin/hello-darwin-amd64') }
let_it_be(:url) { "#{project_releases_path(project)}/#{release.tag}/downloads/bin/darwin-amd64" }
let(:subject) { get url }
context 'valid filepath' do
let(:filepath) { '/bin/darwin-amd64' }
it 'redirects to the asset direct link' do
subject
expect(response).to redirect_to('https://aws.example.com/s3/project/bin/hello-darwin-amd64')
end
it 'redirects with a status of 302' do
subject
expect(response).to have_gitlab_http_status(:redirect)
end
end
context 'invalid filepath' do
let(:filepath) { '/binaries/win32' }
it 'is not found' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
context 'invalid filepath' do
let(:invalid_filepath) { 'bin/darwin-amd64' }
let(:subject) { create(:release_link, name: 'linux-amd64 binaries', filepath: invalid_filepath, url: 'https://aws.example.com/s3/project/bin/hello-darwin-amd64') }
it 'cannot create an invalid filepath' do
expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
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