1. s3cmd
  2. Commit a6366f8dad2341a7ffa8094cf24f880e2c359333
Browse code

Handle headers with spaces in their values properly (#460)

If headers exist to be sent in a request, and the value for the header
contains a space character, the canonical header value being used for
v4 signing would be corrupted - the value was being split at the space
and the two parts of the value were appearing in different parts of
the canonical header after applying a string sort to it. Not good.

This patch keeps the canonical headers intact in dict format until
generating the final string value of the headers and their values, to
be signed.

It also updates the test case for --add-header to include a space in
the value of the header being added, which would have caught this
failure earlier.

Fixes https://github.com/s3tools/s3cmd/issues/460

Matt Domsch authored on 2015/01/18 22:51:05
Showing 2 changed files
S3/Crypto.py
History View file @ a6366f8
... ... 
@@ -97,18 +97,26 @@ def sign_string_v4(method='GET', host='', canonical_uri='/', params={}, region='
97 97 
     else:
98 98 
         payload_hash = sha256(body).hexdigest()
99 99
100 
-    canonical_headers = 'host:' + host + '\n' + 'x-amz-content-sha256:' + payload_hash + '\n' + 'x-amz-date:' + amzdate + '\n'
100 
+    canonical_headers = {'host' : host,
101 
+                         'x-amz-content-sha256': payload_hash,
102 
+                         'x-amz-date' : amzdate
103 
+                         }
101 104 
     signed_headers = 'host;x-amz-content-sha256;x-amz-date'
102 105
103 106 
     for header in cur_headers.keys():
104 107 
         # avoid duplicate headers and previous Authorization
105 108 
         if header == 'Authorization' or header in signed_headers.split(';'):
106 109 
             continue
107 
-        canonical_headers += header.strip() + ':' + str(cur_headers[header]).strip() + '\n'
110 
+        canonical_headers[header.strip()] = str(cur_headers[header]).strip()
108 111 
         signed_headers += ';' + header.strip()
109 112
110 
-    # sort headers
111 
-    canonical_headers = '\n'.join(sorted(canonical_headers.split())) + '\n'
113 
+    # sort headers into a string
114 
+    canonical_headers_str = ''
115 
+    for k, v in sorted(canonical_headers.items()):
116 
+        canonical_headers_str += k + ":" + v + "\n"
117 
+
118 
+    canonical_headers = canonical_headers_str
119 
+    debug(u"canonical_headers = %s" % canonical_headers)
112 120 
     signed_headers = ';'.join(sorted(signed_headers.split(';')))
113 121
114 122 
     canonical_request = method + '\n' + canonical_uri + '\n' + canonical_querystring + '\n' + canonical_headers + '\n' + signed_headers + '\n' + payload_hash
run-tests.py
History View file @ a6366f8
... ... 
@@ -508,7 +508,7 @@ test_s3cmd("Verify ACL and MIME type", ['info', '%s/copy/etc2/Logo.PNG' % pbucke
508 508 
                      "ACL:.*\*anon\*: READ",
509 509 
                      "URL:.*http://%s.%s/copy/etc2/Logo.PNG" % (bucket(2), cfg.host_base) ])
510 510
511 
-test_s3cmd("Add cache-control header", ['modify', '--add-header=cache-control: max-age=3600', '%s/copy/etc2/Logo.PNG' % pbucket(2) ],
511 
+test_s3cmd("Add cache-control header", ['modify', '--add-header=cache-control: max-age=3600, public', '%s/copy/etc2/Logo.PNG' % pbucket(2) ],
512 512 
     must_find_re = [ "File .* modified" ])
513 513
514 514 
 if have_wget: