bytemark/flexnbd-c
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

serve: Make some error conditions non-fatal, test them.

Browse Source 
We don't want flexnbd serve to fall over and die if the client sends an invalid request.
This commit is contained in:
nick
2013-02-15 16:51:28 +00:00
parent 63f7e3e8d4
commit 9b67d30608
7 changed files with 160 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
src/client.c
View File

@@ -1,7 +1,8 @@
#include "client.h"
#include "serve.h"
#include "util.h"
#include "ioutil.h"
#include "sockutil.h"
#include "util.h"
#include "bitset.h"
#include "nbdtypes.h"
#include "self_pipe.h"
@@ -141,7 +142,7 @@ void write_not_zeroes(struct client* client, uint64_t from, int len)
* hand-optimized something specific.
*/
int all_zeros = (zerobuffer[0] == 0) &&
int all_zeros = (zerobuffer[0] == 0) &&
(0 == memcmp( zerobuffer, zerobuffer+1, blockrun-1 ));
if ( !all_zeros ) {
@@ -192,7 +193,7 @@ int client_read_request( struct client * client , struct nbd_request *out_reques
FD_ZERO(&fds);
FD_SET(client->socket, &fds);
self_pipe_fd_set( client->stop_signal, &fds );
fd_count = select(FD_SETSIZE, &fds, NULL, NULL, ptv);
fd_count = sock_try_select(FD_SETSIZE, &fds, NULL, NULL, ptv);
if ( fd_count == 0 ) {
/* This "can't ever happen" */
if ( NULL == ptv ) { fatal( "No FDs selected, and no timeout!" ); }
@@ -347,30 +348,43 @@ void client_flush( struct client * client, size_t len )
int client_request_needs_reply( struct client * client,
struct nbd_request request )
{
debug("request type %d", request.type);
/* The client is stupid, but don't take down the whole server as a result.
* We send a reply before disconnecting so that at least some indication of
* the problem is visible, and so proxies don't retry the same (bad) request
* forever.
*/
if (request.magic != REQUEST_MAGIC) {
fatal("Bad magic %08x", request.magic);
warn("Bad magic %08x from client", request.magic);
client_write_reply( client, &request, EBADMSG );
client->disconnect = 1; // no need to flush
return 0;
}
debug(
"request type=%"PRIu32", from=%"PRIu64", len=%"PRIu32,
request.type, request.from, request.len
);
/* check it's not out of range */
if ( request.from+request.len > client->serve->size) {
warn("write request %"PRIu64"+%"PRIu32" out of range",
request.from, request.len
);
if ( request.type == REQUEST_WRITE ) {
client_flush( client, request.len );
}
client_write_reply( client, &request, EPERM ); /* TODO: Change to ERANGE ? */
client->disconnect = 0;
return 0;
}
switch (request.type)
{
case REQUEST_READ:
break;
case REQUEST_WRITE:
/* check it's not out of range */
if ( request.from+request.len > client->serve->size) {
warn("write request %d+%d out of range",
request.from,
request.len
);
client_write_reply( client, &request, 1 );
client_flush( client, request.len );
client->disconnect = 0;
return 0;
}
break;
case REQUEST_DISCONNECT:
debug("request disconnect");
client->disconnect = 1;
@@ -432,7 +446,7 @@ void client_reply_to_write( struct client* client, struct nbd_request request )
* never consult the allocation_map until the builder thread has
* finished.
*/
bitset_set_range(client->serve->allocation_map,
bitset_set_range(client->serve->allocation_map,
request.from, request.len);
}

6
tests/acceptance/fakes/source/connect_from_banned_ip.rb
View File

@@ -13,10 +13,8 @@ addr, port = *ARGV
client = FakeSource.new( addr, port, "Timed out connecting", "127.0.0.6" )
sleep( 0.25 )
client.ensure_disconnected
rsp = client.disconnected? ? 0 : 1
client.close
exit(0)
exit(rsp)

4
tests/acceptance/file_writer.rb
View File

@@ -8,6 +8,10 @@ class FileWriter
@pattern = ""
end
def size
@blocksize * @pattern.split("").size
end
# We write in fixed block sizes, given by "blocksize"
# _ means skip a block
# 0 means write a block full of zeroes

5
tests/acceptance/flexnbd/constants.rb
View File

@@ -32,6 +32,9 @@ module FlexNBD
end
read_constants()
REQUEST_MAGIC = "\x25\x60\x95\x13" unless defined?(REQUEST_MAGIC)
REPLY_MAGIC = "\x67\x44\x66\x98" unless defined?(REPLY_MAGIC)
end # module FlexNBD

9
tests/acceptance/flexnbd/fake_dest.rb
View File

@@ -56,8 +56,6 @@ module FlexNBD
}
end
REPLY_MAGIC="\x67\x44\x66\x98"
def write_error( handle )
write_reply( handle, 1 )
end
@@ -76,7 +74,7 @@ module FlexNBD
if opts[:magic] == :wrong
write_rand( @sock, 4 )
else
@sock.write( REPLY_MAGIC )
@sock.write( ::FlexNBD::REPLY_MAGIC )
end
@sock.write( [err].pack("N") )
@@ -93,6 +91,10 @@ module FlexNBD
@sock.read( len )
end
def write_data( len )
@sock.write( len )
end
def self.parse_be64(str)
raise "String is the wrong length: 8 bytes expected (#{str.length} received)" unless
@@ -161,3 +163,4 @@ module FlexNBD
end # module FakeDest
end # module FlexNBD

35
tests/acceptance/flexnbd/fake_source.rb
View File

@@ -30,7 +30,7 @@ module FlexNBD
def read_hello()
timing_out( FlexNBD::MS_HELLO_TIME_SECS,
timing_out( ::FlexNBD::MS_HELLO_TIME_SECS,
"Timed out waiting for hello." ) do
fail "No hello." unless (hello = @sock.read( 152 )) &&
hello.length==152
@@ -47,15 +47,14 @@ module FlexNBD
end
def send_request( type, handle="myhandle", from=0, len=0 )
def send_request( type, handle="myhandle", from=0, len=0, magic=REQUEST_MAGIC )
fail "Bad handle" unless handle.length == 8
@sock.write( "\x25\x60\x95\x13" )
@sock.write( magic )
@sock.write( [type].pack( 'N' ) )
@sock.write( handle )
@sock.write( "\x0"*4 )
@sock.write( [from].pack( 'N' ) )
@sock.write( [len ].pack( 'N' ) )
@sock.write( [n64( from )].pack( 'q' ) )
@sock.write( [len].pack( 'N' ) )
end
@@ -122,10 +121,10 @@ module FlexNBD
end
def ensure_disconnected
Timeout.timeout( 2 ) do
@sock.read(1)
end
def disconnected?
result = nil
Timeout.timeout( 2 ) { result = ( @sock.read(1) == nil ) }
result
end
@@ -140,6 +139,22 @@ module FlexNBD
end
end
private
# take a 64-bit number, turn it upside down (due to :
# http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/11920
# )
def n64(b)
((b & 0xff00000000000000) >> 56) |
((b & 0x00ff000000000000) >> 40) |
((b & 0x0000ff0000000000) >> 24) |
((b & 0x000000ff00000000) >> 8) |
((b & 0x00000000ff000000) << 8) |
((b & 0x0000000000ff0000) << 24) |
((b & 0x000000000000ff00) << 40) |
((b & 0x00000000000000ff) << 56)
end
end # class FakeSource
end # module FlexNBD

86
tests/acceptance/test_serve_mode.rb Normal file
View File

@@ -0,0 +1,86 @@
require 'test/unit'
require 'environment'
require 'flexnbd/fake_source'
class TestServeMode < Test::Unit::TestCase
def setup
super
@env = Environment.new
@env.writefile1( "0" )
@env.serve1
end
def teardown
@env.cleanup
super
end
def connect_to_server
client = FlexNBD::FakeSource.new(@env.ip, @env.port1, "Connecting to server failed")
begin
result = client.read_hello
assert_equal "NBDMAGIC", result[:magic]
assert_equal @env.file1.size, result[:size]
yield client
ensure
client.close rescue nil
end
end
def test_bad_request_magic_receives_error_response
connect_to_server do |client|
# replace REQUEST_MAGIC with all 0s to make it look bad
client.send_request( 0, "myhandle", 0, 0, "\x00\x00\x00\x00" )
rsp = client.read_response
assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic]
assert_equal "myhandle", rsp[:handle]
assert rsp[:error] != 0, "Server sent success reply back: #{rsp[:error]}"
# The client should be disconnected now
assert client.disconnected?, "Server not disconnected"
end
end
def test_read_request_out_of_bounds_receives_error_response
connect_to_server do |client|
client.write_read_request( @env.file1.size, 4096 )
rsp = client.read_response
assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic]
assert_equal "myhandle", rsp[:handle]
assert rsp[:error] != 0, "Server sent success reply back: #{rsp[:error]}"
# Ensure we're not disconnected by sending a request. We don't care about
# whether the reply is good or not, here.
client.write_read_request( 0, 4096 )
rsp = client.read_response
assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic]
end
end
def test_write_request_out_of_bounds_receives_error_response
connect_to_server do |client|
client.write( @env.file1.size, "\x00" * 4096 )
rsp = client.read_response
assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic]
assert_equal "myhandle", rsp[:handle]
assert rsp[:error] != 0, "Server sent success reply back: #{rsp[:error]}"
# Ensure we're not disconnected by sending a request. We don't care about
# whether the reply is good or not, here.
client.write( 0, "\x00" * @env.file1.size )
rsp = client.read_response
assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic]
end
end
end