diff --git a/README.rdoc b/README.rdoc
index bbbfe927..a0c5317c 100644
--- a/README.rdoc
+++ b/README.rdoc
@@ -1018,6 +1018,24 @@ try {rack-cache}[http://rtomayko.github.com/rack-cache/]:
Use the :static_cache_control setting (see below) to add
Cache-Control header info to static files.
+According to RFC 2616 your application should behave differently if the
+If-Matches of If-None-Matches header is set to * depending on whether
+the resource requested is already in existence. Sinatra assumes resources for
+safe (like get) and idempotent (like put) requests are already in existence,
+whereas other resources (for instance for post requests), are treated as new
+resources. You can change this behavior by passing in a :new_resource
+option:
+
+ get '/create' do
+ etag '', :new_resource => true
+ Article.create
+ erb :new_article
+ end
+
+If you still want to use a weak ETag, pass in a :kind option:
+
+ etag '', :new_resource => true, :kind => :weak
+
=== Sending Files
For sending files, you can use the send_file helper method:
diff --git a/lib/sinatra/base.rb b/lib/sinatra/base.rb
index df51d3d3..cf5a432b 100644
--- a/lib/sinatra/base.rb
+++ b/lib/sinatra/base.rb
@@ -355,8 +355,12 @@ module Sinatra
return unless time
time = time_for time
response['Last-Modified'] = time.httpdate
- # compare based on seconds since epoch
- halt 304 if Time.httpdate(env['HTTP_IF_MODIFIED_SINCE']).to_i >= time.to_i
+
+ unless env['HTTP_IF_NONE_MATCH']
+ # compare based on seconds since epoch
+ since = Time.httpdate(env['HTTP_IF_MODIFIED_SINCE']).to_i
+ halt 304 if since >= time.to_i
+ end
rescue ArgumentError
end
@@ -369,22 +373,36 @@ module Sinatra
# When the current request includes an 'If-None-Match' header with a
# matching etag, execution is immediately halted. If the request method is
# GET or HEAD, a '304 Not Modified' response is sent.
- def etag(value, kind = :strong)
- raise ArgumentError, ":strong or :weak expected" unless [:strong,:weak].include?(kind)
+ def etag(value, options = {})
+ # Before touching this code, please double check RFC 2616 14.24 and 14.26.
+ options = {:kind => options} unless Hash === options
+ kind = options[:kind] || :strong
+ new_resource = options.fetch(:new_resource) { request.post? }
+
+ unless [:strong, :weak].include?(kind)
+ raise ArgumentError, ":strong or :weak expected"
+ end
+
value = '"%s"' % value
value = 'W/' + value if kind == :weak
response['ETag'] = value
- if etags = env['HTTP_IF_NONE_MATCH']
- etags = etags.split(/\s*,\s*/)
- if etags.include?(value) or etags.include?('*')
- halt 304 if request.safe?
- else
- halt 412 unless request.safe?
+ if success? or status == 304
+ if etag_matches? env['HTTP_IF_NONE_MATCH'], new_resource
+ halt(request.safe? ? 304 : 412)
+ end
+
+ if env['HTTP_IF_MATCH']
+ halt 412 unless etag_matches? env['HTTP_IF_MATCH'], new_resource
end
end
end
+ def etag_matches?(list, new_resource = request.post?)
+ return !new_resource if list == '*'
+ list.to_s.split(/\s*,\s*/).include? response['ETag']
+ end
+
# Sugar for redirect (example: redirect back)
def back
request.referer
diff --git a/test/helper.rb b/test/helper.rb
index 972b416b..1f061d49 100644
--- a/test/helper.rb
+++ b/test/helper.rb
@@ -72,6 +72,10 @@ class Test::Unit::TestCase
assert_equal value.lstrip.gsub(/\s*\n\s*/, ""), body.lstrip.gsub(/\s*\n\s*/, "")
end
+ def assert_status(expected)
+ assert_equal Integer(expected), Integer(status)
+ end
+
def assert_like(a,b)
pattern = /id=['"][^"']*["']|\s+/
assert_equal a.strip.gsub(pattern, ""), b.strip.gsub(pattern, "")
diff --git a/test/helpers_test.rb b/test/helpers_test.rb
index ca6d1c82..46055dda 100644
--- a/test/helpers_test.rb
+++ b/test/helpers_test.rb
@@ -960,69 +960,622 @@ class HelpersTest < Test::Unit::TestCase
end
describe 'etag' do
- setup do
- mock_app {
- get '/' do
- body { 'Hello World' }
- etag 'FOO'
- 'Boo!'
+ context "safe requests" do
+ it 'returns 200 for normal requests' do
+ mock_app do
+ get '/' do
+ etag 'foo'
+ 'ok'
+ end
end
- post '/' do
- etag 'FOO'
- 'Matches!'
+ get('/')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ context "If-None-Match" do
+ it 'returns 304 when If-None-Match is *' do
+ mock_app do
+ get '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_NONE_MATCH' => '*')
+ assert_status 304
+ assert_body ''
end
- }
+
+ it 'returns 200 when If-None-Match is * for new resources' do
+ mock_app do
+ get '/' do
+ etag 'foo', :new_resource => true
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_NONE_MATCH' => '*')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 304 when If-None-Match is * for existing resources' do
+ mock_app do
+ get '/' do
+ etag 'foo', :new_resource => false
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_NONE_MATCH' => '*')
+ assert_status 304
+ assert_body ''
+ end
+
+ it 'returns 304 when If-None-Match is the etag' do
+ mock_app do
+ get '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_NONE_MATCH' => '"foo"')
+ assert_status 304
+ assert_body ''
+ end
+
+ it 'returns 304 when If-None-Match includes the etag' do
+ mock_app do
+ get '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_NONE_MATCH' => '"bar", "foo"')
+ assert_status 304
+ assert_body ''
+ end
+
+ it 'returns 200 when If-None-Match does not include the etag' do
+ mock_app do
+ get '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_NONE_MATCH' => '"bar"')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'ignores If-Modified-Since if If-None-Match does not match' do
+ mock_app do
+ get '/' do
+ etag 'foo'
+ last_modified Time.at(0)
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_NONE_MATCH' => '"bar"')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'does not change a status code other than 2xx or 304' do
+ mock_app do
+ get '/' do
+ status 499
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_NONE_MATCH' => '"foo"')
+ assert_status 499
+ assert_body 'ok'
+ end
+
+ it 'does change 2xx status codes' do
+ mock_app do
+ get '/' do
+ status 299
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_NONE_MATCH' => '"foo"')
+ assert_status 304
+ assert_body ''
+ end
+
+ it 'does not send a body on 304 status codes' do
+ mock_app do
+ get '/' do
+ status 304
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_NONE_MATCH' => '"foo"')
+ assert_status 304
+ assert_body ''
+ end
+ end
+
+ context "If-Match" do
+ it 'returns 200 when If-Match is the etag' do
+ mock_app do
+ get '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_MATCH' => '"foo"')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 200 when If-Match includes the etag' do
+ mock_app do
+ get '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_MATCH' => '"foo", "bar"')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 200 when If-Match is *' do
+ mock_app do
+ get '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_MATCH' => '*')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 412 when If-Match is * for new resources' do
+ mock_app do
+ get '/' do
+ etag 'foo', :new_resource => true
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_MATCH' => '*')
+ assert_status 412
+ assert_body ''
+ end
+
+ it 'returns 200 when If-Match is * for existing resources' do
+ mock_app do
+ get '/' do
+ etag 'foo', :new_resource => false
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_MATCH' => '*')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 412 when If-Match does not include the etag' do
+ mock_app do
+ get '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ get('/', {}, 'HTTP_IF_MATCH' => '"bar"')
+ assert_status 412
+ assert_body ''
+ end
+ end
end
- it 'sets the ETag header' do
- get '/'
- assert_equal '"FOO"', response['ETag']
+ context "idempotent requests" do
+ it 'returns 200 for normal requests' do
+ mock_app do
+ put '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ put('/')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ context "If-None-Match" do
+ it 'returns 412 when If-None-Match is *' do
+ mock_app do
+ put '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_NONE_MATCH' => '*')
+ assert_status 412
+ assert_body ''
+ end
+
+ it 'returns 200 when If-None-Match is * for new resources' do
+ mock_app do
+ put '/' do
+ etag 'foo', :new_resource => true
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_NONE_MATCH' => '*')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 412 when If-None-Match is * for existing resources' do
+ mock_app do
+ put '/' do
+ etag 'foo', :new_resource => false
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_NONE_MATCH' => '*')
+ assert_status 412
+ assert_body ''
+ end
+
+ it 'returns 412 when If-None-Match is the etag' do
+ mock_app do
+ put '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_NONE_MATCH' => '"foo"')
+ assert_status 412
+ assert_body ''
+ end
+
+ it 'returns 412 when If-None-Match includes the etag' do
+ mock_app do
+ put '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_NONE_MATCH' => '"bar", "foo"')
+ assert_status 412
+ assert_body ''
+ end
+
+ it 'returns 200 when If-None-Match does not include the etag' do
+ mock_app do
+ put '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_NONE_MATCH' => '"bar"')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'ignores If-Modified-Since if If-None-Match does not match' do
+ mock_app do
+ put '/' do
+ etag 'foo'
+ last_modified Time.at(0)
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_NONE_MATCH' => '"bar"')
+ assert_status 200
+ assert_body 'ok'
+ end
+ end
+
+ context "If-Match" do
+ it 'returns 200 when If-Match is the etag' do
+ mock_app do
+ put '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_MATCH' => '"foo"')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 200 when If-Match includes the etag' do
+ mock_app do
+ put '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_MATCH' => '"foo", "bar"')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 200 when If-Match is *' do
+ mock_app do
+ put '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_MATCH' => '*')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 412 when If-Match is * for new resources' do
+ mock_app do
+ put '/' do
+ etag 'foo', :new_resource => true
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_MATCH' => '*')
+ assert_status 412
+ assert_body ''
+ end
+
+ it 'returns 200 when If-Match is * for existing resources' do
+ mock_app do
+ put '/' do
+ etag 'foo', :new_resource => false
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_MATCH' => '*')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 412 when If-Match does not include the etag' do
+ mock_app do
+ put '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ put('/', {}, 'HTTP_IF_MATCH' => '"bar"')
+ assert_status 412
+ assert_body ''
+ end
+ end
end
- it 'returns a body when conditional get misses' do
- get '/'
- assert_equal 200, status
- assert_equal 'Boo!', body
- end
+ context "post requests" do
+ it 'returns 200 for normal requests' do
+ mock_app do
+ post '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
- it 'returns a body when posting with no If-None-Match header' do
- post '/'
- assert_equal 200, status
- assert_equal 'Matches!', body
- end
+ post('/')
+ assert_status 200
+ assert_body 'ok'
+ end
- it 'returns a body when conditional post matches' do
- post '/', {}, { 'HTTP_IF_NONE_MATCH' => '"FOO"' }
- assert_equal 200, status
- assert_equal 'Matches!', body
- end
+ context "If-None-Match" do
+ it 'returns 200 when If-None-Match is *' do
+ mock_app do
+ post '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
- it 'halts with 412 when conditional post misses' do
- post '/', {}, { 'HTTP_IF_NONE_MATCH' => '"BAR"' }
- assert_equal 412, status
- assert_equal '', body
- end
+ post('/', {}, 'HTTP_IF_NONE_MATCH' => '*')
+ assert_status 200
+ assert_body 'ok'
+ end
- it 'halts when a conditional GET matches' do
- get '/', {}, { 'HTTP_IF_NONE_MATCH' => '"FOO"' }
- assert_equal 304, status
- assert_equal '', body
- end
+ it 'returns 200 when If-None-Match is * for new resources' do
+ mock_app do
+ post '/' do
+ etag 'foo', :new_resource => true
+ 'ok'
+ end
+ end
- it 'should handle multiple ETag values in If-None-Match header' do
- get '/', {}, { 'HTTP_IF_NONE_MATCH' => '"BAR", *' }
- assert_equal 304, status
- assert_equal '', body
+ post('/', {}, 'HTTP_IF_NONE_MATCH' => '*')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 412 when If-None-Match is * for existing resources' do
+ mock_app do
+ post '/' do
+ etag 'foo', :new_resource => false
+ 'ok'
+ end
+ end
+
+ post('/', {}, 'HTTP_IF_NONE_MATCH' => '*')
+ assert_status 412
+ assert_body ''
+ end
+
+ it 'returns 412 when If-None-Match is the etag' do
+ mock_app do
+ post '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ post('/', {}, 'HTTP_IF_NONE_MATCH' => '"foo"')
+ assert_status 412
+ assert_body ''
+ end
+
+ it 'returns 412 when If-None-Match includes the etag' do
+ mock_app do
+ post '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ post('/', {}, 'HTTP_IF_NONE_MATCH' => '"bar", "foo"')
+ assert_status 412
+ assert_body ''
+ end
+
+ it 'returns 200 when If-None-Match does not include the etag' do
+ mock_app do
+ post '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ post('/', {}, 'HTTP_IF_NONE_MATCH' => '"bar"')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'ignores If-Modified-Since if If-None-Match does not match' do
+ mock_app do
+ post '/' do
+ etag 'foo'
+ last_modified Time.at(0)
+ 'ok'
+ end
+ end
+
+ post('/', {}, 'HTTP_IF_NONE_MATCH' => '"bar"')
+ assert_status 200
+ assert_body 'ok'
+ end
+ end
+
+ context "If-Match" do
+ it 'returns 200 when If-Match is the etag' do
+ mock_app do
+ post '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ post('/', {}, 'HTTP_IF_MATCH' => '"foo"')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 200 when If-Match includes the etag' do
+ mock_app do
+ post '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ post('/', {}, 'HTTP_IF_MATCH' => '"foo", "bar"')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 412 when If-Match is *' do
+ mock_app do
+ post '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ post('/', {}, 'HTTP_IF_MATCH' => '*')
+ assert_status 412
+ assert_body ''
+ end
+
+ it 'returns 412 when If-Match is * for new resources' do
+ mock_app do
+ post '/' do
+ etag 'foo', :new_resource => true
+ 'ok'
+ end
+ end
+
+ post('/', {}, 'HTTP_IF_MATCH' => '*')
+ assert_status 412
+ assert_body ''
+ end
+
+ it 'returns 200 when If-Match is * for existing resources' do
+ mock_app do
+ post '/' do
+ etag 'foo', :new_resource => false
+ 'ok'
+ end
+ end
+
+ post('/', {}, 'HTTP_IF_MATCH' => '*')
+ assert_status 200
+ assert_body 'ok'
+ end
+
+ it 'returns 412 when If-Match does not include the etag' do
+ mock_app do
+ post '/' do
+ etag 'foo'
+ 'ok'
+ end
+ end
+
+ post('/', {}, 'HTTP_IF_MATCH' => '"bar"')
+ assert_status 412
+ assert_body ''
+ end
+ end
end
it 'uses a weak etag with the :weak option' do
- mock_app {
+ mock_app do
get '/' do
etag 'FOO', :weak
"that's weak, dude."
end
- }
+ end
get '/'
assert_equal 'W/"FOO"', response['ETag']
end