Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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

  • Loading branch information...
commit 2e3abc52b3efa99ee53b8c1bd925fafb8581752c 1 parent 4d6833d
@david authored
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
Please sign in to comment.
Something went wrong with that request. Please try again.