Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persistent clients don't support changing headers #725

Open
byteit101 opened this issue Aug 25, 2022 · 3 comments
Open

Persistent clients don't support changing headers #725

byteit101 opened this issue Aug 25, 2022 · 3 comments
Assignees

Comments

@byteit101
Copy link

byteit101 commented Aug 25, 2022

Using Wireshark, I am able to confirm that this works as expected, using only one TCP connection:

 HTTP.persistent("http://api.icndb.com") {|http| 
  http.get("/jokes/random").to_s;
  http.get("/jokes/random").to_s
}

However, this doesn't, closing the TCP connection and creating a new TCP and HTTP connection for each request:

 HTTP.persistent("http://api.icndb.com") {|http| 
  http.headers("x-test" => "foo").get("/jokes/random").to_s;
  http.get("/jokes/random").to_s
}

Is there some ordering to do differently for persistent clients?

@byteit101 byteit101 changed the title HTTPS doesn't support persistent clients Persistent clients don't support changing headers Aug 25, 2022
@byteit101
Copy link
Author

For ease of debugging this issue without Wireshark, I've made a statefull HTTP server for this issue:

require 'socket'

server = TCPServer.new 7182
def run(client)
	class << client
		def readish(n=1)
			begin
			  result = self.read_nonblock(n)
			rescue IO::WaitReadable
			  r,w,e = IO.select([self], [],[],0.2)
			  if r && r.length > 0
				  retry
			  else
				  nil
			  end
			rescue IOError
				nil
			end
		end
	end
	puts "accepted"
	# first url
	s = ""
	until s.include? "\r\n\r\n"
		tmp = client.readish(1)
		if tmp.nil?
			puts "Closed in the middle 1, errror!"
			client.close
			return
		end
		s << tmp
	end
	if s.match(/^(GET|get) \/start/) == nil
		puts "wrong start!"
		client.puts "HTTP/1.1 400 WrongStart\r\n\r\nBAD"
		client.close
		return
	end
	puts "Good start"
	client.puts "HTTP/1.1 200 Ok\r\nKeep-Alive: timeout=5, max=100\r\nConnection: Keep-Alive\r\ncontent-Length: 1\r\n\r\nG"
	
	# second url
	puts "On to next"
	s = ""
	until s.include? "\r\n\r\n"
		tmp = client.readish(1)
		if tmp.nil?
			puts "Closed in the middle3, errror!"
			client.puts "HTTP/1.1 467 YouClosedIt\r\n\r\nBAD"
			client.close
			return
		end
		s << tmp
	end
	if s.match(/^(GET|get) \/finish/) == nil
		client.puts "HTTP/1.1 400 WrongFinish\r\n\r\nBAD"
		client.close
		return
	end
	puts "Good End!"
	client.puts "HTTP/1.1 200 OK\r\nConnection: Close\r\ncontent-Length: 3\r\n\r\n:-)"
	client.close	
end

loop do
	run(server.accept)
end

Correct Persistence (expected)

2.6.2 :046 > HTTP.persistent("http://localhost:7182") {|http| http.get("/start").to_s; puts http.get("/finish").to_s }
:-)
 => nil 

Incorrect Persistence (actual)

2.6.2 :047 > HTTP.persistent("http://localhost:7182") {|http| http.headers("x-test" => "foo").get("/start").to_s; puts http.get("/finish").to_s }
BAD
 => nil 

@tarcieri
Copy link
Member

While this is annoying and weird behavior, I think you can work around it by passing headers as an option to get

@byteit101
Copy link
Author

Ah yes, that appears to have worked. Should put the workaround on the wiki page until it's fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants