From 80df1588eeed78e6b3b0e7ef92b0b32cdaa3cd7b Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sun, 19 Jul 2026 18:33:11 +1200 Subject: [PATCH 1/3] Add request retry preparation --- lib/protocol/http/request.rb | 32 +++++++++++++------------- test/protocol/http/request.rb | 42 +++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/lib/protocol/http/request.rb b/lib/protocol/http/request.rb index c9593cf..e4507a6 100644 --- a/lib/protocol/http/request.rb +++ b/lib/protocol/http/request.rb @@ -132,25 +132,27 @@ def self.[](method, path = nil, _headers = nil, _body = nil, scheme: nil, author self.new(scheme, authority, method, path, nil, headers, body, protocol, interim_response) end - # Whether the request can be replayed without side-effects. + # Whether the request method is idempotent according to HTTP semantics. def idempotent? - # QUERY requests are idempotent, even if they have a body: - if @method == Methods::QUERY - return true - end - - # POST requests are not idempotent, even if they have no body: - if @method == Methods::POST - return false + case @method + when Methods::POST, Methods::PATCH, Methods::CONNECT + false + else + true end + end + + # Prepare the request body to be sent again, if it is safe to retry. + # @returns [Boolean] Whether the request was prepared for retry. + def retry! + return false unless self.idempotent? + return true unless body = @body + return true if body.empty? && !body.rewindable? + return false unless body.rewindable? - # All other requests are idempotent if they have no body: - if @body.nil? || @body.empty? - return true - end + return false unless body.rewind - # Otherwise, we don't know if the request is idempotent or not, so we assume it is not: - return false + return true end # Convert the request to a hash, suitable for serialization. diff --git a/test/protocol/http/request.rb b/test/protocol/http/request.rb index 9958c6b..4510efc 100644 --- a/test/protocol/http/request.rb +++ b/test/protocol/http/request.rb @@ -147,11 +147,53 @@ with "PUT request with a body" do let(:request) {subject["PUT", "/resource", body: "content"]} + it "should be idempotent" do + expect(request).to be(:idempotent?) + end + end + + with "PATCH request without a body" do + let(:request) {subject["PATCH", "/resource"]} + it "should not be idempotent" do expect(request).not.to be(:idempotent?) end end + with "#retry!" do + it "allows idempotent requests without a body" do + expect(request.retry!).to be == true + end + + with "idempotent request with a rewindable body" do + let(:request) {subject["PUT", "/resource", body: "content"]} + + it "rewinds the body" do + expect(request.body.read).to be == "content" + + expect(request.retry!).to be == true + expect(request.body.read).to be == "content" + end + end + + with "idempotent request with a non-rewindable body" do + let(:body) {Protocol::HTTP::Body::Readable.new} + let(:request) {subject.new(nil, nil, "PUT", "/resource", nil, headers, body)} + + it "does not allow retry" do + expect(request.retry!).to be == false + end + end + + with "non-idempotent request" do + let(:request) {subject["POST", "/submit"]} + + it "does not allow retry" do + expect(request.retry!).to be == false + end + end + end + it "should have a string representation" do expect(request.to_s).to be == "http://localhost: GET /index.html HTTP/1.0" end From 077b8d5a46199524ab68d55f036c72742250eae9 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sun, 19 Jul 2026 18:33:56 +1200 Subject: [PATCH 2/3] Clarify retry preparation logic --- lib/protocol/http/request.rb | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/protocol/http/request.rb b/lib/protocol/http/request.rb index e4507a6..0d59132 100644 --- a/lib/protocol/http/request.rb +++ b/lib/protocol/http/request.rb @@ -145,12 +145,27 @@ def idempotent? # Prepare the request body to be sent again, if it is safe to retry. # @returns [Boolean] Whether the request was prepared for retry. def retry! - return false unless self.idempotent? - return true unless body = @body - return true if body.empty? && !body.rewindable? - return false unless body.rewindable? + # Only idempotent requests can be retried safely. + if !self.idempotent? + return false + end - return false unless body.rewind + # Requests without a body can be sent again immediately. + if body = @body + # Empty, non-rewindable bodies have nothing left to send. + if body.empty? && !body.rewindable? + return true + end + + # Non-empty bodies must be rewindable so the same data can be sent again. + if !body.rewindable? + return false + end + + if !body.rewind + return false + end + end return true end From e0143dbabcade8d397d13524a64e8cbd36a985e6 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sun, 19 Jul 2026 18:51:35 +1200 Subject: [PATCH 3/3] Preserve idempotent compatibility --- lib/protocol/http/request.rb | 27 +++++++++++++++++++-------- test/protocol/http/request.rb | 18 +++++++++++++----- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/lib/protocol/http/request.rb b/lib/protocol/http/request.rb index 0d59132..325960b 100644 --- a/lib/protocol/http/request.rb +++ b/lib/protocol/http/request.rb @@ -132,21 +132,32 @@ def self.[](method, path = nil, _headers = nil, _body = nil, scheme: nil, author self.new(scheme, authority, method, path, nil, headers, body, protocol, interim_response) end - # Whether the request method is idempotent according to HTTP semantics. + # Whether the request can be replayed without side-effects. def idempotent? - case @method - when Methods::POST, Methods::PATCH, Methods::CONNECT - false - else - true + # QUERY requests are idempotent, even if they have a body: + if @method == Methods::QUERY + return true + end + + # POST requests are not idempotent, even if they have no body: + if @method == Methods::POST + return false end + + # All other requests are idempotent if they have no body: + if @body.nil? || @body.empty? + return true + end + + # Otherwise, we don't know if the request is idempotent or not, so we assume it is not: + return false end # Prepare the request body to be sent again, if it is safe to retry. # @returns [Boolean] Whether the request was prepared for retry. def retry! - # Only idempotent requests can be retried safely. - if !self.idempotent? + # Only idempotent request methods can be retried safely. + if @method == Methods::POST || @method == Methods::PATCH || @method == Methods::CONNECT return false end diff --git a/test/protocol/http/request.rb b/test/protocol/http/request.rb index 4510efc..de31388 100644 --- a/test/protocol/http/request.rb +++ b/test/protocol/http/request.rb @@ -147,16 +147,16 @@ with "PUT request with a body" do let(:request) {subject["PUT", "/resource", body: "content"]} - it "should be idempotent" do - expect(request).to be(:idempotent?) + it "should not be idempotent" do + expect(request).not.to be(:idempotent?) end end with "PATCH request without a body" do let(:request) {subject["PATCH", "/resource"]} - it "should not be idempotent" do - expect(request).not.to be(:idempotent?) + it "should be idempotent" do + expect(request).to be(:idempotent?) end end @@ -168,7 +168,7 @@ with "idempotent request with a rewindable body" do let(:request) {subject["PUT", "/resource", body: "content"]} - it "rewinds the body" do + it "allows retry and rewinds the body" do expect(request.body.read).to be == "content" expect(request.retry!).to be == true @@ -192,6 +192,14 @@ expect(request.retry!).to be == false end end + + with "PATCH request" do + let(:request) {subject["PATCH", "/resource"]} + + it "does not allow retry" do + expect(request.retry!).to be == false + end + end end it "should have a string representation" do