Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

decode 'Content-MD5' checksum as base64-encoded string #149

Merged
2 commits merged into from

2 participants

@ghost

decode 'Content-MD5' checksum as base64-encoded string.
in the RFC the header is defined as:

    Content-MD5   = "Content-MD5" ":" md5-digest
    md5-digest   = <base64 of 128 bit MD5 digest as per RFC 1864>

closes #97

@seancribbs seancribbs commented on the diff
lib/webmachine/decision/flow.rb
@@ -81,7 +84,7 @@ def b9a
response.body = "Content-MD5 header does not match request body."
400
else # not_validated
- if request.content_md5 == Digest::MD5.hexdigest(request.body)
+ if decode64(request.content_md5) == Digest::MD5.hexdigest(request.body)
@seancribbs Owner

Does decode64 raise an exception if it cannot decode the string? If so, this should be rescued.

@ghost
ghost added a note

not sure, I'll look into that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@seancribbs seancribbs commented on the diff
spec/webmachine/decision/flow_spec.rb
@@ -112,29 +112,41 @@ def validate_content_checksum; @validation; end
let(:body) { "This is the body." }
let(:headers) { Webmachine::Headers["Content-Type" => "text/plain"] }
+ it "should respond with 204 when the request body does match the header" do
+ headers['Content-MD5'] = Base64.encode64 Digest::MD5.hexdigest(body)
+ subject.run
+ response.code.should == 204
+ end
+
+ it "should respond with 400 when the header is not encoded as Base64 but digest matches the body" do
+ headers['Content-MD5'] = Digest::MD5.hexdigest(body)
+ subject.run
+ response.code.should == 400
+ end
@seancribbs Owner

Actually, I think this satisfies my question above, but a test with a header that is explicitly non-hashed, non-base64-encoded would be nice. Just to test the edge-case, of course.

@ghost
ghost added a note

sure, I can try empty string as value in addition to a non-hashed, non-encoded body string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@seancribbs
Owner

Other than the concerns I raised above, good work. The more compliant with the spec, the better!

@ghost

@seancribbs thanks. I added a few more specs in 2a2f884

@strcmp strcmp decode 'Content-MD5' checksum as base64-encoded string.
in the RFC the header is defined as:

        Content-MD5   = "Content-MD5" ":" md5-digest
        md5-digest   = <base64 of 128 bit MD5 digest as per RFC 1864>

closes #97
6190a28
@ghost

@seancribbs squashed my changes into a single commit(6190a28). if it looks good, I can merge.

@seancribbs
Owner

:+1: to merge

@ghost

thanks!

@ghost ghost merged commit 003775e into master

1 check passed

Details default The Travis CI build passed
@ghost ghost deleted the content-md5 branch
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 8, 2014
  1. @strcmp
Commits on Feb 10, 2014
  1. @strcmp

    decode 'Content-MD5' checksum as base64-encoded string.

    strcmp authored
    in the RFC the header is defined as:
    
            Content-MD5   = "Content-MD5" ":" md5-digest
            md5-digest   = <base64 of 128 bit MD5 digest as per RFC 1864>
    
    closes #97
This page is out of date. Refresh to see the latest.
View
4 CHANGELOG.md
@@ -1,3 +1,7 @@
+### HEAD
+
+* decode the value of the header 'Content-MD5' as base64-encoded string.
+
### 1.2.2 January 8, 2014
1.2.2 is a bugfix/patch release that expands functionality with some edge
View
5 lib/webmachine/decision/flow.rb
@@ -1,5 +1,6 @@
require 'time'
require 'digest/md5'
+require 'base64'
require 'webmachine/decision/conneg'
require 'webmachine/decision/falsey'
require 'webmachine/translation'
@@ -16,6 +17,8 @@ module Decision
# of the chart.
# @see https://raw.github.com/wiki/basho/webmachine/images/http-headers-status-v3.png
module Flow
+ include Base64
+
# Version of the flow diagram
VERSION = 3
@@ -81,7 +84,7 @@ def b9a
response.body = "Content-MD5 header does not match request body."
400
else # not_validated
- if request.content_md5 == Digest::MD5.hexdigest(request.body)
+ if decode64(request.content_md5) == Digest::MD5.hexdigest(request.body)
@seancribbs Owner

Does decode64 raise an exception if it cannot decode the string? If so, this should be rescued.

@ghost
ghost added a note

not sure, I'll look into that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
:b9b
else
response.body = "Content-MD5 header does not match request body."
View
31 lib/webmachine/request.rb
@@ -9,7 +9,20 @@ class Request
attr_reader :method, :uri, :headers, :body
attr_accessor :disp_path, :path_info, :path_tokens
- STANDARD_HTTP_METHODS = %w[GET HEAD POST PUT DELETE TRACE CONNECT OPTIONS]
+ GET_METHOD = "GET"
+ HEAD_METHOD = "HEAD"
+ POST_METHOD = "POST"
+ PUT_METHOD = "PUT"
+ DELETE_METHOD = "DELETE"
+ OPTIONS_METHOD = "OPTIONS"
+ TRACE_METHOD = "TRACE"
+ CONNECT_METHOD = "CONNECT"
+
+ STANDARD_HTTP_METHODS = [
+ GET_METHOD, HEAD_METHOD, POST_METHOD,
+ PUT_METHOD, DELETE_METHOD, TRACE_METHOD,
+ CONNECT_METHOD, OPTIONS_METHOD
+ ].map!(&:freeze)
# @param [String] method the HTTP request method
# @param [URI] uri the requested URI, including host, scheme and
@@ -92,7 +105,7 @@ def https?
# @return [Boolean]
# true if this request was made with the GET method
def get?
- method == "GET"
+ method == GET_METHOD
end
# Is this a HEAD request?
@@ -100,7 +113,7 @@ def get?
# @return [Boolean]
# true if this request was made with the HEAD method
def head?
- method == "HEAD"
+ method == HEAD_METHOD
end
# Is this a POST request?
@@ -108,7 +121,7 @@ def head?
# @return [Boolean]
# true if this request was made with the GET method
def post?
- method == "POST"
+ method == POST_METHOD
end
# Is this a PUT request?
@@ -116,7 +129,7 @@ def post?
# @return [Boolean]
# true if this request was made with the PUT method
def put?
- method == "PUT"
+ method == PUT_METHOD
end
# Is this a DELETE request?
@@ -124,7 +137,7 @@ def put?
# @return [Boolean]
# true if this request was made with the DELETE method
def delete?
- method == "DELETE"
+ method == DELETE_METHOD
end
# Is this a TRACE request?
@@ -132,7 +145,7 @@ def delete?
# @return [Boolean]
# true if this request was made with the TRACE method
def trace?
- method == "TRACE"
+ method == TRACE_METHOD
end
# Is this a CONNECT request?
@@ -140,7 +153,7 @@ def trace?
# @return [Boolean]
# true if this request was made with the CONNECT method
def connect?
- method == "CONNECT"
+ method == CONNECT_METHOD
end
# Is this an OPTIONS request?
@@ -148,7 +161,7 @@ def connect?
# @return [Boolean]
# true if this request was made with the OPTIONS method
def options?
- method == "OPTIONS"
+ method == OPTIONS_METHOD
end
end # class Request
View
38 spec/webmachine/decision/flow_spec.rb
@@ -112,29 +112,59 @@ def validate_content_checksum; @validation; end
let(:body) { "This is the body." }
let(:headers) { Webmachine::Headers["Content-Type" => "text/plain"] }
+ it "should respond with 204 when the request body does match the header" do
+ headers['Content-MD5'] = Base64.encode64 Digest::MD5.hexdigest(body)
+ subject.run
+ response.code.should == 204
+ end
+
+ it "should bypass validation when the header has a nil value" do
+ headers['Content-MD5'] = nil
+ subject.run
+ response.code.should == 204
+ end
+
+ it "should respond with 400 when the header has a empty string value" do
+ headers['Content-MD5'] = ""
+ subject.run
+ response.code.should == 400
+ end
+
+ it "should respond with 400 when the header has a non-hashed, non-encoded value" do
+ headers["Content-MD5"] = body
+ subject.run
+ response.code.should == 400
+ end
+
+ it "should respond with 400 when the header is not encoded as Base64 but digest matches the body" do
+ headers['Content-MD5'] = Digest::MD5.hexdigest(body)
+ subject.run
+ response.code.should == 400
+ end
@seancribbs Owner

Actually, I think this satisfies my question above, but a test with a header that is explicitly non-hashed, non-base64-encoded would be nice. Just to test the edge-case, of course.

@ghost
ghost added a note

sure, I can try empty string as value in addition to a non-hashed, non-encoded body string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
it "should respond with 400 when the request body does not match the header" do
- headers['Content-MD5'] = "thiswillnotmatchthehash"
+ headers['Content-MD5'] = Base64.encode64 Digest::MD5.hexdigest("thiswillnotmatchthehash")
subject.run
response.code.should == 400
end
it "should respond with 400 when the resource invalidates the checksum" do
resource.validation = false
- headers['Content-MD5'] = "thiswillnotmatchthehash"
+ headers['Content-MD5'] = Base64.encode64 Digest::MD5.hexdigest("thiswillnotmatchthehash")
subject.run
response.code.should == 400
end
it "should not respond with 400 when the resource validates the checksum" do
resource.validation = true
- headers['Content-MD5'] = "thiswillnotmatchthehash"
+ headers['Content-MD5'] = Base64.encode64 Digest::MD5.hexdigest("thiswillnotmatchthehash")
subject.run
response.code.should_not == 400
end
it "should respond with the given code when the resource returns a code while validating" do
resource.validation = 500
- headers['Content-MD5'] = "thiswillnotmatchthehash"
+ headers['Content-MD5'] = Base64.encode64 Digest::MD5.hexdigest("thiswillnotmatchthehash")
subject.run
response.code.should == 500
end
Something went wrong with that request. Please try again.