Skip to content

Commit

Permalink
Add runtime string checks and additional unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
sirknightj committed Jan 25, 2025
1 parent d88e505 commit b5b6278
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 3 deletions.
38 changes: 35 additions & 3 deletions src/gstreamer/gstkvssink.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ GST_DEBUG_CATEGORY_STATIC (gst_kvs_sink_debug);

#define MAX_GSTREAMER_MEDIA_TYPE_LEN 16

#define INTERNAL_CHECK_PREFIX "Assertion failed:"

namespace KvsSinkSignals {
guint err_signal_id;
guint ack_signal_id;
Expand Down Expand Up @@ -261,6 +263,11 @@ void closed(UINT64 custom_data, STREAM_HANDLE stream_handle, UPLOAD_HANDLE uploa
void kinesis_video_producer_init(GstKvsSink *kvssink)
{
auto data = kvssink->data;

if (kvssink->stream_name == NULL) {
throw runtime_error(string(INTERNAL_CHECK_PREFIX) + " kvssink->stream_name is unexpectedly NULL!");
}

unique_ptr<DeviceInfoProvider> device_info_provider(new KvsSinkDeviceInfoProvider(kvssink->storage_size,
kvssink->stop_stream_timeout,
kvssink->service_connection_timeout,
Expand Down Expand Up @@ -302,11 +309,11 @@ void kinesis_video_producer_init(GstKvsSink *kvssink)
}

} else {
access_key_str = string(kvssink->access_key);
secret_key_str = string(kvssink->secret_key);
access_key_str = kvssink->access_key ? string(kvssink->access_key) : "";
secret_key_str = kvssink->secret_key ? string(kvssink->secret_key) : "";
}

// Handle session token seperately, since this is optional with long term credentials
// Handle session token separately, since this is optional with long term credentials
if (0 == strcmp(kvssink->session_token, DEFAULT_SESSION_TOKEN)) {
session_token_str = "";
if (nullptr != (session_token = getenv(SESSION_TOKEN_ENV_VAR))) {
Expand Down Expand Up @@ -370,6 +377,11 @@ void kinesis_video_producer_init(GstKvsSink *kvssink)
LOG_INFO("Getting URL from env for " << kvssink->stream_name);
control_plane_uri_str = string(control_plane_uri);
}

if (kvssink->user_agent == NULL) {
throw runtime_error(string(INTERNAL_CHECK_PREFIX) + " kvssink->user_agent is unexpectedly NULL!");
}

LOG_INFO("User agent string: " << kvssink->user_agent);
data->kinesis_video_producer = KinesisVideoProducer::createSync(std::move(device_info_provider),
std::move(client_callback_provider),
Expand Down Expand Up @@ -423,6 +435,13 @@ void create_kinesis_video_stream(GstKvsSink *kvssink) {
break;
}

// StreamDefinition takes in C++ strings, check the gchar* for nullptr before constructing
if (kvssink->stream_name == NULL || kvssink->content_type == NULL ||
kvssink->user_agent == NULL || kvssink->kms_key_id == NULL ||
kvssink->codec_id == NULL || kvssink->track_name == NULL) {
throw runtime_error(string(INTERNAL_CHECK_PREFIX) + " A string was unexpectedly NULL!");
}

unique_ptr<StreamDefinition> stream_definition(new StreamDefinition(kvssink->stream_name,
hours(kvssink->retention_period_hours),
p_stream_tags,
Expand Down Expand Up @@ -476,6 +495,15 @@ bool kinesis_video_stream_init(GstKvsSink *kvssink, string &err_msg) {
create_kinesis_video_stream(kvssink);
break;
} catch (runtime_error &err) {

// Don't retry if the error is an internal error
if (STRNCMP(INTERNAL_CHECK_PREFIX, err.what(), STRLEN(INTERNAL_CHECK_PREFIX)) == 0) {
ostringstream oss;
oss << "Failed to create stream. Error: " << err.what();
err_msg = oss.str();
return false;
}

if (--retry_count == 0) {
ostringstream oss;
oss << "Failed to create stream. Error: " << err.what();
Expand Down Expand Up @@ -1569,6 +1597,10 @@ init_track_data(GstKvsSink *kvssink) {

g_free(video_content_type);
g_free(audio_content_type);

if (kvssink->content_type == NULL) {
throw runtime_error(string(INTERNAL_CHECK_PREFIX) + " kvssink->content_type is unexpectedly NULL!");
}
}

static GstStateChangeReturn
Expand Down
44 changes: 44 additions & 0 deletions tst/gstreamer/gstkvstest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,48 @@ GST_START_TEST(kvsproducersinkteststop)
}
GST_END_TEST;

GST_START_TEST(check_stream_name_null_handled)
{
GstElement *kinesisvideoproducersink;

// Setup
kinesisvideoproducersink = gst_check_setup_element ("kvssink");
fail_unless(kinesisvideoproducersink != nullptr, "Failed to create kvssink element");

g_object_set(G_OBJECT (kinesisvideoproducersink),
"stream-name", nullptr,
NULL);

// Test - Initialization of the client & stream occurs here
fail_unless_equals_int(GST_STATE_CHANGE_FAILURE, gst_element_set_state(kinesisvideoproducersink, GST_STATE_PLAYING));

// Teardown
fail_unless_equals_int(GST_STATE_CHANGE_SUCCESS, gst_element_set_state(kinesisvideoproducersink, GST_STATE_NULL));
gst_check_teardown_element(kinesisvideoproducersink);
}
GST_END_TEST;

GST_START_TEST(check_no_pads_content_type_set_correctly)
{
GstElement *kinesisvideoproducersink;

// Setup
kinesisvideoproducersink = gst_check_setup_element ("kvssink");
fail_unless(kinesisvideoproducersink != nullptr, "Failed to create kvssink element");

g_object_set(G_OBJECT (kinesisvideoproducersink),
"stream-name", "test-stream",
NULL);

// Test - Initialization of the client & stream occurs here
fail_unless_equals_int(GST_STATE_CHANGE_FAILURE, gst_element_set_state(kinesisvideoproducersink, GST_STATE_PLAYING));

// Teardown
fail_unless_equals_int(GST_STATE_CHANGE_SUCCESS, gst_element_set_state(kinesisvideoproducersink, GST_STATE_NULL));
gst_check_teardown_element(kinesisvideoproducersink);
}
GST_END_TEST;

GST_START_TEST(check_properties_are_passed_correctly)
{
GstElement *pElement =
Expand Down Expand Up @@ -266,6 +308,8 @@ Suite *gst_kinesisvideoproducer_suite(void) {
return s;
}

tcase_add_test(tc, check_stream_name_null_handled);
tcase_add_test(tc, check_no_pads_content_type_set_correctly);
tcase_add_test(tc, kvsproducersinktestplayandstop);
tcase_add_test(tc, kvsproducersinktestplaytopausetoplay);
tcase_add_test(tc, kvsproducersinkteststop);
Expand Down

0 comments on commit b5b6278

Please sign in to comment.