Skip to content
Browse files

Fixed entering an infinite loop when the buffer starts with \r\n

  • Loading branch information...
1 parent 4d6833d commit 2e3abc52b3efa99ee53b8c1bd925fafb8581752c @david committed Jul 10, 2008
Showing with 104 additions and 172 deletions.
  1. +55 −66 lib/minibot/server.rb
  2. +49 −106 spec/server_spec.rb
View
121 lib/minibot/server.rb
@@ -13,66 +13,47 @@ def connect
end
def read
- if @messages.first && message_complete?(@messages.first)
- to_message(@messages.shift)
- else
- read_messages
+ if @buffer.empty?
+ @buffer << @socket.recvfrom(512).first
+ read
+ elsif @buffer.start_with? "\r\n"
+ @buffer = @buffer[2 .. -1]
read
- end
- end
-
- def match_code(code, message)
- if (match = reply?(message)) && match[1] == code
- match[1, 2]
else
- nil
+ message, buffer = *@buffer.split(/\r\n/, 2)
+
+ if !message.empty? && buffer
+ @buffer = buffer
+ message
+ else
+ @buffer << @socket.recvfrom(512).first
+ read
+ end
end
end
def write(msg, *replies)
- @socket.print "#{msg}\r\n"
+ print_to_socket msg
unless replies.empty?
- index = 0
- matches, messages = catch :halted do
- loop do
- index, message = next_message(index)
- replies.each do |r|
- case r
- when String
- if match = /:\S+ (#{r}) \S+ :?(.+)/.match(message)
- throw :halted, [[ match ], [ message ]]
- end
- when Array
- matched = []
- messages = []
- r.each do |ri|
- if match = /:\S+ (#{ri}) \S+ :?(.+)/.match(message)
- matched << match
- messages << message
- index, message = next_message(index)
- elsif !matched.empty?
- index -= matched.length
- message = matched.first
- break
- end
- end
-
- throw :halted, [matched, messages] unless matched.empty?
- else
- raise ArgumentError, "Unknown reply argument type: #{r.inspect}", caller
- end
- end
+ buffer = []
+ matches = nil
+ until matches
+ replies.map { |r| Array === r ? r : [ r ] }.each do |r|
+ break if (matches = match(r))
end
- end
- messages.each { |m| delete_message(m) }
- matches.each { |m| yield m[1, 2] }
- end
- end
+ buffer << read unless matches
+ end
- def extract(regexp)
+ unread *buffer
+ if block_given?
+ matches.each { |m| yield *m }
+ else
+ matches
+ end
+ end
end
def disconnect
@@ -81,22 +62,31 @@ def disconnect
private
- def delete_message(m)
- @messages.delete("#{m}\r")
+ def match_code(code, message)
+ if (match = reply?(message)) && match[1] == code
+ match[1, 2]
+ else
+ nil
+ end
end
- def message_complete?(message)
- message[-1] == ?\r
- end
+ def match(codes)
+ catch :halt do
+ matches = []
- def to_message(raw)
- raw.chomp
- end
+ codes.each do |code|
+ msg = read
- def next_message(index)
- read_messages if !@messages[index] || !message_complete?(@messages[index])
+ if match = match_code(code, msg)
+ matches << match
+ else
+ unread *matches
+ throw :halt, nil
+ end
+ end
- [index + 1, to_message(@messages[index])]
+ matches
+ end
end
def reply?(msg)
@@ -106,16 +96,15 @@ def reply?(msg)
def initialize(server, port)
@server = server
@port = port
- @messages = []
+ @buffer = ""
end
- def read_messages
- buffer = @socket.recvfrom(512).first
- messages = buffer.split /\n/
-
- @messages.last << messages.shift if @messages.last && @messages.last[-1] != ?\r
+ def print_to_socket(msg)
+ @socket.print "#{msg}\r\n"
+ end
- @messages += messages
+ def unread(*messages)
+ @buffer = messages.join("\r\n") << @buffer
end
end
end
View
155 spec/server_spec.rb
@@ -15,109 +15,43 @@ def server
it "should fetch messages" do
socket = mock "socket", :null_object => true
- buffer = ("a" * 254) + "\r\n" + ("b" * 254) + "\r\n"
- socket.stub!(:recvfrom).and_return([buffer, nil])
+ buffer1 = ("a" * 154) + "\r\n" + ("b" * 354) + "\r\n"
+ socket.stub!(:recvfrom).and_return([buffer1, nil])
s = server
s.instance_variable_set "@socket", socket
- s.send :read_messages
- s.instance_variable_get("@messages").should == buffer.split(/\n/)
+ s.send(:read).should == ('a' * 154)
+ s.send(:read).should == ('b' * 354)
end
it "should write messages" do
- socket = mock "socket"
- socket.should_receive(:print).with("how now brown cow\r\n")
-
s = server
- s.instance_variable_set "@socket", socket
+ s.should_receive(:print_to_socket).with("how now brown cow")
s.write "how now brown cow"
end
+ # TODO: go back to mocking the socket. Tests are hard to understand this way.
describe "writing with replies" do
- describe "advancing messages" do
- it "should not destroy them" do
- s = server
- s.instance_variable_set "@messages", ["aaa\r", "bbb\r"]
-
- cursor, msg = s.send :next_message, 0
- cursor.should == 1
- msg.should == "aaa"
-
- cursor, msg = s.send :next_message, cursor
- cursor.should == 2
- msg.should == "bbb"
-
- s.instance_variable_get("@messages").should == ["aaa\r", "bbb\r"]
- end
-
- it "should fetch new ones when no more are left" do
- socket = mock "socket", :null_object => true
- socket.stub!(:recvfrom).and_return(["aaa\r\nbbb\r\n", nil])
-
- s = server
- s.instance_variable_set "@socket", socket
- s.instance_variable_set "@messages", []
-
- cursor, msg = s.send :next_message, 0
- msg.should == "aaa"
- end
-
- it "should detect incomplete ones and fetch more before returning" do
- socket = mock "socket", :null_object => true
- socket.stub!(:recvfrom).and_return(["aaa\r\nbbb\r\n", nil])
-
- s = server
- s.instance_variable_set "@socket", socket
- s.instance_variable_set "@messages", ["aa"]
-
- cursor, msg = s.send :next_message, 0
- msg.should == "aaaaa"
- end
- end
-
it "should return the right reply" do
- socket = mock "socket"
- socket.stub! :print
-
s = server
- s.instance_variable_set "@socket", socket
- s.instance_variable_set "@messages", ["aaa\r", ":my.server.com 432 whatever :A message\r"]
+ s.stub!(:print_to_socket)
+ s.should_receive(:read).and_return("aaa", "aaa", ":my.server.com 432 whatever :A message")
tester = mock "tester"
tester.should_receive(:call).with("432", "A message")
s.write("duh", "432") { |code, reply| tester.call code, reply }
end
- it "should return the second wanted reply when it's found first" do
- socket = mock "socket"
- socket.stub! :print
-
- s = server
- s.instance_variable_set "@socket", socket
- s.instance_variable_set "@messages", [
- "aaa\r",
- ":my.server.com 433 whatever :A message2\r",
- ":my.server.com 432 whatever :A message\r"
- ]
-
- tester = mock "tester"
- tester.should_receive(:call).with("433", "A message2")
- s.write("duh", "433") { |code, reply| tester.call code, reply }
- end
-
it "should return all replies when it's passed an array" do
- socket = mock "socket", :null_object => true
- socket.stub! :print
- socket.stub!(:recvfrom).and_return("1\r\n")
-
s = server
- s.instance_variable_set "@socket", socket
- s.instance_variable_set "@messages", [
- "aaa\r",
- ":my.server.com 433 whatever :A message2\r",
- ":my.server.com 432 whatever :A message\r"
- ]
+ s.stub!(:print_to_socket)
+ s.should_receive(:read).and_return(
+ "aaa",
+ "aaa",
+ ":my.server.com 433 whatever :A message2",
+ ":my.server.com 432 whatever :A message"
+ )
tester = mock "tester"
tester.should_receive(:call).with("433", "A message2")
@@ -131,33 +65,30 @@ def server
s = server
s.instance_variable_set "@socket", socket
- s.instance_variable_set "@messages", [
- "aaa\r",
- ":my.server.com 433 whatever :A message2\r",
- "bbb\r",
- ":my.server.com 432 whatever :A message\r"
- ]
+ s.instance_variable_set "@buffer",
+ "aaa\r\n" <<
+ ":my.server.com 433 whatever :A message2\r\n" <<
+ "bbb\r\n" <<
+ ":my.server.com 432 whatever :A message\r\n"
tester = mock "tester"
tester.should_receive(:call).with("433", "A message2")
s.write("duh", ["433", "432"], "433") { |code, reply| tester.call code, reply }
end
- it "should delete replies from message buffer" do
- socket = mock "socket", :null_object => true
- socket.stub! :print
- socket.stub!(:recvfrom).and_return("1\r\n")
-
+ it "should put back unused messages" do
s = server
- s.instance_variable_set "@socket", socket
- s.instance_variable_set "@messages", [
- "aaa\r",
- ":my.server.com 433 whatever :A message2\r",
- ":my.server.com 432 whatever :A message\r"
- ]
-
- s.write("duh", ["433", "432"]) { |code, reply| nil }
- s.instance_variable_get("@messages").should == ["aaa\r", "1\r"]
+ s.stub!(:print_to_socket)
+ s.should_receive(:read).and_return(
+ "aaa",
+ "aaa",
+ ":my.server.com 433 whatever :A message2",
+ ":my.server.com 432 whatever :A message"
+ )
+ s.should_receive(:unread).with(no_args)
+ s.should_receive(:unread).with("aaa")
+
+ s.write("duh", ["433", "432"])
end
end
@@ -171,7 +102,19 @@ def server
s.instance_variable_set "@socket", socket
s.read.should == "a" * 254
- s.instance_variable_get("@messages").should == ["b" * 256]
+ s.instance_variable_get("@buffer").should == ("b" * 256)
+ end
+
+ it "should skip over empty messages" do
+ socket = mock "socket", :null_object => true
+ buffer = "\r\n" << ("a" * 254) << "\r\n" << ("b" * 256) << "\r\n"
+ socket.should_receive(:recvfrom).and_return([buffer, nil])
+
+ s = server
+ s.instance_variable_set "@socket", socket
+
+ s.read.should == "a" * 254
+ s.instance_variable_get("@buffer").should == ("b" * 256) << "\r\n"
end
it "should join incomplete messages" do
@@ -181,17 +124,17 @@ def server
s = server
s.instance_variable_set "@socket", socket
- s.instance_variable_set "@messages", [buffer]
+ s.instance_variable_set "@buffer", buffer
s.read.should == "b" * 260
end
- it "should return a single complete messages" do
+ it "should return a single complete message" do
s = server
- s.instance_variable_set "@messages", [("b" * 256) + "\r"]
+ s.instance_variable_set "@buffer", ("b" * 256) + "\r\n"
s.read.should == "b" * 256
- s.instance_variable_get("@messages").should == []
+ s.instance_variable_get("@buffer").should == ""
end
end

0 comments on commit 2e3abc5

Please sign in to comment.
Something went wrong with that request. Please try again.