Skip to content

Commit

Permalink
don't return async handle if login aborted synchronously (#288)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jamiras
Jamiras authored Nov 12, 2023
1 parent ff8e169 commit c6887b2
Show file tree
Hide file tree
Showing 2 changed files with 147 additions and 17 deletions.
47 changes: 42 additions & 5 deletions src/rc_client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,28 @@ void rc_client_abort_async(rc_client_t* client, rc_client_async_handle_t* async_
}
}

static int rc_client_async_handle_valid(rc_client_t* client, rc_client_async_handle_t* async_handle)
{
int valid = 0;
size_t i;

/* there is a small window of opportunity where the client could have been destroyed before calling
* this function, but this function assumes the possibility that the handle has been destroyed, so
* we can't check it for RC_CLIENT_ASYNC_DESTROYED before attempting to scan the client data */
rc_mutex_lock(&client->state.mutex);

for (i = 0; i < sizeof(client->state.async_handles) / sizeof(client->state.async_handles[0]); ++i) {
if (client->state.async_handles[i] == async_handle) {
valid = 1;
break;
}
}

rc_mutex_unlock(&client->state.mutex);

return valid;
}

static const char* rc_client_server_error_message(int* result, int http_status_code, const rc_api_response_t* response)
{
if (!response->succeeded) {
Expand Down Expand Up @@ -461,6 +483,10 @@ static int rc_client_should_retry(const rc_api_server_response_t* server_respons
/* connection to server from cloudfare was dropped before request was completed */
return 1;

case 525: /* 525 SSL Handshake Failed */
/* web server worker connection pool is exhausted */
return 1;

case RC_API_SERVER_RESPONSE_RETRYABLE_CLIENT_ERROR:
/* client provided non-HTTP error (explicitly retryable) */
return 1;
Expand Down Expand Up @@ -614,7 +640,13 @@ static rc_client_async_handle_t* rc_client_begin_login(rc_client_t* client,

rc_api_destroy_request(&request);

return &callback_data->async_handle;
/* if the user state has changed, the async operation completed synchronously */
rc_mutex_lock(&client->state.mutex);
if (client->state.user != RC_CLIENT_USER_STATE_LOGIN_REQUESTED)
callback_data = NULL;
rc_mutex_unlock(&client->state.mutex);

return callback_data ? &callback_data->async_handle : NULL;
}

rc_client_async_handle_t* rc_client_begin_login_with_password(rc_client_t* client,
Expand Down Expand Up @@ -2475,6 +2507,7 @@ rc_client_async_handle_t* rc_client_begin_change_media(rc_client_t* client, cons
else {
/* call the server to make sure the hash is valid for the loaded game */
rc_client_load_state_t* callback_data;
rc_client_async_handle_t* async_handle;
rc_api_resolve_hash_request_t resolve_hash_request;
rc_api_request_t request;
int result;
Expand All @@ -2500,12 +2533,14 @@ rc_client_async_handle_t* rc_client_begin_change_media(rc_client_t* client, cons
callback_data->hash = game_hash;
callback_data->game = game;

rc_client_begin_async(client, &callback_data->async_handle);
async_handle = &callback_data->async_handle;
rc_client_begin_async(client, async_handle);
client->callbacks.server_call(&request, rc_client_identify_changed_media_callback, callback_data, client);

rc_api_destroy_request(&request);

return &callback_data->async_handle;
/* if handle is no longer valid, the async operation completed synchronously */
return rc_client_async_handle_valid(client, async_handle) ? async_handle : NULL;
}
}

Expand Down Expand Up @@ -3894,6 +3929,7 @@ static rc_client_async_handle_t* rc_client_begin_fetch_leaderboard_info(rc_clien
rc_client_fetch_leaderboard_entries_callback_t callback, void* callback_userdata)
{
rc_client_fetch_leaderboard_entries_callback_data_t* callback_data;
rc_client_async_handle_t* async_handle;
rc_api_request_t request;
int result;
const char* error_message;
Expand All @@ -3917,11 +3953,12 @@ static rc_client_async_handle_t* rc_client_begin_fetch_leaderboard_info(rc_clien
callback_data->callback_userdata = callback_userdata;
callback_data->leaderboard_id = lbinfo_request->leaderboard_id;

rc_client_begin_async(client, &callback_data->async_handle);
async_handle = &callback_data->async_handle;
rc_client_begin_async(client, async_handle);
client->callbacks.server_call(&request, rc_client_fetch_leaderboard_entries_callback, callback_data, client);
rc_api_destroy_request(&request);

return &callback_data->async_handle;
return rc_client_async_handle_valid(client, async_handle) ? async_handle : NULL;
}

rc_client_async_handle_t* rc_client_begin_fetch_leaderboard_entries(rc_client_t* client, uint32_t leaderboard_id,
Expand Down
117 changes: 105 additions & 12 deletions test/test_rc_client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,22 @@ static void rc_client_callback_expect_success(int result, const char* error_mess
ASSERT_PTR_EQUALS(callback_userdata, g_callback_userdata);
}

static void rc_client_callback_expect_timeout(int result, const char* error_message, rc_client_t* client, void* callback_userdata)
{
ASSERT_NUM_EQUALS(result, RC_NO_RESPONSE);
ASSERT_STR_EQUALS(error_message, "Request has timed out.");
ASSERT_PTR_EQUALS(client, g_client);
ASSERT_PTR_EQUALS(callback_userdata, g_callback_userdata);
}

static void rc_client_callback_expect_no_internet(int result, const char* error_message, rc_client_t* client, void* callback_userdata)
{
ASSERT_NUM_EQUALS(result, RC_NO_RESPONSE);
ASSERT_STR_EQUALS(error_message, "Internet not available.");
ASSERT_PTR_EQUALS(client, g_client);
ASSERT_PTR_EQUALS(callback_userdata, g_callback_userdata);
}

static void rc_client_callback_expect_uncalled(int result, const char* error_message, rc_client_t* client, void* callback_userdata)
{
ASSERT_FAIL("Callback should not have been called.");
Expand Down Expand Up @@ -885,6 +901,26 @@ static void test_login_with_password_async_destroyed(void)
"{\"Success\":true,\"User\":\"User\",\"Token\":\"ApiToken\",\"Score\":12345,\"SoftcoreScore\":123,\"Messages\":2,\"Permissions\":1,\"AccountType\":\"Registered\"}");
}

static void test_login_with_password_client_error(void)
{
const rc_client_user_t* user;
rc_client_async_handle_t* handle;

g_client = mock_client_not_logged_in();

mock_api_error("r=login2&u=User&p=Pa%24%24word", "Internet not available.", RC_API_SERVER_RESPONSE_CLIENT_ERROR);

handle = rc_client_begin_login_with_password(g_client, "User", "Pa$$word",
rc_client_callback_expect_no_internet, g_callback_userdata);

user = rc_client_get_user_info(g_client);
ASSERT_PTR_NULL(user);

ASSERT_PTR_NULL(handle);

rc_client_destroy(g_client);
}

static void rc_client_callback_expect_login_required(int result, const char* error_message, rc_client_t* client, void* callback_userdata)
{
ASSERT_NUM_EQUALS(result, RC_LOGIN_REQUIRED);
Expand Down Expand Up @@ -1467,14 +1503,6 @@ static void test_load_game_startsession_failure(void)
rc_client_destroy(g_client);
}

static void rc_client_callback_expect_timeout(int result, const char* error_message, rc_client_t* client, void* callback_userdata)
{
ASSERT_NUM_EQUALS(result, RC_NO_RESPONSE);
ASSERT_STR_EQUALS(error_message, "Request has timed out.");
ASSERT_PTR_EQUALS(client, g_client);
ASSERT_PTR_EQUALS(callback_userdata, g_callback_userdata);
}

static void test_load_game_startsession_timeout(void)
{
g_client = mock_client_logged_in();
Expand Down Expand Up @@ -2676,7 +2704,7 @@ static void test_change_media_while_loading_later(void)
free(image);
}

static void test_change_media_aborted(void)
static void test_change_media_async_aborted(void)
{
rc_client_async_handle_t* handle;
const size_t image_size = 32768;
Expand Down Expand Up @@ -2722,6 +2750,39 @@ static void test_change_media_aborted(void)
free(image);
}

static void test_change_media_client_error(void)
{
rc_client_async_handle_t* handle;
const size_t image_size = 32768;
uint8_t* image = generate_generic_file(image_size);

g_client = mock_client_game_loaded(patchdata_2ach_1lbd, no_unlocks);
mock_api_error("r=gameid&m=6a2305a2b6675a97ff792709be1ca857", "Internet not available.", RC_API_SERVER_RESPONSE_CLIENT_ERROR);

handle = rc_client_begin_change_media(g_client, "foo.zip#foo.nes", image, image_size,
rc_client_callback_expect_no_internet, g_callback_userdata);

ASSERT_PTR_NULL(g_client->state.load);
ASSERT_PTR_NOT_NULL(g_client->game);
if (g_client->game)
{
ASSERT_PTR_EQUALS(rc_client_get_game_info(g_client), &g_client->game->public_);

ASSERT_NUM_EQUALS(g_client->game->public_.id, 1234);
ASSERT_NUM_EQUALS(g_client->game->public_.console_id, 17);
ASSERT_STR_EQUALS(g_client->game->public_.title, "Sample Game");
ASSERT_STR_EQUALS(g_client->game->public_.hash, "0123456789ABCDEF"); /* old hash retained */
ASSERT_STR_EQUALS(g_client->game->public_.badge_name, "112233");
ASSERT_NUM_EQUALS(g_client->game->subsets->public_.num_achievements, 2);
ASSERT_NUM_EQUALS(g_client->game->subsets->public_.num_leaderboards, 1);
}

ASSERT_PTR_NULL(handle);

rc_client_destroy(g_client);
free(image);
}

/* ----- get game image ----- */

static void test_game_get_image_url(void)
Expand Down Expand Up @@ -4718,7 +4779,7 @@ static void rc_client_callback_expect_leaderboard_uncalled(int result, const cha
ASSERT_FAIL("Callback should not have been called.")
}

static void test_fetch_leaderboard_entries_aborted(void)
static void test_fetch_leaderboard_entries_async_aborted(void)
{
rc_client_async_handle_t* handle;

Expand All @@ -4738,6 +4799,35 @@ static void test_fetch_leaderboard_entries_aborted(void)
rc_client_destroy(g_client);
}

static void rc_client_callback_expect_leaderboard_internet_not_available(int result, const char* error_message,
rc_client_leaderboard_entry_list_t* list, rc_client_t* client, void* callback_userdata)
{
ASSERT_NUM_EQUALS(result, RC_NO_RESPONSE);
ASSERT_STR_EQUALS(error_message, "Internet not available.");
ASSERT_PTR_EQUALS(client, g_client);
ASSERT_PTR_EQUALS(callback_userdata, g_callback_userdata);
ASSERT_PTR_NULL(list);
}

static void test_fetch_leaderboard_entries_client_error(void)
{
rc_client_async_handle_t* handle;

g_client = mock_client_game_loaded(patchdata_2ach_1lbd, no_unlocks);

g_leaderboard_entries = NULL;

mock_api_error("r=lbinfo&i=4401&c=10", "Internet not available.", RC_API_SERVER_RESPONSE_CLIENT_ERROR);

handle = rc_client_begin_fetch_leaderboard_entries(g_client, 4401, 1, 10,
rc_client_callback_expect_leaderboard_internet_not_available, g_callback_userdata);

ASSERT_PTR_NULL(handle);
ASSERT_PTR_NULL(g_leaderboard_entries);

rc_client_destroy(g_client);
}

/* ----- do frame ----- */

static void test_do_frame_bounds_check_system(void)
Expand Down Expand Up @@ -7955,6 +8045,7 @@ void test_client(void) {
TEST(test_login_with_password_async);
TEST(test_login_with_password_async_aborted);
TEST(test_login_with_password_async_destroyed);
TEST(test_login_with_password_client_error);

/* logout */
TEST(test_logout);
Expand Down Expand Up @@ -8024,7 +8115,8 @@ void test_client(void) {
TEST(test_change_media_back_and_forth);
TEST(test_change_media_while_loading);
TEST(test_change_media_while_loading_later);
TEST(test_change_media_aborted);
TEST(test_change_media_async_aborted);
TEST(test_change_media_client_error);

/* game */
TEST(test_game_get_image_url);
Expand Down Expand Up @@ -8061,7 +8153,8 @@ void test_client(void) {
TEST(test_fetch_leaderboard_entries_no_user);
TEST(test_fetch_leaderboard_entries_around_user);
TEST(test_fetch_leaderboard_entries_around_user_not_logged_in);
TEST(test_fetch_leaderboard_entries_aborted);
TEST(test_fetch_leaderboard_entries_async_aborted);
TEST(test_fetch_leaderboard_entries_client_error);

/* do frame */
TEST(test_do_frame_bounds_check_system);
Expand Down

0 comments on commit c6887b2

Please sign in to comment.