shlist

share and manage lists between multiple people
Log | Files | Refs

commit 4b3b327ff2de93510bc9de50e740ec9f2201f46c
parent 5776d05bb18df8e85178433475e932e602a418c3
Author: kyle <kyle@0x30.net>
Date:   Sun, 17 Jan 2016 22:33:48 -0700

sl: change fundamental format of message payload

- change message payloads to JSON encoded strings
  - this has lots of nice properties like utf8, lists, dictionaries
- the old hand rolled splitting on '\0' and '\n' is barbaric
  - it's argument order dependent, breaks easily
  - that issue on android where splitting on '\0\0' collapsed into 1 field
- responses always contain a 'status' key in the json root
  - if set to 'err', 'reason' will be a key whose value is a reason why the
    request failed
- convert tests over to send/receive json also
  - makes things simpler here too because we can use the deserialized data
    structures directly
- also, this proves easy for objective C to interface with
  - preliminary code for ios client written and works

Diffstat:
Mserver/sl | 473+++++++++++++++++++++++++++++++++++++++++++------------------------------------
Mserver/tests/client.pm | 197+++++++++++++++++++++++++++++++------------------------------------------------
Mserver/tests/friend_delete/test.pl | 2+-
Mserver/tests/get_other_lists_filters_my_lists/test.pl | 2+-
Mserver/tests/header/server.log.good | 2+-
Mserver/tests/invalid_deviceid/server.log.good | 35+++++++++++++++++++++--------------
Mserver/tests/invalid_deviceid/test.pl | 11++++++-----
Mserver/tests/leave_list_your_not_in/test.pl | 2+-
Mserver/tests/list_add/test.pl | 2+-
Mserver/tests/list_join/test.pl | 2+-
Mserver/tests/list_join_unit/test.pl | 7+++----
Mserver/tests/list_leave/test.pl | 3+--
Mserver/tests/list_reference_counting/test.pl | 2+-
Mserver/tests/list_update/test.pl | 2+-
Mserver/tests/lists_get/test.pl | 31+++++++++++--------------------
Mserver/tests/lists_get_other/test.pl | 13+++++--------
Mserver/tests/multiple_friends_same_other_list/server.log.good | 1-
Mserver/tests/multiple_friends_same_other_list/test.pl | 4++--
Mserver/tests/update_list_youre_not_in/test.pl | 2+-
Mserver/tests/zero_payload/server.log.good | 20++++++++++----------
Mserver/tests/zero_payload/test.pl | 19++++---------------
21 files changed, 407 insertions(+), 425 deletions(-)

