Skip to content

Loading…

Bugfix for 166 #167

Merged
merged 3 commits into from

2 participants

@seancribbs
Webmachine member

As reported in #166, if a resource did not define an ETag, but the client sent an If-None-Match header, a 500 would result. This instead replies with 200.

@lgierth lgierth added the bug label
@lgierth

Looks good :+1:

@seancribbs
Webmachine member

@lgierth Actually, looking back at it, I think this is wrong. If the resource generates no ETag, then if-none-match should always succeed. I will rewrite.

@lgierth

Don't know, is there any specification about what should happen if the client sends If-None-Match to a resource that doesn't respond with an ETag? Responding with a client error (4xx), in order to make the client retry without If-None-Match, seems okay to me.

@seancribbs
Webmachine member

@lgierth Rewrote branch with 200 instead of 412.

@lgierth

200 is definitely more in the spirit of being liberal in what you expect from clients :+1:

@seancribbs seancribbs merged commit bd7418e into master

1 check passed

Details continuous-integration/travis-ci The Travis CI build passed
@seancribbs seancribbs deleted the bugfix-166 branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Showing with 26 additions and 1 deletion.
  1. +6 −1 lib/webmachine/decision/flow.rb
  2. +20 −0 spec/webmachine/decision/flow_spec.rb
View
7 lib/webmachine/decision/flow.rb
@@ -326,7 +326,12 @@ def k7
# Etag in if-none-match?
def k13
request_etags = request.if_none_match.split(/\s*,\s*/).map {|etag| ETag.new(etag) }
- request_etags.include?(ETag.new(resource.generate_etag)) ? :j18 : :l13
+ resource_etag = resource.generate_etag
+ if resource_etag && request_etags.include?(ETag.new(resource_etag))
+ :j18
+ else
+ :l13
+ end
end
# Moved temporarily?
View
20 spec/webmachine/decision/flow_spec.rb
@@ -575,6 +575,26 @@ def allowed_methods; %w{GET HEAD POST}; end
end
after { subject.run; expect(response.code).to eq 412 }
end
+
+ context "when the resource does not define an ETag" do
+ let(:resource) do
+ resource_with do
+ def generate_etag; nil; end
+ end
+ end
+
+ it "should reply with 200 when If-None-Match is missing" do
+ headers.delete 'If-None-Match'
+ subject.run
+ expect(response.code).to eq 200
+ end
+
+ it "should reply with 200 when If-None-Match is present" do
+ headers['If-None-Match'] = '"etag"'
+ subject.run
+ expect(response.code).to eq 200
+ end
+ end
end
describe "#l13, #l14, #l15, #l17 (If-Modified-Since match)" do
Something went wrong with that request. Please try again.