Skip to content

Commit

Permalink
Improve channel identifiers
Browse files Browse the repository at this point in the history 
1. Don't URL-encode control/message channel identifiers. The valid character sets for identifiers varies by platform, and on no platform does the valid character set conform to the RFCs for URL encoding. We'll let the underlying platform technologies (used by nanomsg) validate the identifier.

2. Don't add a ".ipc" suffix to the channel identifiers. On Posix-based platforms the identifier is actually a file path. If we change the file path in any way then it won't be where the user might expect it.
  • Loading branch information
@damonbarry
damonbarry committed Apr 11, 2017
1 parent 04e6623 commit bcffca6
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 31 deletions.
21 changes: 7 additions & 14 deletions core/tests/outprocess_loader_ut/outprocess_loader_ut.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,6 @@ MOCK_FUNCTION_WITH_CODE(, MODULE_API*, Fake_GetAPI, MODULE_API_VERSION, gateway_
MODULE_API* val = (MODULE_API*)0x42;
MOCK_FUNCTION_END(val)

STRING_HANDLE myURL_EncodeString(const char* textEncode)
{
STRING_HANDLE val = real_STRING_construct(textEncode);
return val;
}

//parson mocks

MOCK_FUNCTION_WITH_CODE(, JSON_Object*, json_value_get_object, const JSON_Value*, value)
Expand Down Expand Up @@ -197,7 +191,6 @@ TEST_SUITE_INITIALIZE(TestClassInitialize)
REGISTER_GLOBAL_MOCK_HOOK(STRING_clone, real_STRING_clone);
REGISTER_GLOBAL_MOCK_HOOK(STRING_delete, real_STRING_delete);
REGISTER_GLOBAL_MOCK_HOOK(STRING_c_str, real_STRING_c_str);
REGISTER_GLOBAL_MOCK_HOOK(URL_EncodeString, myURL_EncodeString);

REGISTER_GLOBAL_MOCK_FAIL_RETURN(STRING_clone, NULL);

Expand Down Expand Up @@ -627,7 +620,7 @@ TEST_FUNCTION(OutprocessModuleLoader_ParseEntrypointFromJson_returns_NULL_when_u
STRICT_EXPECTED_CALL(gballoc_malloc(sizeof(OUTPROCESS_LOADER_ENTRYPOINT)));
STRICT_EXPECTED_CALL(json_object_get_number((JSON_Object*)0x43, "timeout"))
.SetReturn(0);
STRICT_EXPECTED_CALL(URL_EncodeString(control_id))
STRICT_EXPECTED_CALL(STRING_construct(control_id))
.SetReturn(NULL);
STRICT_EXPECTED_CALL(gballoc_free(IGNORED_PTR_ARG))
.IgnoreArgument(1);
Expand Down Expand Up @@ -667,8 +660,8 @@ TEST_FUNCTION(OutprocessModuleLoader_ParseEntrypointFromJson_succeeds)
STRICT_EXPECTED_CALL(gballoc_malloc(sizeof(OUTPROCESS_LOADER_ENTRYPOINT)));
STRICT_EXPECTED_CALL(json_object_get_number((JSON_Object*)0x43, "timeout"))
.SetReturn(2000);
STRICT_EXPECTED_CALL(URL_EncodeString(control_id));
STRICT_EXPECTED_CALL(URL_EncodeString(NULL));
STRICT_EXPECTED_CALL(STRING_construct(control_id));
STRICT_EXPECTED_CALL(STRING_construct(NULL));

// act
void* result = OutprocessModuleLoader_ParseEntrypointFromJson(NULL, (JSON_Value*)0x42);
Expand Down Expand Up @@ -708,8 +701,8 @@ TEST_FUNCTION(OutprocessModuleLoader_FreeEntrypoint_frees_resources)
STRICT_EXPECTED_CALL(json_object_get_string((JSON_Object*)0x43, "message.id"))
.SetReturn(message_id);
STRICT_EXPECTED_CALL(gballoc_malloc(sizeof(OUTPROCESS_LOADER_ENTRYPOINT)));
STRICT_EXPECTED_CALL(URL_EncodeString(control_id));
STRICT_EXPECTED_CALL(URL_EncodeString(message_id));
STRICT_EXPECTED_CALL(STRING_construct(control_id));
STRICT_EXPECTED_CALL(STRING_construct(message_id));

void* entrypoint = OutprocessModuleLoader_ParseEntrypointFromJson(NULL, (JSON_Value*)0x42);
ASSERT_IS_NOT_NULL(entrypoint);
Expand Down Expand Up @@ -805,8 +798,8 @@ TEST_FUNCTION(OutprocessModuleLoader_BuildModuleConfiguration_success_with_msg_u
//assert
ASSERT_IS_NOT_NULL(result);
ASSERT_ARE_EQUAL(char_ptr, umock_c_get_expected_calls(), umock_c_get_actual_calls());
ASSERT_ARE_EQUAL(char_ptr, STRING_c_str(omc->control_uri), "ipc://control_id.ipc");
ASSERT_ARE_EQUAL(char_ptr, STRING_c_str(omc->message_uri), "ipc://message_id.ipc");
ASSERT_ARE_EQUAL(char_ptr, STRING_c_str(omc->control_uri), "ipc://control_id");
ASSERT_ARE_EQUAL(char_ptr, STRING_c_str(omc->message_uri), "ipc://message_id");
ASSERT_ARE_EQUAL(char_ptr, STRING_c_str(omc->outprocess_module_args), STRING_c_str(mc));

//cleanup
Expand Down
2 changes: 1 addition & 1 deletion ...module/src/main/java/com/microsoft/azure/gateway/remote/CommunicationControlStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,6 @@ public RemoteMessage deserializeMessage(ByteBuffer messageBuffer, byte version)
*/
@Override
public String getEndpointUri(String identifier) {
return String.format("ipc://%s.ipc", identifier);
return String.format("ipc://%s", identifier);
}
}
6 changes: 1 addition & 5 deletions proxy/gateway/native/src/proxy_gateway.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ ProxyGateway_Attach (
/* Codes_SRS_PROXY_GATEWAY_027_008: [If memory allocation fails for the instance data, then `ProxyGateway_Attach` shall return `NULL`] */
LogError("%s: Unable to allocate memory!", __FUNCTION__);
} else {
static const size_t ENDPOINT_DECORATION_SIZE = sizeof("ipc://") - 1 + sizeof(".ipc") - 1;
static const size_t ENDPOINT_DECORATION_SIZE = sizeof("ipc://") - 1;

const size_t control_channel_uri_size = strlen(connection_id) + ENDPOINT_DECORATION_SIZE + 1;
char * control_channel_uri;
Expand All @@ -146,10 +146,6 @@ ProxyGateway_Attach (
LogError("%s: Unable to compose channel uri body!", __FUNCTION__);
free(remote_module);
remote_module = NULL;
} else if (NULL == strcat(control_channel_uri, ".ipc")) {
LogError("%s: Unable to compose channel uri suffix!", __FUNCTION__);
free(remote_module);
remote_module = NULL;
} else {
/* Codes_SRS_PROXY_GATEWAY_027_011: [`ProxyGateway_Attach` shall create a socket for the Azure IoT Gateway control channel by calling `int nn_socket(int domain, int protocol)` with `AF_SP` as the `domain` and `NN_PAIR` as the `protocol`] */
if (-1 == (remote_module->control_socket = nn_socket(AF_SP, NN_PAIR))) {
Expand Down
12 changes: 6 additions & 6 deletions proxy/gateway/native/tests/proxy_gateway_ut/proxy_gateway_ut.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,7 @@ TEST_FUNCTION(attach_SCENARIO_success)
// Arrange
static const int COMMAND_ENDPOINT = 917;
static const int COMMAND_SOCKET = 1979;
static const char CONTROL_CHANNEL_URI[] = "ipc://proxy_gateway_ut.ipc";
static const char CONTROL_CHANNEL_URI[] = "ipc://proxy_gateway_ut";
const MODULE_API_1 module_apis = {
{ MODULE_API_VERSION_1 },
mock_parseConfigurationFromJson,
Expand Down Expand Up @@ -840,7 +840,7 @@ TEST_FUNCTION(attach_SCENARIO_negative_tests)

static const int COMMAND_ENDPOINT = 917;
static const int COMMAND_SOCKET = 1979;
static const char CONTROL_CHANNEL_URI[] = "ipc://proxy_gateway_ut.ipc";
static const char CONTROL_CHANNEL_URI[] = "ipc://proxy_gateway_ut";

REMOTE_MODULE_HANDLE remote_module;

Expand Down Expand Up @@ -1958,9 +1958,9 @@ TEST_FUNCTION(connect_to_message_channel_SCENARIO_success)
{
// Arrange
static const MESSAGE_URI MESSAGE = {
sizeof("ipc://proxy_gateway_ut.ipc"),
sizeof("ipc://proxy_gateway_ut"),
NN_PAIR,
"ipc://proxy_gateway_ut.ipc"
"ipc://proxy_gateway_ut"
};

int result;
Expand Down Expand Up @@ -1992,9 +1992,9 @@ TEST_FUNCTION(connect_to_message_channel_SCENARIO_negative_tests)
ASSERT_ARE_EQUAL(int, 0, negativeTestsInitResult);

static const MESSAGE_URI MESSAGE = {
sizeof("ipc://proxy_gateway_ut.ipc"),
sizeof("ipc://proxy_gateway_ut"),
NN_PAIR,
"ipc://proxy_gateway_ut.ipc"
"ipc://proxy_gateway_ut"
};

int result;
Expand Down
10 changes: 5 additions & 5 deletions proxy/outprocess/src/module_loaders/outprocess_loader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ static void* OutprocessModuleLoader_ParseEntrypointFromJson(const struct MODULE_
/*Codes_SRS_OUTPROCESS_LOADER_17_017: [ This function shall assign the entrypoint activation_type to NONE. ] */
config->activation_type = OUTPROCESS_LOADER_ACTIVATION_NONE;
/*Codes_SRS_OUTPROCESS_LOADER_17_018: [ This function shall assign the entrypoint control_id to the string value of "control.id" in json, NULL if not present. ] */
config->control_id = URL_EncodeString(controlId);
config->control_id = STRING_construct(controlId);
if (config->control_id == NULL)
{
/*Codes_SRS_OUTPROCESS_LOADER_17_021: [ This function shall return NULL if any calls fails. ] */
Expand All @@ -206,7 +206,7 @@ static void* OutprocessModuleLoader_ParseEntrypointFromJson(const struct MODULE_
else
{
/*Codes_SRS_OUTPROCESS_LOADER_17_019: [ This function shall assign the entrypoint message_id to the string value of "message.id" in json, NULL if not present. ] */
config->message_id = URL_EncodeString(messageId);
config->message_id = STRING_construct(messageId);
/*Codes_SRS_OUTPROCESS_LOADER_17_022: [ This function shall return a valid pointer to an OUTPROCESS_LOADER_ENTRYPOINT on success. ]*/
}
}
Expand Down Expand Up @@ -294,13 +294,13 @@ static void* OutprocessModuleLoader_BuildModuleConfiguration(
else
{
/*Codes_SRS_OUTPROCESS_LOADER_17_032: [ The message uri shall be composed of "ipc://" + unique id . ]*/
fullModuleConfiguration->message_uri = STRING_construct_sprintf("%s%s%s", IPC_URI_HEAD, uuid, ".ipc");
fullModuleConfiguration->message_uri = STRING_construct_sprintf("%s%s", IPC_URI_HEAD, uuid);
}
}
else
{
/*Codes_SRS_OUTPROCESS_LOADER_17_033: [ This function shall allocate and copy each string in OUTPROCESS_LOADER_ENTRYPOINT and assign them to the corresponding fields in OUTPROCESS_MODULE_CONFIG. ]*/
fullModuleConfiguration->message_uri = STRING_construct_sprintf("%s%s%s", IPC_URI_HEAD, STRING_c_str(ep->message_id), ".ipc");
fullModuleConfiguration->message_uri = STRING_construct_sprintf("%s%s", IPC_URI_HEAD, STRING_c_str(ep->message_id));
}
if (fullModuleConfiguration->message_uri == NULL)
{
Expand All @@ -312,7 +312,7 @@ static void* OutprocessModuleLoader_BuildModuleConfiguration(
else
{
/*Codes_SRS_OUTPROCESS_LOADER_17_033: [ This function shall allocate and copy each string in OUTPROCESS_LOADER_ENTRYPOINT and assign them to the corresponding fields in OUTPROCESS_MODULE_CONFIG. ]*/
fullModuleConfiguration->control_uri = STRING_construct_sprintf("%s%s%s", IPC_URI_HEAD, STRING_c_str(ep->control_id), ".ipc");
fullModuleConfiguration->control_uri = STRING_construct_sprintf("%s%s", IPC_URI_HEAD, STRING_c_str(ep->control_id));
if (fullModuleConfiguration->control_uri == NULL)
{
/*Codes_SRS_OUTPROCESS_LOADER_17_026: [ This function shall return NULL if entrypoint, control_id, or module_configuration is NULL. ] */
Expand Down

0 comments on commit bcffca6

Please sign in to comment.