diff --git a/server/sl b/server/sl @@ -3,11 +3,12 @@ use warnings; use strict; use BSD::arc4random qw(arc4random_bytes arc4random_stir); +use Digest::SHA qw(sha256_base64); use DBI; use File::Temp; -use Digest::SHA qw(sha256_base64); use Getopt::Std; use IO::Socket::SSL; +use JSON::PP; use Scalar::Util qw(looks_like_number); require "msgs.pl"; @@ -74,11 +75,11 @@ while (my $client_socket = $server_socket->accept()) { $db->prepare_stmt_handles(); while (1) { - my ($ver, $msg_type, $msg) = recv_msg($client_socket); + my ($ver, $msg_type, $request) = recv_msg($client_socket); $log->set_msg($msg_str[$msg_type]); $db->{dbh}->begin_work; - my $reply = $msg_func[$msg_type]->($db, $msg); + my $response = $msg_func[$msg_type]->($db, $request); $db->{dbh}->commit; if ($@) { @@ -86,13 +87,13 @@ while (my $client_socket = $server_socket->accept()) { # but do it in an eval{} as it may also fail eval { $db->{dbh}->rollback }; - $log->print("discarding reply '$reply'\n"); $log->print("db transaction aborted: $@\n"); - $reply = "err\0database transaction aborted"; + $response->{status} = 'err'; + $response->{reason} = "database transaction aborted"; } $log->set_msg(''); - send_msg($client_socket, $ver, $msg_type, $reply); + send_msg($client_socket, $ver, $msg_type, $response); } } @@ -100,7 +101,7 @@ sub recv_msg { my ($sock) = @_; my $header = read_all($sock, 6); - my ($version, $msg_type, $msg_size) = unpack("nnn", $header); + my ($version, $msg_type, $payload_size) = unpack("nnn", $header); if ($version != 0) { $log->print("error: unsupported protocol version $version\n"); @@ -110,17 +111,15 @@ sub recv_msg { $log->print("error: unknown message type $msg_type\n"); exit 0; } - elsif ($msg_size > 4096) { - $log->print("error: $msg_size byte message too large\n"); + elsif ($payload_size > 4096 || $payload_size == 0) { + $log->print("error: $payload_size byte payload invalid\n"); exit 0; } - elsif ($msg_size == 0) { - # don't try and do another read, as a read of size 0 is EOF - return ($version, $msg_type, ""); - } - my $msg = read_all($sock, $msg_size); - return ($version, $msg_type, $msg); + my $payload = read_all($sock, $payload_size); + my $request = decode_json($payload); + + return ($version, $msg_type, $request); } sub read_all { @@ -146,7 +145,9 @@ sub read_all { } sub send_msg { - my ($sock, $ver, $msg_type, $payload) = @_; + my ($sock, $ver, $msg_type, $response) = @_; + + my $payload = encode_json($response); my $header_len = 6; my $payload_len = length($payload); @@ -174,24 +175,24 @@ sub send_all { } sub msg_device_add { - my ($db, $msg) = @_; + my ($db, $request) = @_; - my ($err, $ph_num, $os) = split_fields($msg, 2); - return "err\0$err" if ($err); + my ($err, $ph_num, $os) = unpack_request($db, $request, 'phone_number', 'os'); + return make_error($err) if ($err); unless (looks_like_number($ph_num)) { $log->print("phone number '$ph_num' invalid\n"); - return "err\0the sent phone number is not a number"; + return make_error("the sent phone number is not a number"); } $db->{ph_num_exists}->execute($ph_num); if ($db->{ph_num_exists}->fetchrow_array()) { $log->print("phone number '$ph_num' already exists\n"); - return "err\0the sent phone number already exists"; + return make_error("the sent phone number already exists"); } if ($os ne 'unix' && $os ne 'android' && $os ne 'ios') { $log->print("unknown operating system '$os'\n"); - return "err\0operating system not supported"; + return make_error("operating system not supported"); } my $device_id = sha256_base64(arc4random_bytes(32)); @@ -201,75 +202,77 @@ sub msg_device_add { $db->{select_device_id}->execute($device_id); if ($db->{select_device_id}->fetchrow_array()) { $log->print("id generation collision for '$device_id'\n"); - return "err\0device id collision, please try again" + return make_error("device id collision, please try again"); } $db->{new_device}->execute($device_id, $ph_num, $os, undef, time, time); $log->print("success, '$ph_num':'$fp' os '$os'\n"); - return "ok\0$device_id"; + return make_ok( { device_id => $device_id } ); } sub msg_list_add { - my ($db, $msg) = @_; + my ($db, $request) = @_; - my ($err, $dev_id, $list_num, $list_name, $list_date) = split_fields($msg, 4); - return "err\0$err" if ($err); + my ($err, $dev, $list) = unpack_request($db, $request, 'device_id', 'list'); + return make_error($err) if ($err); - my ($dev_err, $dev_num, $dev_fp, $phnum) = device_id_valid($db, $dev_id); - return "err\0$dev_err" if ($dev_err); - - # Sending a list number of 0 triggers new list mode - if ($list_num == 0) { - $log->print("device '$dev_fp'\n"); - $log->print("new list name '$list_name'\n"); + if ($list->{num} == 0) { + # Sending a list number of 0 triggers new list mode + $log->print("device '$dev->{fp}'\n"); + $log->print("new list name '$list->{name}'\n"); my $now = time; - $db->{new_list}->execute($list_name, $list_date, $now, $now); + $db->{new_list}->execute($list->{name}, $list->{date}, $now, $now); - $list_num = $db->{dbh}->last_insert_id("", "", "", ""); - $log->print("new list number is '$list_num'\n"); + $list->{num} = $db->{dbh}->last_insert_id("", "", "", ""); + $log->print("new list number is '$list->{num}'\n"); - $db->{new_list_member}->execute($list_num, $dev_num, $now); + $db->{new_list_member}->execute($list->{num}, $dev->{num}, $now); # XXX: send push notifications to all my mutual friends to # update their 'other lists' section - - return "ok\0$list_num\0$list_name\0$list_date\0$phnum"; - } - - # Otherwise we're in list update mode - $err = list_number_valid($db, $list_num); - return "err\0$err" if ($err); - - # Check that the device is in the list it wants to update - $db->{check_list_member}->execute($list_num, $dev_num); - unless ($db->{check_list_member}->fetchrow_array()) { - $log->print("device '$dev_fp' not in list '$list_num'\n"); - return "err\0client tried to update a list it was not in"; } + else { + # Otherwise we're in list update mode + $err = list_number_valid($db, $list->{num}); + return make_error($err) if ($err); + + # Check that the device is in the list it wants to update + $db->{check_list_member}->execute($list->{num}, $dev->{num}); + unless ($db->{check_list_member}->fetchrow_array()) { + $log->print("device '$dev->{fp}' not in list '$list->{num}'\n"); + return make_error("client tried to update a list it was not in"); + } - # Do the update - $db->{update_list}->execute($list_name, $list_date, time, $list_num); - $log->print("updated list '$list_num'\n"); - - # XXX: send push notifications to all my mutual friends to update their - # 'other lists' section, also tell all list members to update their - # 'lists' section - - return "ok\0$list_num\0$list_name\0$list_date\0$phnum"; + # Do the update + $db->{update_list}->execute($list->{name}, $list->{date}, time, $list->{num}); + $log->print("updated list '$list->{num}'\n"); + + # XXX: send push notifications to all my mutual friends to update their + # 'other lists' section, also tell all list members to update their + # 'lists' section + } + + my $resp_list = { + num => $list->{num}, + name => $list->{name}, + date => $list->{date}, + items_complete => 0, + items_total => 0, + members => [ $dev->{phnum} ], + num_members => 1 + }; + return make_ok( { list => $resp_list } ); } sub msg_list_item_add { - my ($db, $msg) = @_; - - my ($err, $device_id) = split_fields($msg, 1); - return "err\0$err" if ($err); + my ($db, $request) = @_; - ($err) = device_id_valid($db, $device_id); - return "err\0$err" if ($err); + my ($err, $device) = unpack_request($db, $request, 'device_id'); + return make_error($err) if ($err); - return "err\0unimplemented"; + return make_error("unimplemented"); # my ($list_id, $position, $text) = split ("\0", $msg); @@ -286,98 +289,103 @@ sub msg_list_item_add { } sub msg_list_join { - my ($db, $msg) = @_; + my ($db, $request) = @_; - my ($err, $dev_id, $list_num) = split_fields($msg, 2); - return "err\0$err" if ($err); - - my ($err2, $dev_num, $dev_fp) = device_id_valid($db, $dev_id); - return "err\0$err2" if ($err2); + my ($err, $dev, $list_num) = unpack_request($db, $request, 'device_id', 'list_num'); + return make_error($err) if ($err); $err = list_number_valid($db, $list_num); - return "err\0$err" if ($err); + return make_error($err) if ($err); - $log->print("device '$dev_fp'\n"); + $log->print("device '$dev->{fp}'\n"); $log->print("list '$list_num'\n"); my $time = time; - $db->{check_list_member}->execute($list_num, $dev_num); + $db->{check_list_member}->execute($list_num, $dev->{num}); if (!$db->{check_list_member}->fetchrow_array()) { - $db->{new_list_member}->execute($list_num, $dev_num, $time); - $log->print("device '$dev_fp' has been added to list '$list_num'\n"); + $db->{new_list_member}->execute($list_num, $dev->{num}, $time); + $log->print("device '$dev->{fp}' has been added to list '$list_num'\n"); } else { - $log->print("tried to create a duplicate list member entry for device '$dev_fp' and list '$list_num'\n"); - return "err\0the device is already part of this list"; + $log->print("tried to create a duplicate list member entry for device '$dev->{fp}' and list '$list_num'\n"); + return make_error("the device is already part of this list"); } - return "ok\0$list_num"; + my $list = { + num => $list_num, + # XXX: we should return the following here too + # name => $list_name, + # date => $list_date, + # items_complete => 0, + # items_total => 0, + # members => [ $phnum ], + # num_members => 1 + }; + return make_ok( { list => $list } ); } sub msg_list_leave { - my ($db, $msg) = @_; - - my ($split_err, $dev_id, $list_num) = split_fields($msg, 2); - return "err\0$split_err" if ($split_err); + my ($db, $request) = @_; - my ($dev_err, $dev_num, $dev_fp) = device_id_valid($db, $dev_id); - return "err\0$dev_err" if ($dev_err); + my ($err, $dev, $list_num) = unpack_request($db, $request, 'device_id', 'list_num'); + return make_error($err) if ($err); - my $err = list_number_valid($db, $list_num); - return "err\0$err" if ($err); + $err = list_number_valid($db, $list_num); + return make_error($err) if ($err); - $log->print("device '$dev_fp'\n"); + $log->print("device '$dev->{fp}'\n"); $log->print("list '$list_num'\n"); - $db->{check_list_member}->execute($list_num, $dev_num); + $db->{check_list_member}->execute($list_num, $dev->{num}); if ($db->{check_list_member}->fetchrow_array()) { - $db->{remove_list_member}->execute($list_num, $dev_num); - $log->print("device '$dev_fp' has been removed from list '$list_num'\n"); + $db->{remove_list_member}->execute($list_num, $dev->{num}); + $log->print("device '$dev->{fp}' has been removed from list '$list_num'\n"); } else { - $log->print("tried to leave a list the user was not in for device '$dev_fp' and list '$list_num'\n"); - return "err\0the client was not a member of the list"; + $log->print("tried to leave a list the user was not in for device '$dev->{fp}' and list '$list_num'\n"); + return make_error("the client was not a member of the list"); } $db->{check_list_member}->finish(); $db->{get_list_members}->execute($list_num); - my $alive = 1; + my $list_empty = 0; if (!$db->{get_list_members}->fetchrow_array()) { $log->print("list '$list_num' is empty... deleting\n"); $db->{delete_list}->execute($list_num); $db->{delete_list_data}->execute($list_num); - $alive = 0; + $list_empty = 1; } - return "ok\0$list_num\0$alive"; + my $response = { + list_num => $list_num, + list_empty => $list_empty + }; + return make_ok($response); } sub msg_friend_add { - my ($db, $msg) = @_; + my ($db, $request) = @_; - my ($err, $dev_id, $friend_phnum) = split_fields($msg, 2); - return "err\0$err" if ($err); + my ($err, $dev, $friend_phnum) = unpack_request($db, $request, 'device_id', 'friend_phnum'); + return make_error($err) if ($err); - my ($dev_err, $dev_num, $dev_fp, $dev_phnum) = device_id_valid($db, $dev_id); - return "err\0$dev_err" if ($dev_err); - - $log->print("'$dev_fp' adding '$friend_phnum'\n"); + $log->print("'$dev->{fp}' adding '$friend_phnum'\n"); unless (looks_like_number($friend_phnum)) { $log->print("bad friends number '$friend_phnum'\n"); - return "err\0friends phone number is not a valid phone number"; + return make_error("friends phone number is not a valid phone number"); } # Check if I'm adding myself as a friend - if ($dev_phnum eq $friend_phnum) { - $log->print("device '$dev_fp' tried adding itself\n"); - return "err\0device cannot add itself as a friend"; + if ($dev->{phnum} eq $friend_phnum) { + $log->print("device '$dev->{fp}' tried adding itself\n"); + return make_error("device cannot add itself as a friend"); } # Add a 1 way friendship for this person - $db->{friends_insert}->execute($dev_num, $friend_phnum); + $db->{friends_insert}->execute($dev->{num}, $friend_phnum); # Check if the added friend has registered their phone number $db->{ph_num_exists}->execute($friend_phnum); @@ -388,43 +396,39 @@ sub msg_friend_add { $log->print("friends device id is '$friend_fp'\n"); # Check if my phone number is in their friends list - $db->{friends_select}->execute($friend_num, $dev_phnum); + $db->{friends_select}->execute($friend_num, $dev->{phnum}); if ($db->{friends_select}->fetchrow_array()) { $log->print("found mutual friendship\n"); # Adding both is technically not necessary but makes # lookups easier - $db->{mutual_friend_insert}->execute($dev_num, $friend_num); - $db->{mutual_friend_insert}->execute($friend_num, $dev_num); + $db->{mutual_friend_insert}->execute($dev->{num}, $friend_num); + $db->{mutual_friend_insert}->execute($friend_num, $dev->{num}); } } - - return "ok\0$friend_phnum"; + return make_ok( { friend_phnum => $friend_phnum } ); } sub msg_friend_delete { - my ($db, $msg) = @_; + my ($db, $request) = @_; - my ($err, $dev_id, $friend_phnum) = split_fields($msg, 2); - return "err\0$err" if ($err); - - my ($dev_err, $dev_num, $dev_fp) = device_id_valid($db, $dev_id); - return "err\0$dev_err" if ($dev_err); + my ($err, $dev, $friend_phnum) = unpack_request($db, $request, 'device_id', 'friend_phnum'); + return make_error($err) if ($err); unless (looks_like_number($friend_phnum)) { $log->print("bad friends number '$friend_phnum'\n"); - return "err\0friends phone number is not a valid phone number"; + return make_error("friends phone number is not a valid phone number"); } - $db->{friends_select}->execute($dev_num, $friend_phnum); + $db->{friends_select}->execute($dev->{num}, $friend_phnum); if ($db->{friends_select}->fetchrow_array()) { $log->print("removing '$friend_phnum' from friends list\n"); - $db->{friends_delete}->execute($dev_num, $friend_phnum); + $db->{friends_delete}->execute($dev->{num}, $friend_phnum); } else { $log->print("tried deleting friend '$friend_phnum' but they weren't a friend\n"); - return "err\0friend sent for deletion was not a friend"; + return make_error("friend sent for deletion was not a friend"); } # Check for and delete any mutual friend references @@ -432,23 +436,23 @@ sub msg_friend_delete { if (my ($friend_num) = $db->{ph_num_exists}->fetchrow_array()) { $log->print("also removing mutual friend relationship\n"); - $db->{mutual_friends_delete}->execute($dev_num, $friend_num); - $db->{mutual_friends_delete}->execute($friend_num, $dev_num); + $db->{mutual_friends_delete}->execute($dev->{num}, $friend_num); + $db->{mutual_friends_delete}->execute($friend_num, $dev->{num}); } - return "ok\0$friend_phnum"; + return make_ok( { friend_phnum => $friend_phnum } ); } sub msg_lists_get { - my ($db, $dev_id) = @_; + my ($db, $request) = @_; - my ($err, $dev_num, $dev_fp, $phnum) = device_id_valid($db, $dev_id); - return "err\0$err" if ($err); + my ($err, $dev) = unpack_request($db, $request, 'device_id'); + return make_error($err) if ($err); - $log->print("gathering lists for '$dev_fp'\n"); + $log->print("gathering lists for '$dev->{fp}'\n"); my @lists; - $db->{get_lists}->execute($dev_num); + $db->{get_lists}->execute($dev->{num}); while (my ($list_num, $list_name) = $db->{get_lists}->fetchrow_array()) { $log->print("found list '$list_num':'$list_name'\n"); @@ -459,96 +463,108 @@ sub msg_lists_get { while (my ($member_num) = $db->{get_list_members}->fetchrow_array()) { # Don't re look-up our own number - if ($member_num eq $dev_num) { - push @members, $phnum; + if ($member_num eq $dev->{num}) { + push @members, $dev->{phnum}; next; } - push @members, devnum_to_phnum($db, $member_num); } - my $members = join("\0", @members); - $log->print("list has " . @members . " members\n"); - - # Find how many items are complete in this list - my $num_items = 0; - $db->{get_list_items}->execute($list_num); - while (my @results = $db->{get_list_items}->fetchrow_array()) { - my (undef, $item_name, $item_status) = @results; - # XXX: actually check the item status - $num_items++; - } - $log->print("list has $num_items items\n"); - - push @lists, "$list_num\0$list_name\0$num_items\0$members"; - } - return "ok\0" . join("\n", @lists); + my $list = { + num => $list_num, + name => $list_name, + date => 0, + items_complete => 0, + items_total => 0, + members => \@members, + num_members => scalar(@members) + }; + push @lists, $list; + + $log->print("list has $list->{num_members} members\n"); + $log->print("list has $list->{items_total} items\n"); + } + + my $response = { + lists => \@lists, + num_lists => scalar(@lists) + }; + return make_ok($response); } sub msg_lists_get_other { - my ($db, $dev_id) = @_; + my ($db, $request) = @_; - my ($err, $dev_num, $dev_fp) = device_id_valid($db, $dev_id); - return "err\0$err" if ($err); + my ($err, $dev) = unpack_request($db, $request, 'device_id'); + return make_error($err) if ($err); - $log->print("gathering lists for '$dev_fp'\n"); + $log->print("gathering lists for '$dev->{fp}'\n"); - my @list_nums; - $db->{get_lists}->execute($dev_num); + # Get lists this device is already in, used for filtering later + my @my_lists; + $db->{get_lists}->execute($dev->{num}); while (my ($list_num) = $db->{get_lists}->fetchrow_array()) { - push @list_nums, $list_num; + push @my_lists, $list_num; } - # Find all mutual friends of this device number - my (%members, %names); - $db->{mutual_friend_select}->execute($dev_num); + my %list_nums; + # Find all mutual friends of this device + $db->{mutual_friend_select}->execute($dev->{num}); while (my ($friend_num) = $db->{mutual_friend_select}->fetchrow_array()) { # We can't send device id's back to the client my $friend_phnum = devnum_to_phnum($db, $friend_num); - $log->print("found mutual friend '$friend_phnum'\n"); # Find all of the lists my mutual friend is in (but not me) $db->{get_lists}->execute($friend_num); - while (my ($num, $name) = $db->{get_lists}->fetchrow_array()) { + while (my ($list_num, $name) = $db->{get_lists}->fetchrow_array()) { + + # Filter out lists my device is already in + next if (grep {$_ eq $list_num} @my_lists); - # filter out lists this device id is already in - next if (grep {$_ eq $num} @list_nums); + if (exists $list_nums{$list_num}) { + # Append member and move on + push @{ $list_nums{$list_num}->{members} }, $friend_phnum; + $list_nums{$list_num}->{num_members} += 1; + next + } - push(@{ $members{$num} }, $friend_phnum); - $names{$num} = $name; + my $list = { + num => $list_num, + name => $name, + members => [ $friend_phnum ], + num_members => 1 + }; + $list_nums{$list_num} = $list; $log->print("found list '$name'\n"); } } - my @lists; - for (keys %names) { - push @lists, "$_\0$names{$_}\0" . join("\0", @{$members{$_}}); - } - - return "ok\0" . join("\n", @lists); + my @other_lists = values(%list_nums); + my $response = { + other_lists => \@other_lists, + num_other_lists => scalar(@other_lists) + }; + return make_ok($response); } sub msg_list_items_get { - my ($db, $msg) = @_; - - my ($err, $device_id, $list_id) = split_fields($msg, 2); - return "err\0$err" if ($err); + my ($db, $request) = @_; - ($err) = device_id_valid($db, $device_id); - return "err\0$err" if ($err); + my ($err, $dev, $list_id) = unpack_request($db, $request, 'device_id', 'list_num'); + return make_error($err) if ($err); if (!$list_id) { $log->print("received null list id"); - return "err\0the sent list id was empty"; + return make_error("the sent list id was empty"); } # unless ($dbh->selectrow_array($sth{check_list_member}, undef, $list_id, $device_id)) { # # XXX: table list_members list_id's should always exist in table lists # $log->print("list_items: $device_id not a member of $list_id\n"); # return "err\0the sent device id is not a member of the list"; # } - $log->print("$device_id request items for $list_id\n"); + $log->print("$dev->{id} request items for $list_id\n"); $db->{get_list_items}->execute($list_id); @@ -561,26 +577,13 @@ sub msg_list_items_get { } my $out = join("\0", @items); - return "ok\0$out"; + return make_ok(); } sub fingerprint { return substr shift, 0, 8; } -sub split_fields { - my ($msg, $total_fields) = @_; - - my @fields = split("\0", $msg, $total_fields); - if (@fields != $total_fields) { - my $fields = @fields; - $log->print("got $fields fields, expected $total_fields\n"); - return ("the wrong number of message arguments were sent"); - } - - return (undef, @fields); -} - sub devnum_to_phnum { my ($db, $dev_num) = @_; @@ -590,24 +593,6 @@ sub devnum_to_phnum { return $ph_num; } -sub device_id_valid { - my ($db, $device_id) = @_; - - unless ($device_id =~ m/^[a-zA-Z0-9+\/=]*$/) { - $log->print("'$device_id' not base64\n"); - return ('the client sent a device id that was not base64'); - } - - $db->{select_device_id}->execute($device_id); - if (my ($num, $id, $phnum) = $db->{select_device_id}->fetchrow_array()) { - my $fp = fingerprint($id); - return (undef, $num, $fp, $phnum); - } - - $log->print("unknown device '$device_id'\n"); - return ('the client sent an unknown device id'); -} - sub list_number_valid { my ($db, $list_num) = @_; @@ -625,6 +610,66 @@ sub list_number_valid { return; } +sub make_error { + my ($reason) = @_; + return { status => 'err', reason => $reason }; +} + +sub make_ok { + my ($args) = @_; + + $args->{status} = 'ok'; + return $args; +} + +sub unpack_request { + # Function that gets values from a hash in a specified key order + my ($db, $request, @expected_keys) = @_; + + my @values; + for (@expected_keys) { + # Check if the caller requested a key that was not provided + if (! exists $request->{$_}) { + $log->print("bad request, missing key '$_'\n"); + return ("a missing message argument was required"); + } + + if ($_ ne 'device_id') { + push @values, $request->{$_}; + next; + } + + # When callers request 'device_id' we check the id is valid here + my ($err, $device) = device_id_valid($db, $request->{$_}); + return ($err) if ($err); + + push @values, $device; + } + return (undef, @values); +} + +sub device_id_valid { + my ($db, $device_id) = @_; + + unless ($device_id && $device_id =~ m/^[a-zA-Z0-9+\/=]+$/) { + $log->print("bad device id\n"); + return ('the client sent a device id that was not base64'); + } + + $db->{select_device_id}->execute($device_id); + if (my ($num, $id, $phnum) = $db->{select_device_id}->fetchrow_array()) { + my $device = { + num => $num, + fp => fingerprint($id), + phnum => $phnum, + id => $id + }; + return (undef, $device); + } + + $log->print("unknown device '$device_id'\n"); + return ('the client sent an unknown device id'); +} package database; diff --git a/server/tests/client.pm b/server/tests/client.pm @@ -3,6 +3,7 @@ use strict; use warnings; use IO::Socket::SSL; +use JSON::PP; use test; require "msgs.pl"; @@ -26,10 +27,9 @@ sub new { die "failed connect or ssl handshake: $!,$SSL_ERROR"; } - # make sure we don't try and use this without setting it $self->{device_id} = undef; - # By default register this device immediately + # Register this device immediately by default if ($dont_register == 0) { $self->device_add(rand_phnum()); } @@ -39,173 +39,132 @@ sub new { sub list_add { my $self = shift; - my $name = shift; + my $list = { + num => 0, + name => shift, + date => 0 + }; my $status = shift || 'ok'; - my $list_data = communicate($self, 'list_add', $status, 0, $name, 0); - + my $response = communicate($self, 'list_add', $status, { list => $list }); return if ($status eq 'err'); - save_list($self, $list_data); + + push @{$self->{lists}}, $response->{list}; } sub list_update { my $self = shift; - my $num = shift; - my $name = shift; - my $date = shift || 0; + my $list = { + num => shift, + name => shift, + date => shift + }; my $status = shift || 'ok'; - my $list_data = communicate($self, 'list_add', $status, $num, $name, $date); + my $list_data = communicate($self, 'list_add', $status, { list => $list }); return if ($status eq 'err'); } sub list_join { my $self = shift; - my $list_id = shift; + my $msg_args = { + list_num => shift, + }; my $status = shift || 'ok'; - my $list_data = communicate($self, 'list_join', $status, $list_id); + my $list_data = communicate($self, 'list_join', $status, $msg_args); } sub list_leave { my $self = shift; - my $id = shift; + my $msg_args = { + list_num => shift, + }; my $status = shift || 'ok'; - my $list_data = communicate($self, 'list_leave', $status, $id); -} - -sub save_list { - my $self = shift; - my $list_data = shift; - - my ($num, $name, $date, @members) = split("\0", $list_data); - my $list = { - id => $num, - name => $name, - members => \@members, - num_members => scalar(@members), - }; - push @{$self->{lists}}, $list; + my $list_data = communicate($self, 'list_leave', $status, $msg_args); } sub friend_add { my $self = shift; - my $friend = shift; + my $msg_args = { + friend_phnum => shift, + }; my $status = shift || 'ok'; - communicate($self, 'friend_add', $status, $friend); + communicate($self, 'friend_add', $status, $msg_args); } sub friend_delete { my $self = shift; - my $friend = shift; + my $msg_args = { + friend_phnum => shift, + }; my $status = shift || 'ok'; - communicate($self, 'friend_delete', $status, $friend); + communicate($self, 'friend_delete', $status, $msg_args); } sub lists_get { my $self = shift; my $status = shift || 'ok'; - my $list_data = communicate($self, 'lists_get', $status); - my @lists; - - for (split("\n", $list_data)) { - my ($id, $name, $num, @members) = split("\0", $_); - # fail "bad list" unless ($id && $name && @members != 0); - - my %tmp = ( - id => $id, - name => $name, - num => $num, - members => \@members, - num_members => scalar(@members), - ); - push @lists, \%tmp; - } - return @lists; + my $response = communicate($self, 'lists_get', $status); + return if ($response->{status} eq 'err'); + + return @{ $response->{lists} }; } sub lists_get_other { my $self = shift; my $status = shift || 'ok'; - my $list_data = communicate($self, 'lists_get_other', $status); - my @lists; - - for (split("\n", $list_data)) { - my ($id, $name, @members) = split("\0", $_); - # fail "bad other list" unless ($id && $name && @members != 0); + my $response = communicate($self, 'lists_get_other', $status); + return if ($response->{status} eq 'err'); - my %tmp = ( - id => $id, - name => $name, - members => \@members, - num_members => scalar(@members) - ); - push @lists, \%tmp; - } - return @lists; + return @{ $response->{other_lists} }; } sub device_add { my $self = shift; - my $phone_number = shift || '4038675309'; - my $os = shift || 'unix'; + my $msg_args = { + phone_number => shift || '4038675309', + os => shift || 'unix' + }; my $exp_status = shift || 'ok'; - # always reset error messages to guard against stale state + # Reset error messages to guard against stale state $self->{err_msg} = undef; $self->{msg_type} = $msg_num{'device_add'}; - send_msg($self, "$phone_number\0$os"); - my $msg = recv_msg($self); - - my ($status, $device_id) = parse_status($self, $msg); - fail "wrong message status '$status'" if ($status ne $exp_status); + send_msg($self, $msg_args); + my $response = recv_msg($self, $exp_status); - $self->{phnum} = $phone_number; - $self->{device_id} = $device_id; + if ($response->{status} eq 'ok') { + $self->{phnum} = $msg_args->{phone_number}; + $self->{device_id} = $response->{device_id}; + } } sub communicate { - my ($self, $msg_type, $exp_status, @msg_args) = @_; + my ($self, $msg_type, $exp_status, $msg_args) = @_; - # always reset error messages to guard against stale state + # Reset error message so it doesn't get reused $self->{err_msg} = undef; $self->{msg_type} = $msg_num{$msg_type}; - # prepend device id to @msg_args array - unshift @msg_args, $self->{device_id}; - - send_msg($self, join("\0", @msg_args)); - my $msg = recv_msg($self); + # Add device id to message arguments + $msg_args->{device_id} = $self->{device_id}; - my ($status, $payload) = parse_status($self, $msg); - fail "wrong message status '$status'" if ($status ne $exp_status); - - return $payload; -} - -sub parse_status { - my ($self, $msg) = @_; - - my $first_null = index($msg, "\0"); - fail "no null byte found in response" if ($first_null == -1); - - my $msg_status = substr($msg, 0, $first_null); - my $msg_rest = substr($msg, $first_null + 1); - - # if this is an error message keep track of it right now - $self->{err_msg} = $msg_rest if ($msg_status eq 'err'); - - return ($msg_status, $msg_rest); + send_msg($self, $msg_args); + return recv_msg($self, $exp_status); } sub send_msg { - my ($self, $payload) = @_; + my ($self, $request) = @_; + + # Request comes in as a hash ref, do this now to figure out length + my $payload = encode_json($request); my $msg_type = $self->{msg_type}; fail "invalid message type $msg_type" if ($msg_type > @msg_str); @@ -233,22 +192,28 @@ sub send_all { } sub recv_msg { - my ($self) = @_; + my ($self, $exp_status) = @_; - # read header part first + # Read header my $header = read_all($self, 6); my ($version, $msg_type, $payload_size) = unpack("nnn", $header); + # Check some things fail "unsupported protocol version $version" if ($version != 0); fail "unknown message type $msg_type" if ($msg_type >= @msg_str); fail "$payload_size byte message too large" if ($payload_size > 4096); + fail "0 byte payload" if ($payload_size == 0); fail "unexpected message type $self->{msg_type}" if ($self->{msg_type} != $msg_type); - # don't try a read_all() of size 0 - return '' if ($payload_size == 0); - + # Read again for payload, $payload_size > 0 my $payload = read_all($self, $payload_size); - return $payload; + my $response = decode_json($payload); + + my $status = $response->{status}; + fail "wrong message status '$status'" if ($status ne $exp_status); + $self->{err_msg} = $response->{reason} if ($status eq 'err'); + + return $response; } sub read_all { @@ -269,32 +234,22 @@ sub read_all { } sub num_lists { - my $self = shift; + my ($self) = @_; return scalar(@{$self->{lists}}); } sub lists { - my $self = shift; - my $i = shift; - - my $num_lists = scalar(@{$self->{lists}}); - fail "tried accessing out of bounds index $i" if ($i > $num_lists); - + my ($self, $i) = @_; return $self->{lists}[$i]; } -sub lists_all { - my $self = shift; - return $self->{lists}; -} - sub phnum { - my $self = shift; + my ($self) = @_; return $self->{phnum}; } sub device_id { - my $self = shift; + my ($self) = @_; return $self->{device_id}; } diff --git a/server/tests/friend_delete/test.pl b/server/tests/friend_delete/test.pl @@ -18,7 +18,7 @@ $A->list_add("this is a's second list"); $B->list_add("this is b's first list"); # B joins one of A's list -$B->list_join($A->lists(0)->{'id'}); +$B->list_join($A->lists(0)->{num}); # A deletes B's friendship $A->friend_delete($B->phnum()); diff --git a/server/tests/get_other_lists_filters_my_lists/test.pl b/server/tests/get_other_lists_filters_my_lists/test.pl @@ -20,7 +20,7 @@ $B->friend_add($A->phnum()); # A adds a new list, B joins A's new list $A->list_add('as new list'); -$B->list_join($A->lists(0)->{'id'}); +$B->list_join($A->lists(0)->{num}); # A should only see B's list that it never joined my @other = $A->lists_get_other(); diff --git a/server/tests/header/server.log.good b/server/tests/header/server.log.good @@ -6,7 +6,7 @@ ssl ok, ver = 'TLSv1_2' cipher = 'ECDHE-RSA-AES128-SHA256' error: unsupported protocol version 101 new connection (pid = <digits>) ssl ok, ver = 'TLSv1_2' cipher = 'ECDHE-RSA-AES128-SHA256' -error: 25143 byte message too large +error: 25143 byte payload invalid new connection (pid = <digits>) ssl ok, ver = 'TLSv1_2' cipher = 'ECDHE-RSA-AES128-SHA256' disconnected! diff --git a/server/tests/invalid_deviceid/server.log.good b/server/tests/invalid_deviceid/server.log.good @@ -1,17 +1,24 @@ new connection (pid = <digits>) ssl ok, ver = 'TLSv1_2' cipher = 'ECDHE-RSA-AES128-SHA256' -friend_add: unknown device <digits> -friend_delete: unknown device <digits> -list_add: unknown device <digits> -list_join: unknown device <digits> -list_leave: unknown device <digits> -lists_get: unknown device <digits> -lists_get_other: unknown device <digits> -friend_add: 'somebull$hit' not base64 -friend_delete: 'somebull$hit' not base64 -list_add: 'somebull$hit' not base64 -list_join: 'somebull$hit' not base64 -list_leave: 'somebull$hit' not base64 -lists_get: 'somebull$hit' not base64 -lists_get_other: 'somebull$hit' not base64 +friend_add: bad device id +friend_delete: bad device id +list_add: bad device id +list_join: bad device id +list_leave: bad device id +lists_get: bad device id +lists_get_other: bad device id +friend_add: bad device id +friend_delete: bad device id +list_add: bad device id +list_join: bad device id +list_leave: bad device id +lists_get: bad device id +lists_get_other: bad device id +friend_add: unknown device <base64> +friend_delete: unknown device <base64> +list_add: unknown device <base64> +list_join: unknown device <base64> +list_leave: unknown device <base64> +lists_get: unknown device <base64> +lists_get_other: unknown device <base64> disconnected! diff --git a/server/tests/invalid_deviceid/test.pl b/server/tests/invalid_deviceid/test.pl @@ -1,18 +1,19 @@ #!/usr/bin/perl -I../ use strict; use warnings; - use client; use test; # test that sending invalid device id's results in errors my $A = client->new(1); -my @device_ids = ('', 'somebull$hit'); -my @good_msgs = ('the client sent an unknown device id', - 'the client sent a device id that was not base64'); +my @device_ids = ('' , 'somebull$hit', 'legit'); +my @good_msgs = ('the client sent a device id that was not base64', + 'the client sent a device id that was not base64', + 'the client sent an unknown device id' +); -for (0..1) { +for (0..2) { $A->set_device_id($device_ids[$_]); # for messages that send 2 arguments, send an empty 2nd argument diff --git a/server/tests/leave_list_your_not_in/test.pl b/server/tests/leave_list_your_not_in/test.pl @@ -13,5 +13,5 @@ my $B = client->new(); $A->list_add('only a can see this list'); # Who knows how B got this list id, but he did -$B->list_leave($A->lists(0)->{'id'}, 'err'); +$B->list_leave($A->lists(0)->{num}, 'err'); fail_msg_ne 'the client was not a member of the list', $B->get_error(); diff --git a/server/tests/list_add/test.pl b/server/tests/list_add/test.pl @@ -11,7 +11,7 @@ my $A = client->new(); $A->list_add(my $name = 'this is a new list'); my $list = $A->lists(0); -fail "list num isn't numeric" unless (looks_like_number($list->{id})); +fail "list num isn't numeric" unless (looks_like_number($list->{num})); fail_msg_ne $name, $list->{name}; fail_num_ne "wrong number of members", $list->{num_members}, 1; fail_msg_ne $list->{members}->[0], $A->phnum(); diff --git a/server/tests/list_join/test.pl b/server/tests/list_join/test.pl @@ -17,7 +17,7 @@ my $list_name = "this is a new list"; $A->list_add($list_name); # B joins A's list -$B->list_join($A->lists(0)->{'id'}); +$B->list_join($A->lists(0)->{num}); # B requests its lists to make sure its committed to the list my @lists = $B->lists_get(); diff --git a/server/tests/list_join_unit/test.pl b/server/tests/list_join_unit/test.pl @@ -1,17 +1,16 @@ #!/usr/bin/perl -I../ use strict; use warnings; - use client; use test; my $A = client->new(); -# try joining a list that doesn't exist +# Try joining a list that doesn't exist $A->list_join('12345678', 'err'); fail_msg_ne 'the client sent an unknown list number', $A->get_error(); -# test joining a list your already in +# Test joining a list your already in $A->list_add('my new test list'); -$A->list_join($A->lists(0)->{'id'}, 'err'); +$A->list_join($A->lists(0)->{num}, 'err'); fail_msg_ne 'the device is already part of this list', $A->get_error(); diff --git a/server/tests/list_leave/test.pl b/server/tests/list_leave/test.pl @@ -1,14 +1,13 @@ #!/usr/bin/perl -I../ use strict; use warnings; - use client; use test; my $A = client->new(); $A->list_add('this list was made for leaving'); -$A->list_leave($A->lists(0)->{id}); +$A->list_leave($A->lists(0)->{num}); # verify we don't get this list back when requesting all lists my @lists = $A->lists_get(); diff --git a/server/tests/list_reference_counting/test.pl b/server/tests/list_reference_counting/test.pl @@ -11,7 +11,7 @@ my $B = client->new(); # A creates a new list $A->list_add('this list will belong to B soon enough'); -my $list_id = $A->lists(0)->{'id'}; +my $list_id = $A->lists(0)->{num}; # XXX: missing steps # - A and B become mutual friends diff --git a/server/tests/list_update/test.pl b/server/tests/list_update/test.pl @@ -12,7 +12,7 @@ fail_msg_ne 'the client sent an unknown list number', $A->get_error(); # Make sure a normal list_update works $A->list_add('this is a new list'); -$A->list_update($A->lists(0)->{id}, 'this is an updated name', 54345); +$A->list_update($A->lists(0)->{num}, 'this is an updated name', 54345); my @lists = $A->lists_get(); fail_msg_ne $lists[0]->{name}, 'this is an updated name'; diff --git a/server/tests/lists_get/test.pl b/server/tests/lists_get/test.pl @@ -1,35 +1,26 @@ #!/usr/bin/perl -I../ use strict; use warnings; - use client; use test; -# this test: -# - gets a new device id -# - creates some new lists -# - requests all lists -# - checks that what's received is what was sent - my $A = client->new(); -my $phnum = $A->phnum(); +# Create 3 new lists for ('new list 1', 'new list 2', 'new list 3') { $A->list_add($_); } -my @client_lists = $A->lists_all(); -my @lists = $A->lists_get(); my $i = 0; -my $num_lists = 0; -for (@lists) { - my $list = $A->lists($i++); - - #fail_msg_ne $_->{'id'}, $list->{'id'}; - fail_num_ne 'wrong number of members', $_->{num_members}, $list->{num_members}; - fail_msg_ne $phnum, $_->{members}->[0]; - #fail_msg_ne $_->{'name'}, $list->{name}; +# Verify the information from lists_get matches what we know if true +for my $list ($A->lists_get()) { + my $num = $list->{num}; + my $stored_list = $A->lists($i++); - $num_lists++; + fail_msg_ne $list->{num}, $stored_list->{num}; + fail_num_ne 'wrong number of members', $list->{num_members}, $stored_list->{num_members}; + fail_msg_ne $A->phnum, $list->{members}->[0]; + fail_msg_ne $list->{name}, $stored_list->{name}; + fail_num_ne 'date not the same', $list->{date}, $stored_list->{date}; } -fail_num_ne 'wrong number of lists', $num_lists, 3; +fail_num_ne 'wrong number of lists', $i, 3; diff --git a/server/tests/lists_get_other/test.pl b/server/tests/lists_get_other/test.pl @@ -1,28 +1,25 @@ #!/usr/bin/perl -I../ use strict; use warnings; - use client; use test; -# this test: -# - gets two new device id's -# - the two devices add each other mutually -# - device 1 creates a new list -# - then verify that device 2 can see it - +# Create A and B my $A = client->new(); my $B = client->new(); +# A and B become mutual friends $A->friend_add($B->phnum()); $B->friend_add($A->phnum()); +# A adds a new list $A->list_add('this is a new list that B can see'); my $as_list = $A->lists(0); +# Check that B can see As list my @other_lists = $B->lists_get_other(); fail_msg_ne $other_lists[0]->{name}, $as_list->{'name'}; -fail_msg_ne $other_lists[0]->{id}, $as_list->{'id'}; +fail_msg_ne $other_lists[0]->{num}, $as_list->{'num'}; fail_num_ne 'wrong number of list members', $other_lists[0]->{num_members}, 1; fail_msg_ne $other_lists[0]->{members}->[0], $A->phnum(); fail_num_ne 'wrong number of other lists', scalar(@other_lists), 1; diff --git a/server/tests/multiple_friends_same_other_list/server.log.good b/server/tests/multiple_friends_same_other_list/server.log.good @@ -25,7 +25,6 @@ list_join: device <base64> list_join: device <base64> has been added to list <digits> list_join: list <digits> lists_get_other: found list <string> -lists_get_other: found list <string> lists_get_other: found mutual friend <digits> lists_get_other: found mutual friend <digits> lists_get_other: gathering lists for <base64> diff --git a/server/tests/multiple_friends_same_other_list/test.pl b/server/tests/multiple_friends_same_other_list/test.pl @@ -23,12 +23,12 @@ $C->friend_add($A->phnum()); # B and C need to be in the same list $B->list_add('this is Bs new list'); -$C->list_join($B->lists(0)->{'id'}); +$C->list_join($B->lists(0)->{num}); # A makes sure he got a single list my @other = $A->lists_get_other(); fail_num_ne 'wrong number of list members', $other[0]->{num_members}, 2; -fail_msg_ne $other[0]->{id}, $B->lists(0)->{'id'}; +fail_msg_ne $other[0]->{num}, $B->lists(0)->{num}; fail_num_ne 'wrong number of lists', scalar(@other), 1; fail "A found unexpectedly" if (grep {$_ eq $A->phnum()} @{$other[0]->{members}}); fail "member B not found" unless (grep {$_ eq $B->phnum()} @{$other[0]->{members}}); diff --git a/server/tests/update_list_youre_not_in/test.pl b/server/tests/update_list_youre_not_in/test.pl @@ -12,5 +12,5 @@ my $B = client->new(); $A->list_add('this is a new list for a'); # B tries to update A's list without joining it first -$B->list_update($A->lists(0)->{id}, 'some new title', 123, 'err'); +$B->list_update($A->lists(0)->{num}, 'some new title', 123, 'err'); fail_msg_ne 'client tried to update a list it was not in', $B->get_error(); diff --git a/server/tests/zero_payload/server.log.good b/server/tests/zero_payload/server.log.good @@ -1,13 +1,13 @@ new connection (pid = <digits>) ssl ok, ver = 'TLSv1_2' cipher = 'ECDHE-RSA-AES128-SHA256' -device_add: got 0 fields, expected 2 -friend_add: got 0 fields, expected 2 -friend_delete: got 0 fields, expected 2 -list_add: got 0 fields, expected 4 -list_join: got 0 fields, expected 2 -list_leave: got 0 fields, expected 2 -lists_get: unknown device <digits> -lists_get_other: unknown device <digits> -list_items_get: got 0 fields, expected 2 -list_item_add: got 0 fields, expected 1 +device_add: bad request, missing key 'phone_number' +friend_add: bad request, missing key 'device_id' +friend_delete: bad request, missing key 'device_id' +list_add: bad request, missing key 'device_id' +list_join: bad request, missing key 'device_id' +list_leave: bad request, missing key 'device_id' +lists_get: bad request, missing key 'device_id' +lists_get_other: bad request, missing key 'device_id' +list_items_get: bad request, missing key 'device_id' +list_item_add: bad request, missing key 'device_id' disconnected! diff --git a/server/tests/zero_payload/test.pl b/server/tests/zero_payload/test.pl @@ -12,20 +12,9 @@ my $A = client->new(1); # Send size zero payload to all message types for (@msg_str) { - $A->set_msg_type($_); - $A->send_msg(''); - my $response = $A->recv_msg(); + $A->set_msg_type( $_ ); - my ($status, $err_str) = split("\0", $response, 2); - fail "unexpected status '$status'" if ($status ne 'err'); - - # Depending on the number of expected arguments, an empty message can be - # either the wrong number of arguments (because no '\0' was sent) or an - # unknown device id (for messages expecting 1 argument) - my $good1 = 'the wrong number of message arguments were sent'; - my $good2 = 'the client sent an unknown device id'; - - next if ($err_str eq $good1 || $err_str eq $good2); - - fail "expected either '$good1' or '$good2', instead got '$err_str'"; + $A->send_msg( {} ); + my $response = $A->recv_msg('err'); + fail_msg_ne 'a missing message argument was required', $response->{reason}; }