Skip to content
Sign up
This repository

/ruby

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

set content-length to zero on empty post requests #201

Closed
wants to merge 2 commits into from
+12 −0

Labels

Milestone

No milestone

Assignee

No one assigned

4 participants

Gregory Ostermayr Eric Hodel Zachary Scott NARUSE, Yui
Gregory Ostermayr
gregors commented

It is bad form to not set the content-length header on empty post requests. This will most likely end with a 411 response from the server. Sometimes post requests will be empty e.g. the initial request during a challenge-response authentication scenario.

I've also included a test. It checks to see that webrick does not return the 411 response code. This is a resubmitted pull request. I initially dev'd on the 1.9.3 branch pull request #200. Sorry about that - still trying to figure out the workflow around here.

Eric Hodel drbrain commented on the diff
View full changes
lib/net/http/generic_request.rb
@@ -75,6 +75,9 @@ def body_stream=(input)
def set_body_internal(str) #:nodoc: internal use only
raise ArgumentError, "both of body argument and HTTPRequest#body set" if str and (@body or @body_stream)
self.body = str if str
+ if @body.nil? && @body_stream.nil? && @body_data.nil?
Eric Hodel Collaborator
drbrain added a note

This also sets the content-length for GET requests (and other requests that don't have a body).

I think this should also check request_body_permitted?.

See this for an example:

require 'net/http'

h = Net::HTTP.new 'localhost', 8000
h.set_debug_output $stderr

h.get '/'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Eric Hodel
Collaborator
drbrain commented

When you're done you should create a ticket on bugs.ruby-lang.org that references this pull request. You don't need to submit the patch.

Zachary Scott
Collaborator
zzak commented

@gregors ping! Are you planning on writing this patch?

Gregory Ostermayr
gregors commented

Sorry holidays got in the way. Yes I am but I'm having trouble writing a proper test. After chatting with drbrain I thought that instead of looking at the http response code, I would check the headers in the request, but I can't seem to figure out how to do that. Any pointers anyone could give me?

Gregory Ostermayr gregors add check for request body permitted … 
We don't want get requests to have a content-length header
 cb4df80
Zachary Scott
Collaborator
zzak commented

@gregors You're likely to receive more feedback if you open a ticket on bugs.ruby-lang.org, since that is preferred way and most committers don't check github.

NARUSE, Yui nurse closed this pull request from a commit
NARUSE, Yui nurse * lib/net/http/generic_request.rb (Net::HTTPGenericRequest): … 
  set content-length to zero on empty post requests
  by Gregory Ostermayr <gregory.ostermayr@gmail.com>
  #201 fix GH-201

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@38578 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
 02253a7
Aaron Patterson tenderlove referenced this pull request from a commit in tenderlove/ruby
NARUSE, Yui nurse * lib/net/http/generic_request.rb (Net::HTTPGenericRequest): … 
  set content-length to zero on empty post requests
  by Gregory Ostermayr <gregory.ostermayr@gmail.com>
  ruby#201 fix GH-201

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@38578 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
 66a6a27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 22, 2012
  1. Gregory Ostermayr

    set content-length to zero on empty post requests

    gregors authored
    d3a9bee
Commits on Nov 28, 2012
  1. Gregory Ostermayr

    add check for request body permitted …

    gregors authored 
    We don't want get requests to have a content-length header
    cb4df80
This page is out of date. Refresh to see the latest.
Unified Split
Showing with 12 additions and 0 deletions.
  1. +3 −0  lib/net/http/generic_request.rb
  2. +9 −0 test/net/http/test_http.rb
3  lib/net/http/generic_request.rb
View
@@ -75,6 +75,9 @@ def body_stream=(input)
def set_body_internal(str) #:nodoc: internal use only
raise ArgumentError, "both of body argument and HTTPRequest#body set" if str and (@body or @body_stream)
self.body = str if str
+ if @body.nil? && @body_stream.nil? && @body_data.nil? && request_body_permitted?
Eric Hodel Collaborator
drbrain added a note

This also sets the content-length for GET requests (and other requests that don't have a body).

I think this should also check request_body_permitted?.

See this for an example:

require 'net/http'

h = Net::HTTP.new 'localhost', 8000
h.set_debug_output $stderr

h.get '/'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ self.body = ''
+ end
end
#
9 test/net/http/test_http.rb
View
@@ -311,6 +311,7 @@ def test_post
start {|http|
_test_post__base http
_test_post__file http
+ _test_post__no_data http
}
end
@@ -333,6 +334,14 @@ def _test_post__file(http)
assert_equal data, f.string
end
+ def _test_post__no_data(http)
+ unless self.is_a?(TestNetHTTP_v1_2_chunked)
+ data = nil
+ res = http.post('/', data)
+ assert_not_equal '411', res.code
+ end
+ end
+
def test_s_post_form
url = "http://#{config('host')}:#{config('port')}/"
res = Net::HTTP.post_form(
Something went wrong with that request. Please try again.