Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Instead of immediately deleting files when a DELE command is received,

they should be queued for deletion and removed on QUIT.

The reason for this is that if a client retrieves a list of messages,
it doesn't expect the message indices to change. Deleting a file
straight away means a request for a file using RETR with a higher index
could result in an out of bounds error.
  • Loading branch information...
commit 9bc36bdcaa0087e0d6c57e70fbb6d698e3b271de 1 parent 49e29d2
@lukeredpath lukeredpath authored
Showing with 63 additions and 8 deletions.
  1. +15 −2 lib/mailshovel.rb
  2. +48 −6 test/mailshovel_test.rb
View
17 lib/mailshovel.rb
@@ -13,7 +13,8 @@ class Mailshovel
def initialize(service, msgdir)
@service = service
@msgdir = msgdir
-
+ @deletion_queue = []
+
accept(service)
end
@@ -35,6 +36,7 @@ def get_line
begin
serve( session )
rescue Exception => e
+ warn e.message
puts "Erk! #{e.message}"
end
end
@@ -75,9 +77,10 @@ def serve( connection )
senddata connection, "\r\n.\r\n"
when "DELE"
senddata connection, "+OK\r\n"
- File.unlink(messages[incoming.split(" ")[1].to_i-1])
+ schedule_for_delete(messages[incoming.split(" ")[1].to_i-1])
when "QUIT"
senddata connection, "+OK Closing connection.\r\n"
+ delete_pending_mail
connection.close
else
senddata connection, "-ERR I don't understand '#{incoming}'.\r\n"
@@ -90,4 +93,14 @@ def senddata(connection,data)
puts "> #{data}"
connection.puts data
end
+
+ private
+
+ def schedule_for_delete(message)
+ @deletion_queue << message
+ end
+
+ def delete_pending_mail
+ @deletion_queue.each { |message| FileUtils.rm(message) }
+ end
end
View
54 test/mailshovel_test.rb
@@ -22,9 +22,9 @@ def puts(string)
end
def say(string)
+ @input.puts(string + "\r\n")
@input.rewind
- @input.puts(string)
- @input.rewind
+ sleep(0.1)
end
def gets
@@ -46,6 +46,7 @@ class MailshovelTest < Test::Unit::TestCase
end
teardown do
+ FakeFS::FileSystem.clear
$stdout = STDOUT
@runner.kill if @runner
end
@@ -61,7 +62,7 @@ class MailshovelTest < Test::Unit::TestCase
end
should "respond to LIST command with a list of mail" do
- 3.times { |x| FileUtils.touch("/tmp/mailshovel/message#{x}.txt") }
+ 3.times { |x| FileUtils.touch("/tmp/mailshovel/message#{x + 1}.txt") }
with_mailshovel do
say("LIST")
@@ -71,15 +72,56 @@ class MailshovelTest < Test::Unit::TestCase
end
should "respond to STAT command with the number of messages" do
- 5.times { |x| FileUtils.touch("/tmp/mailshovel/message#{x}.txt") }
+ 3.times { |x| FileUtils.touch("/tmp/mailshovel/message#{x + 1}.txt") }
with_mailshovel do
say("STAT")
end
- assert_received_response(/OK 5 5/)
+ assert_received_response(/OK 3 3/)
end
+ context "when sending a DELE command" do
+ setup do
+ 2.times do |x|
+ File.open("/tmp/mailshovel/message#{x + 1}.txt", 'w+') do |io|
+ io.write("MESSAGE #{x + 1}")
+ end
+ end
+ end
+
+ teardown do
+ FakeFS::FileSystem.clear
+ end
+
+ should "not immediately delete messages" do
+ with_mailshovel do
+ say("DELE 2")
+ end
+
+ assert File.exist?("/tmp/mailshovel/message2.txt")
+ end
+
+ should "still allow the message to be retrieved using its original index" do
+ with_mailshovel do
+ say("DELE 1")
+ say("RETR 1")
+ end
+
+ assert_received_response(/MESSAGE 1/)
+ end
+
+ should "delete messages on QUIT" do
+ with_mailshovel do
+ say("DELE 1")
+ say("DELE 2")
+ say("QUIT")
+ end
+
+ assert !File.exist?("/tmp/mailshovel/message1.txt")
+ assert !File.exist?("/tmp/mailshovel/message2.txt")
+ end
+ end
end
private
@@ -90,10 +132,10 @@ def with_mailshovel(&block)
end
@connection.instance_eval(&block)
+ sleep 1
end
def assert_received_response(regexp)
- sleep 0.1
@connection.output.rewind
assert_match regexp, @connection.output.read
end
Please sign in to comment.
Something went wrong with that request. Please try again.