Skip to content

Commit 89e5557

Browse files
committed
Robustify script name checking and reporting
1 parent d38ece8 commit 89e5557

2 files changed

Lines changed: 33 additions & 19 deletions

File tree

include/ur_client_library/primary/primary_client.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,7 @@ class PrimaryClient
351351

352352
ScriptInfo prepare_script(std::string script, std::string script_name);
353353
std::vector<std::string> strip_comments_and_whitespace(std::vector<std::string> script_lines);
354+
std::string truncate_and_check_script_name(std::string candidate_name);
354355

355356
PrimaryParser parser_;
356357
std::shared_ptr<PrimaryConsumer> consumer_;

src/primary/primary_client.cpp

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,26 @@ std::vector<std::string> PrimaryClient::strip_comments_and_whitespace(std::vecto
340340
return stripped_script;
341341
}
342342

343+
std::string PrimaryClient::truncate_and_check_script_name(const std::string candidate_name)
344+
{
345+
std::string final_name = candidate_name;
346+
// Limit script name length to 31, to ensure backwards compatibility
347+
if (final_name.size() > 31)
348+
{
349+
final_name = final_name.substr(0, 31);
350+
URCL_LOG_WARN("Given script name was too long, and has been truncated. New script name is: %s", final_name.c_str());
351+
}
352+
// Validate script_name
353+
static const std::regex valid_name(R"(^[A-Za-z_][A-Za-z0-9_]*$)");
354+
if (!std::regex_match(final_name, valid_name))
355+
{
356+
throw urcl::ScriptCodeSyntaxException("Invalid script name: '" + final_name +
357+
"'. Can only contain letters, numbers and underscores. First character "
358+
"must be a letter or underscore.");
359+
}
360+
return final_name;
361+
}
362+
343363
ScriptInfo PrimaryClient::prepare_script(std::string script, std::string script_name)
344364
{
345365
// Split the given script in to separate lines
@@ -357,7 +377,12 @@ ScriptInfo PrimaryClient::prepare_script(std::string script, std::string script_
357377
int64_t current_time =
358378
std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch())
359379
.count();
380+
// Assign name according to inputs
360381
std::string actual_script_name = script_name.empty() ? "script_" + std::to_string(current_time) : script_name;
382+
383+
// Check that the final name is valid
384+
actual_script_name = truncate_and_check_script_name(actual_script_name);
385+
361386
ScriptTypes actual_script_type = urcl::primary_interface::ScriptTypes::DEF;
362387
// Is the script wrapped in a function definition? If not add one
363388
if (stripped_script[0].substr(0, 4).find("def ") == script.npos &&
@@ -378,7 +403,7 @@ ScriptInfo PrimaryClient::prepare_script(std::string script, std::string script_
378403
else
379404
{
380405
size_t name_end = stripped_script[0].find("(");
381-
actual_script_name = stripped_script[0].substr(4, name_end - 4);
406+
std::string name_in_script = stripped_script[0].substr(4, name_end - 4);
382407
if (stripped_script[0].find("def") != stripped_script[0].npos)
383408
{
384409
actual_script_type = ScriptTypes::DEF;
@@ -387,26 +412,14 @@ ScriptInfo PrimaryClient::prepare_script(std::string script, std::string script_
387412
{
388413
actual_script_type = ScriptTypes::SEC;
389414
}
415+
// Check that the script name is valid, replace it if it is not
416+
actual_script_name = truncate_and_check_script_name(name_in_script);
417+
if (actual_script_name.size() != name_in_script.size())
418+
{
419+
stripped_script[0].replace(stripped_script[0].find(name_in_script), name_in_script.size(), actual_script_name);
420+
}
390421
}
391422

392-
// Validate script_name
393-
static const std::regex valid_name(R"(^[A-Za-z_][A-Za-z0-9_]*$)");
394-
if (!std::regex_match(actual_script_name, valid_name))
395-
{
396-
throw urcl::ScriptCodeSyntaxException("Invalid script name: '" + script_name +
397-
"'. Can only contain letters, numbers and underscores. First character "
398-
"must "
399-
"be a letter or "
400-
"underscore.");
401-
}
402-
403-
// Limit script name length to 31, to ensure backwards compatibility
404-
if (actual_script_name.size() > 31)
405-
{
406-
actual_script_name = actual_script_name.substr(0, 31);
407-
URCL_LOG_WARN("Given script name was too long, and has been truncated. New script name is: %s",
408-
actual_script_name.c_str());
409-
}
410423
if (stripped_script.back().find("end") == script.npos)
411424
{
412425
throw urcl::ScriptCodeSyntaxException("Script contains either function definition or secondary process "

0 commit comments

Comments
 (0)