From f0bb0fea66b504aa14e0121aaf62f35d75438faa Mon Sep 17 00:00:00 2001 From: Jrester Date: Mon, 30 Mar 2020 10:38:15 +0200 Subject: [PATCH 1/4] Correclty encode url modified: src/awscr-signer/v4/uri.cr --- src/awscr-signer/v4/uri.cr | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/awscr-signer/v4/uri.cr b/src/awscr-signer/v4/uri.cr index 9b7450b..87ea143 100644 --- a/src/awscr-signer/v4/uri.cr +++ b/src/awscr-signer/v4/uri.cr @@ -14,7 +14,9 @@ module Awscr @query = QueryString.new def self.encode(path : String) - URI.encode(path) + String.build do |io| + URI.encode(path, io) { |byte| URI.unreserved?(byte) || byte.chr == '/' } + end end # The path must be non encoded. From 806d30735936906940911c8024f1c504d1be1521 Mon Sep 17 00:00:00 2001 From: Jrester Date: Wed, 1 Apr 2020 14:17:40 +0200 Subject: [PATCH 2/4] Do not force encoding of path modified: spec/awscr-signer/v4/request_spec.cr deleted: spec/awscr-signer/v4/uri_spec.cr modified: src/awscr-signer/signers/v4.cr modified: src/awscr-signer/v4/request.cr deleted: src/awscr-signer/v4/uri.cr --- spec/awscr-signer/v4/request_spec.cr | 16 ++--- spec/awscr-signer/v4/uri_spec.cr | 103 --------------------------- src/awscr-signer/signers/v4.cr | 8 +-- src/awscr-signer/v4/request.cr | 36 +++++++--- src/awscr-signer/v4/uri.cr | 52 -------------- 5 files changed, 39 insertions(+), 176 deletions(-) delete mode 100644 spec/awscr-signer/v4/uri_spec.cr delete mode 100644 src/awscr-signer/v4/uri.cr diff --git a/spec/awscr-signer/v4/request_spec.cr b/spec/awscr-signer/v4/request_spec.cr index 95c398a..44ff4f3 100644 --- a/spec/awscr-signer/v4/request_spec.cr +++ b/spec/awscr-signer/v4/request_spec.cr @@ -5,26 +5,26 @@ module Awscr describe Request do it "does not modify http request body" do body = IO::Memory.new("body") - request = Request.new("GET", URI.parse("/"), body) + request = Request.new("GET", "/", body) body.gets_to_end.should eq("body") end it "alerts of ignored query params" do expect_raises Exception do - Request.new("GET", URI.parse("http://google.com?test=1"), "") + Request.new("GET", "http://google.com?test=1", "") end end describe "digest" do it "returns unsigned payload if body is unsigned payload" do - request = Request.new("GET", URI.parse("/"), "UNSIGNED-PAYLOAD") + request = Request.new("GET", "/", "UNSIGNED-PAYLOAD") request.digest.should eq("UNSIGNED-PAYLOAD") end it "returns digest of body" do body = IO::Memory.new("body") - request = Request.new("GET", URI.parse("/"), body) + request = Request.new("GET", "/", body) request.digest.should eq("230d8358dc8e8890b4c58deeb62912ee2f20357ae92a5cc861b98e68fe31acb5") end @@ -32,12 +32,12 @@ module Awscr describe "host" do it "returns the uri host" do - request = Request.new("GET", URI.parse("/"), "") + request = Request.new("GET", "/", "") request.host.should eq(nil) end it "returns host if set" do - request = Request.new("GET", URI.parse("/"), "") + request = Request.new("GET", "/", "") request.headers.add("Host", "test") request.host.should eq("test") @@ -46,7 +46,7 @@ module Awscr describe "full_path" do it "returns the full url incl query string" do - request = Request.new("GET", URI.parse("http://google.com"), "") + request = Request.new("GET", "http://google.com", "") request.query.add("blah", "1") request.full_path.should eq("http://google.com?blah=1") @@ -56,7 +56,7 @@ module Awscr describe "to_s" do it "returns a valid string" do body = "" - request = Request.new("GET", URI.parse("/"), body) + request = Request.new("GET", "/", body) request.headers.add("Content-Type", "application/x-www-form-urlencoded; charset=utf-8") request.headers.add("Host", "iam.amazonaws.com") diff --git a/spec/awscr-signer/v4/uri_spec.cr b/spec/awscr-signer/v4/uri_spec.cr deleted file mode 100644 index 98b2b2d..0000000 --- a/spec/awscr-signer/v4/uri_spec.cr +++ /dev/null @@ -1,103 +0,0 @@ -require "../../spec_helper" - -module Awscr - module Signer::V4 - describe Uri do - it "can be created from a string" do - curi = Uri.new("https://www.google.com/blah") - - curi.to_s.should eq("https://www.google.com/blah") - end - - describe "to_s" do - it "full uri as string" do - curi = Uri.new( - URI.parse("https://www.google.com/blah") - ) - - curi.to_s.should eq("https://www.google.com/blah") - end - end - - describe "host" do - it "returns host" do - curi = Uri.new("https://www.google.com/blah") - - curi.host.should eq("www.google.com") - end - - it "gives nil if no host" do - curi = Uri.new("/") - - curi.host.should eq(nil) - end - end - - describe "path" do - it "supports empty" do - curi = Uri.new( - URI.parse("/") - ) - - curi.path.should eq "/" - end - - it "supports slash" do - curi = Uri.new( - URI.parse("/") - ) - - curi.path.should eq "/" - end - - it "supports relative uri" do - curi = Uri.new( - URI.parse("/test/../?x=1") - ) - - curi.path.should eq "/" - end - - it "handles utf8 reasonably" do - curi = Uri.new( - URI.parse("/ሴ") - ) - - curi.path.should eq "/%E1%88%B4" - end - - it "encodes the uri" do - curi = Uri.new( - URI.parse("/test?x=1") - ) - - curi.path.should eq "/test" - end - - it "can handle spaces" do - curi = Uri.new("/test ing") - - curi.path.should eq "/test%20ing" - end - - it "can handle multiple slashes" do - curi = Uri.new("/test ing/stuff") - - curi.path.should eq "/test%20ing/stuff" - end - - it "can handle a path that has a query string" do - curi = Uri.new("?list-type=2") - - curi.path.should eq "/" - end - - it "can handle a path that has a query string and a slash prefixing it" do - curi = Uri.new("/?list-type=2") - - curi.path.should eq "/" - end - end - end - end -end diff --git a/src/awscr-signer/signers/v4.cr b/src/awscr-signer/signers/v4.cr index f0ef12c..64bda4f 100644 --- a/src/awscr-signer/signers/v4.cr +++ b/src/awscr-signer/signers/v4.cr @@ -25,8 +25,8 @@ module Awscr end # Sign an HTTP::Request - def sign(request : HTTP::Request, add_sha = true) - header_impl(request, add_sha) + def sign(request : HTTP::Request, add_sha = true, encode_path = true) + header_impl(request, add_sha, encode_path) end def presign(request) @@ -57,7 +57,7 @@ module Awscr request.query_params.add("X-Amz-Signature", signature.to_s) end - private def header_impl(request, add_sha) + private def header_impl(request, add_sha, encode_path) scope = Signer::Scope.new(@region, @service) @amz_security_token.try do |token| @@ -74,7 +74,7 @@ module Awscr end canonical_request = Signer::V4::Request.new(request.method, - URI.parse(request.path), request.body) + request.path, request.body, encode_path) request.query_params.to_h.each do |k, v| canonical_request.query.add(k, v) diff --git a/src/awscr-signer/v4/request.cr b/src/awscr-signer/v4/request.cr index db8e5f2..6236dbf 100644 --- a/src/awscr-signer/v4/request.cr +++ b/src/awscr-signer/v4/request.cr @@ -30,17 +30,14 @@ module Awscr # The request body getter body - @uri : Uri + @path : String @digest : String @body : IO @query : QueryString + @encode : Bool - def initialize(method : String, uri : URI, body : IO | String | Nil) - raise Exception.new("You may not give a URI with query params, they are - ignored. Use #query object intead") unless uri.query.nil? - - @method = method - @uri = Uri.new(uri) + def initialize(@method : String, path : String, body : IO | String | Nil, @encode=true) + @path = build_path(path) @query = QueryString.new @headers = HeaderCollection.new @body = build_body(body) @@ -52,14 +49,14 @@ module Awscr end def full_path - "#{@uri}?#{query}" + "#{@path}?#{query}" end # Returns the request as a String. def to_s(io : IO) io << [ @method, - @uri.path, + @path, query, headers, @headers.keys.join(";"), @@ -83,6 +80,27 @@ module Awscr end end + # Encodes the path except '/' and '~' + def self.encode_path(path) + String.build do |io| + URI.encode(path, io) { |byte| URI.unreserved?(byte) || byte.chr == '/' || byte.chr == '~' } + end + end + + private def build_path(path) + return "/" if path.blank? + uri = URI.parse(path) + raise Exception.new("You may not give a URI with query params, they are + ignored. Use #query object intead") unless uri.query.nil? + + path = uri.normalize.path + if @encode + self.class.encode_path(path) + else + path + end + end + private def build_body(body) case body when IO diff --git a/src/awscr-signer/v4/uri.cr b/src/awscr-signer/v4/uri.cr deleted file mode 100644 index 87ea143..0000000 --- a/src/awscr-signer/v4/uri.cr +++ /dev/null @@ -1,52 +0,0 @@ -require "uri" - -module Awscr - module Signer::V4 - # A Uri - # - # ``` - # uri = Uri.new(::URI.parse("/")) - # uri.path - # ``` - class Uri - @uri : URI - - @query = QueryString.new - - def self.encode(path : String) - String.build do |io| - URI.encode(path, io) { |byte| URI.unreserved?(byte) || byte.chr == '/' } - end - end - - # The path must be non encoded. - def initialize(path : String) - @uri = URI.parse(path) - end - - # The path must be non encoded. - def initialize(@uri : URI) - end - - # Returns the uri encoded - def to_s(io : IO) - scheme = @uri.scheme ? @uri.scheme : "http" - io << "#{scheme}://#{@uri.host}#{@uri.path}" - end - - # Returns the host of the UI - def host - @uri.host - end - - # Returns path of the `Uri`, normalizes the path, and encode the path as - # required by version 4 - def path - uri = @uri.normalize - path = uri.path.to_s - path = "/" if path.blank? - self.class.encode(path) - end - end - end -end From 17f2ea79f07d2837ec89baea6318f69f6b231ccb Mon Sep 17 00:00:00 2001 From: Jrester Date: Sat, 4 Apr 2020 14:16:12 +0200 Subject: [PATCH 3/4] Add encoding options for presigned url modified: src/awscr-signer/signers/v4.cr --- src/awscr-signer/signers/v4.cr | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/awscr-signer/signers/v4.cr b/src/awscr-signer/signers/v4.cr index 64bda4f..2ae2732 100644 --- a/src/awscr-signer/signers/v4.cr +++ b/src/awscr-signer/signers/v4.cr @@ -29,18 +29,18 @@ module Awscr header_impl(request, add_sha, encode_path) end - def presign(request) - querystring_impl(request) + def presign(request, encode_path = true) + querystring_impl(request, encode_path) end - private def querystring_impl(request) + private def querystring_impl(request, encode_path) scope = Signer::Scope.new(@region, @service) request.query_params.add("X-Amz-Algorithm", Signer::ALGORITHM) request.query_params.add("X-Amz-Credential", "#{@credentials.key}/#{scope}") request.query_params.add("X-Amz-Date", scope.date.iso8601) canonical_request = Signer::V4::Request.new(request.method, - URI.parse(request.path), request.body) + request.path, request.body, encode_path) request.query_params.to_h.each do |k, v| canonical_request.query.add(k, v) From facd6987fde3ca10225305fd7d9aa61fcc7c3e4e Mon Sep 17 00:00:00 2001 From: Jrester Date: Wed, 29 Apr 2020 12:59:05 +0200 Subject: [PATCH 4/4] Remove full_path from request modified: spec/awscr-signer/v4/request_spec.cr modified: src/awscr-signer/v4/request.cr --- spec/awscr-signer/v4/request_spec.cr | 9 --------- src/awscr-signer/v4/request.cr | 4 ---- 2 files changed, 13 deletions(-) diff --git a/spec/awscr-signer/v4/request_spec.cr b/spec/awscr-signer/v4/request_spec.cr index 44ff4f3..3ba94ad 100644 --- a/spec/awscr-signer/v4/request_spec.cr +++ b/spec/awscr-signer/v4/request_spec.cr @@ -44,15 +44,6 @@ module Awscr end end - describe "full_path" do - it "returns the full url incl query string" do - request = Request.new("GET", "http://google.com", "") - request.query.add("blah", "1") - - request.full_path.should eq("http://google.com?blah=1") - end - end - describe "to_s" do it "returns a valid string" do body = "" diff --git a/src/awscr-signer/v4/request.cr b/src/awscr-signer/v4/request.cr index 6236dbf..49ce3f2 100644 --- a/src/awscr-signer/v4/request.cr +++ b/src/awscr-signer/v4/request.cr @@ -48,10 +48,6 @@ module Awscr @headers["Host"]?.try(&.value) end - def full_path - "#{@path}?#{query}" - end - # Returns the request as a String. def to_s(io : IO) io << [