From bcffca6407fea6b8fc26d3bdd28ec8b8f0d15a8f Mon Sep 17 00:00:00 2001 From: Damon Barry Date: Tue, 11 Apr 2017 16:42:10 -0700 Subject: [PATCH] Improve channel identifiers 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. --- .../outprocess_loader_ut.c | 21 +++++++------------ .../remote/CommunicationControlStrategy.java | 2 +- proxy/gateway/native/src/proxy_gateway.c | 6 +----- .../tests/proxy_gateway_ut/proxy_gateway_ut.c | 12 +++++------ .../src/module_loaders/outprocess_loader.c | 10 ++++----- 5 files changed, 20 insertions(+), 31 deletions(-) diff --git a/core/tests/outprocess_loader_ut/outprocess_loader_ut.c b/core/tests/outprocess_loader_ut/outprocess_loader_ut.c index a11b7049..71a17807 100644 --- a/core/tests/outprocess_loader_ut/outprocess_loader_ut.c +++ b/core/tests/outprocess_loader_ut/outprocess_loader_ut.c @@ -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) @@ -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); @@ -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); @@ -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); @@ -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); @@ -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 diff --git a/proxy/gateway/java/gateway-remote-module/src/main/java/com/microsoft/azure/gateway/remote/CommunicationControlStrategy.java b/proxy/gateway/java/gateway-remote-module/src/main/java/com/microsoft/azure/gateway/remote/CommunicationControlStrategy.java index edd93ba7..6cd99c33 100644 --- a/proxy/gateway/java/gateway-remote-module/src/main/java/com/microsoft/azure/gateway/remote/CommunicationControlStrategy.java +++ b/proxy/gateway/java/gateway-remote-module/src/main/java/com/microsoft/azure/gateway/remote/CommunicationControlStrategy.java @@ -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); } } diff --git a/proxy/gateway/native/src/proxy_gateway.c b/proxy/gateway/native/src/proxy_gateway.c index 33fa65b2..eff5e119 100644 --- a/proxy/gateway/native/src/proxy_gateway.c +++ b/proxy/gateway/native/src/proxy_gateway.c @@ -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; @@ -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))) { diff --git a/proxy/gateway/native/tests/proxy_gateway_ut/proxy_gateway_ut.c b/proxy/gateway/native/tests/proxy_gateway_ut/proxy_gateway_ut.c index cfe62085..431479a5 100644 --- a/proxy/gateway/native/tests/proxy_gateway_ut/proxy_gateway_ut.c +++ b/proxy/gateway/native/tests/proxy_gateway_ut/proxy_gateway_ut.c @@ -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, @@ -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; @@ -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; @@ -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; diff --git a/proxy/outprocess/src/module_loaders/outprocess_loader.c b/proxy/outprocess/src/module_loaders/outprocess_loader.c index f4719688..31e12141 100644 --- a/proxy/outprocess/src/module_loaders/outprocess_loader.c +++ b/proxy/outprocess/src/module_loaders/outprocess_loader.c @@ -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. ] */ @@ -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. ]*/ } } @@ -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) { @@ -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. ] */