Skip to content

Commit b5b6278

Browse files
committed
Add runtime string checks and additional unit tests
1 parent d88e505 commit b5b6278

File tree

2 files changed

+79
-3
lines changed

2 files changed

+79
-3
lines changed

src/gstreamer/gstkvssink.cpp

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ GST_DEBUG_CATEGORY_STATIC (gst_kvs_sink_debug);
137137

138138
#define MAX_GSTREAMER_MEDIA_TYPE_LEN 16
139139

140+
#define INTERNAL_CHECK_PREFIX "Assertion failed:"
141+
140142
namespace KvsSinkSignals {
141143
guint err_signal_id;
142144
guint ack_signal_id;
@@ -261,6 +263,11 @@ void closed(UINT64 custom_data, STREAM_HANDLE stream_handle, UPLOAD_HANDLE uploa
261263
void kinesis_video_producer_init(GstKvsSink *kvssink)
262264
{
263265
auto data = kvssink->data;
266+
267+
if (kvssink->stream_name == NULL) {
268+
throw runtime_error(string(INTERNAL_CHECK_PREFIX) + " kvssink->stream_name is unexpectedly NULL!");
269+
}
270+
264271
unique_ptr<DeviceInfoProvider> device_info_provider(new KvsSinkDeviceInfoProvider(kvssink->storage_size,
265272
kvssink->stop_stream_timeout,
266273
kvssink->service_connection_timeout,
@@ -302,11 +309,11 @@ void kinesis_video_producer_init(GstKvsSink *kvssink)
302309
}
303310

304311
} else {
305-
access_key_str = string(kvssink->access_key);
306-
secret_key_str = string(kvssink->secret_key);
312+
access_key_str = kvssink->access_key ? string(kvssink->access_key) : "";
313+
secret_key_str = kvssink->secret_key ? string(kvssink->secret_key) : "";
307314
}
308315

309-
// Handle session token seperately, since this is optional with long term credentials
316+
// Handle session token separately, since this is optional with long term credentials
310317
if (0 == strcmp(kvssink->session_token, DEFAULT_SESSION_TOKEN)) {
311318
session_token_str = "";
312319
if (nullptr != (session_token = getenv(SESSION_TOKEN_ENV_VAR))) {
@@ -370,6 +377,11 @@ void kinesis_video_producer_init(GstKvsSink *kvssink)
370377
LOG_INFO("Getting URL from env for " << kvssink->stream_name);
371378
control_plane_uri_str = string(control_plane_uri);
372379
}
380+
381+
if (kvssink->user_agent == NULL) {
382+
throw runtime_error(string(INTERNAL_CHECK_PREFIX) + " kvssink->user_agent is unexpectedly NULL!");
383+
}
384+
373385
LOG_INFO("User agent string: " << kvssink->user_agent);
374386
data->kinesis_video_producer = KinesisVideoProducer::createSync(std::move(device_info_provider),
375387
std::move(client_callback_provider),
@@ -423,6 +435,13 @@ void create_kinesis_video_stream(GstKvsSink *kvssink) {
423435
break;
424436
}
425437

438+
// StreamDefinition takes in C++ strings, check the gchar* for nullptr before constructing
439+
if (kvssink->stream_name == NULL || kvssink->content_type == NULL ||
440+
kvssink->user_agent == NULL || kvssink->kms_key_id == NULL ||
441+
kvssink->codec_id == NULL || kvssink->track_name == NULL) {
442+
throw runtime_error(string(INTERNAL_CHECK_PREFIX) + " A string was unexpectedly NULL!");
443+
}
444+
426445
unique_ptr<StreamDefinition> stream_definition(new StreamDefinition(kvssink->stream_name,
427446
hours(kvssink->retention_period_hours),
428447
p_stream_tags,
@@ -476,6 +495,15 @@ bool kinesis_video_stream_init(GstKvsSink *kvssink, string &err_msg) {
476495
create_kinesis_video_stream(kvssink);
477496
break;
478497
} catch (runtime_error &err) {
498+
499+
// Don't retry if the error is an internal error
500+
if (STRNCMP(INTERNAL_CHECK_PREFIX, err.what(), STRLEN(INTERNAL_CHECK_PREFIX)) == 0) {
501+
ostringstream oss;
502+
oss << "Failed to create stream. Error: " << err.what();
503+
err_msg = oss.str();
504+
return false;
505+
}
506+
479507
if (--retry_count == 0) {
480508
ostringstream oss;
481509
oss << "Failed to create stream. Error: " << err.what();
@@ -1569,6 +1597,10 @@ init_track_data(GstKvsSink *kvssink) {
15691597

15701598
g_free(video_content_type);
15711599
g_free(audio_content_type);
1600+
1601+
if (kvssink->content_type == NULL) {
1602+
throw runtime_error(string(INTERNAL_CHECK_PREFIX) + " kvssink->content_type is unexpectedly NULL!");
1603+
}
15721604
}
15731605

15741606
static GstStateChangeReturn

tst/gstreamer/gstkvstest.cpp

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,48 @@ GST_START_TEST(kvsproducersinkteststop)
101101
}
102102
GST_END_TEST;
103103

104+
GST_START_TEST(check_stream_name_null_handled)
105+
{
106+
GstElement *kinesisvideoproducersink;
107+
108+
// Setup
109+
kinesisvideoproducersink = gst_check_setup_element ("kvssink");
110+
fail_unless(kinesisvideoproducersink != nullptr, "Failed to create kvssink element");
111+
112+
g_object_set(G_OBJECT (kinesisvideoproducersink),
113+
"stream-name", nullptr,
114+
NULL);
115+
116+
// Test - Initialization of the client & stream occurs here
117+
fail_unless_equals_int(GST_STATE_CHANGE_FAILURE, gst_element_set_state(kinesisvideoproducersink, GST_STATE_PLAYING));
118+
119+
// Teardown
120+
fail_unless_equals_int(GST_STATE_CHANGE_SUCCESS, gst_element_set_state(kinesisvideoproducersink, GST_STATE_NULL));
121+
gst_check_teardown_element(kinesisvideoproducersink);
122+
}
123+
GST_END_TEST;
124+
125+
GST_START_TEST(check_no_pads_content_type_set_correctly)
126+
{
127+
GstElement *kinesisvideoproducersink;
128+
129+
// Setup
130+
kinesisvideoproducersink = gst_check_setup_element ("kvssink");
131+
fail_unless(kinesisvideoproducersink != nullptr, "Failed to create kvssink element");
132+
133+
g_object_set(G_OBJECT (kinesisvideoproducersink),
134+
"stream-name", "test-stream",
135+
NULL);
136+
137+
// Test - Initialization of the client & stream occurs here
138+
fail_unless_equals_int(GST_STATE_CHANGE_FAILURE, gst_element_set_state(kinesisvideoproducersink, GST_STATE_PLAYING));
139+
140+
// Teardown
141+
fail_unless_equals_int(GST_STATE_CHANGE_SUCCESS, gst_element_set_state(kinesisvideoproducersink, GST_STATE_NULL));
142+
gst_check_teardown_element(kinesisvideoproducersink);
143+
}
144+
GST_END_TEST;
145+
104146
GST_START_TEST(check_properties_are_passed_correctly)
105147
{
106148
GstElement *pElement =
@@ -266,6 +308,8 @@ Suite *gst_kinesisvideoproducer_suite(void) {
266308
return s;
267309
}
268310

311+
tcase_add_test(tc, check_stream_name_null_handled);
312+
tcase_add_test(tc, check_no_pads_content_type_set_correctly);
269313
tcase_add_test(tc, kvsproducersinktestplayandstop);
270314
tcase_add_test(tc, kvsproducersinktestplaytopausetoplay);
271315
tcase_add_test(tc, kvsproducersinkteststop);

0 commit comments

Comments
 (0)