From 8d77e99827936179248b419448c0974c44d72e3a Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Fri, 31 Jan 2020 09:09:34 +1000 Subject: [PATCH] FIX: Stop encoding presigned URLs with UrlHelper (#8818) When FinalDestination is given a URL it encodes it before doing anything else. however S3 presigned URLs should not be messed with in any way otherwise we can end up with 400 errors when downloading the URL e.g. InvalidTokenThe provided token is malformed or otherwise invalid. The signature of presigned URLs is very important and is automatically generated and should be preserved. --- lib/url_helper.rb | 5 +++++ spec/components/url_helper_spec.rb | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/lib/url_helper.rb b/lib/url_helper.rb index 3345a37fd90..e1299c33585 100644 --- a/lib/url_helper.rb +++ b/lib/url_helper.rb @@ -63,9 +63,14 @@ class UrlHelper # Prevents double URL encode # https://stackoverflow.com/a/37599235 def self.escape_uri(uri) + return uri if s3_presigned_url?(uri) UrlHelper.encode_component(CGI.unescapeHTML(UrlHelper.unencode(uri))) end + def self.s3_presigned_url?(url) + (url.downcase =~ /x-amz-algorithm|x-amz-credential/).present? + end + def self.cook_url(url, secure: false) return url unless is_local(url) diff --git a/spec/components/url_helper_spec.rb b/spec/components/url_helper_spec.rb index 3dd4360a6f4..72ac5acacaf 100644 --- a/spec/components/url_helper_spec.rb +++ b/spec/components/url_helper_spec.rb @@ -118,6 +118,14 @@ describe UrlHelper do url = UrlHelper.escape_uri('http://example.com/foo%20bar/foo bar/') expect(url).to eq('http://example.com/foo%20bar/foo%20bar/') end + + it "doesn't escape S3 presigned URLs" do + # both of these were originally real presigned URLs and have had all + # sensitive information stripped + presigned_url = "https://test.com/original/3X/b/5/575bcc2886bf7a39684b57ca90be85f7d399bbc7.png?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AK8888999977%2F20200130%2Fus-west-1%2Fs3%2Faws4_request&X-Amz-Date=20200130T064355Z&X-Amz-Expires=15&X-Amz-SignedHeaders=host&X-Amz-Security-Token=blahblah%2Bblahblah%2Fblah%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FwEQAR&X-Amz-Signature=test" + encoded_presigned_url = "https://test.com/original/3X/b/5/575bcc2886bf7a39684b57ca90be85f7d399bbc7.png?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AK8888999977/20200130/us-west-1/s3/aws4_request&X-Amz-Date=20200130T064355Z&X-Amz-Expires=15&X-Amz-SignedHeaders=host&X-Amz-Security-Token=blahblah+blahblah/blah//////////wEQA==&X-Amz-Signature=test" + expect(UrlHelper.escape_uri(presigned_url)).not_to eq(encoded_presigned_url) + end end describe "#local_cdn_url" do