I agree with Kevin – the API’s name should have been consistent with the HLD document, not the PermissionPolicy class’ method names.
Also, I would agree with Vincent to change the XML name “policyVersion” to “version”. I don’t know why I’ve started with “policyVersion” in the first place. This fix should be possible (and easy) to implement without touching the APIs, but we would end up having to support both XML element names. I can do that, if you think it’s worth it. Thanks, Pawel From: Kevin Kane Sent: Monday, November 21, 2016 14:31 To: Vincent Du <v...@affinegy.com>; Josh Spain <jsp...@affinegy.com>; Pawel Winogrodzki <pawe...@microsoft.com> Cc: allseen-c...@lists.allseenalliance.org Subject: RE: [Alljoyn-core] Remaining 16.10 Blocking Issue (ASACORE-3483) +Pawel I agree, this is confusing. I would prefer to use “serial number” throughout to refer to this field. So if and when a change is made, I suggest changing the name of the API in SecurityApplicationProxy. Changing the name is probably too big a change for right now, although we could consider taking a doc comment only change for 16.10 to clarify its behavior without incurring regression risk at this late date, and note that the name is poorly chosen. We can then introduce a better-named API in 17.04 and deprecate the old. From: alljoyn-core-boun...@lists.alljoyn.org<mailto:alljoyn-core-boun...@lists.alljoyn.org> [mailto:alljoyn-core-boun...@lists.alljoyn.org] On Behalf Of Vincent Du Sent: Monday, November 21, 2016 1:31 PM To: Josh Spain <jsp...@affinegy.com<mailto:jsp...@affinegy.com>> Cc: allseen-c...@lists.allseenalliance.org<mailto:allseen-c...@lists.allseenalliance.org> Subject: Re: [Alljoyn-core] Remaining 16.10 Blocking Issue (ASACORE-3483) With some debugging I think the issue I have seen is more of a naming one. Basically there are two attributes for PermissionPolicy: 1. Specification version, which is always hardcoded to "1" for now 2. Serial Number, which could be specified in XML string Internally, alljoyn_core considers a "version" of a policy to be the Serial Number attribute, instead of Specification version, here are some proof: 1. the "GetPolicyVersion" function in the "alljoyn_core/src/SecurityApplicationProxy.cc" always returns the "Serial Number" field of a policy; 2. Check the "alljoyn_core/src/SecurityApplicationProxy.cc" 95 void XmlPoliciesConverter::BuildPolicy(const XmlElement* root, ajn::PermissionPolicy& policy) 96 { 97 SetPolicyVersion(root->GetChildren()[POLICY_VERSION_INDEX], policy); 98 SetPolicySerialNumber(root->GetChildren()[SERIAL_NUMBER_INDEX], policy); 99 SetPolicyAcls(root->GetChildren()[ACLS_INDEX], policy); 100 } 101 102 void XmlPoliciesConverter::SetPolicyVersion(const XmlElement* xmlPolicyVersion, PermissionPolicy& policy) 103 { 104 uint32_t policyVersion = StringToU32(xmlPolicyVersion->GetContent()); 105 policy.SetSpecificationVersion(policyVersion); 106 } 107 108 void XmlPoliciesConverter::SetPolicySerialNumber(const XmlElement* xmlSerialNumber, PermissionPolicy& policy) 109 { 110 uint32_t serialNumber = StringToU32(xmlSerialNumber->GetContent()); 111 policy.SetVersion(serialNumber); 112 } Notice that "SetPolicyVersion" internally calls the "SetSpecificationVersion", but "SetPolicySerialNumber" calls the "SetVersion". However a typical policy XML string looks like this: 103 static AJ_PCSTR s_validNewerPolicy = 104 "<policy>" 105 "<policyVersion>1</policyVersion>" 106 "<serialNumber>200</serialNumber>" 107 "<acls>" 108 "<acl>" 109 "<peers>" 110 "<peer>" 111 "<type>ALL</type>" 112 "</peer>" 113 "</peers>" 114 VALID_ALLOW_ALL_RULES 115 "</acl>" 116 "</acls>" 117 "</policy>"; And here is our documentation : https://allseenalliance.org/framework/documentation/learn/core/security2_0/hld<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fallseenalliance.org%2Fframework%2Fdocumentation%2Flearn%2Fcore%2Fsecurity2_0%2Fhld&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856896188&sdata=AltzbzFUOY8LXJlATQo35tv0lQo%2FDLqE0dvWLj%2Fi4Q0%3D&reserved=0> Name Data type Required Description version number yes The specification version number. The current spec version number is 1. serialNumber number yes The serial number of the policy. The serial number is used to detect of an update to an older policy. ACLs Array of ACLs yes List of access control lists. I think using "policyVersion" tag is some what confusing to new users -- he/she would think GetPolicyVersion returns "1", instead of "200" in this case. I don't think this is a blocking issue. I will file an enhancement bug for 17.04 if I can get a consensus in fixing it. Vincent On Mon, Nov 21, 2016 at 1:18 PM, Josh Spain <jsp...@affinegy.com<mailto:jsp...@affinegy.com>> wrote: +Vincent to make sure he's on the email. [https://s3.amazonaws.com/ucwebapp.wisestamp.com/12898986-6b24-4785-b392-dfd3cd5cdd09/Affinegylogo201450px.format_png.resize_200x.png] Josh Spain Director of Engineering Affinegy, Inc. t.512-535-1700 x1006<tel:512-535-1700%20x1006> a. 1705 S Capital of Texas Hwy, Suite 310, Austin, TX 78746 USA Website<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Faffinegy.com&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856896188&sdata=NDx0oJHYOm2El031W23Jkgqc%2Fa%2BWNL6WBeyqX1c%2FenQ%3D&reserved=0> Email<mailto:jsp...@affinegy.com> [https://s3.amazonaws.com/images.wisestamp.com/icons_32/linkedin.png]<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.linkedin.com%2Fin%2Fjoshspain&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856896188&sdata=RQQr12sDah92l4zxX25yjIDxoSvEub%2BV6DJFlLq0ork%3D&reserved=0> [https://s3.amazonaws.com/images.wisestamp.com/icons_32/twitter.png] <https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitter.com%2Faffinegy&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856896188&sdata=33Bvw9tGXYmbbWIaev4gXGdrI%2BUKuz69rzwHColemXM%3D&reserved=0> [https://s3.amazonaws.com/images.wisestamp.com/icons/twitter.png]Latest Tweet:<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftwitter.com%2FAffinegy&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856896188&sdata=WOZwU8A39R9XGdT71Nypgdpl55zmANqx4iR47Tov7EI%3D&reserved=0> Mighty Micro-Engines of IoT - https://t.co/ambxjzUKYE<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ft.co%2FambxjzUKYE&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856896188&sdata=q%2B6JWaUFNCNzOlRXx%2FpdPCf9eHi3LoKvMshnTCt8XCE%3D&reserved=0> Read More<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftwitter.com%2FAffinegy%2Fstatuses%2F799973507090108417&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856896188&sdata=BvTMrNiOv8G%2F6CXGGFBfgnxCpz90lq6hZOTaqwKjszE%3D&reserved=0> [https://s3.amazonaws.com/images.wisestamp.com/email-apps/twitter_button/twitter-white.png]<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftwitter.com%2FAffinegy&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856906201&sdata=3UgmWU3mIu5xwkRPNgB37bgeXS%2Fx65Q2J3e4ZFHKg3Y%3D&reserved=0> On Mon, Nov 21, 2016 at 1:09 PM, Josh Spain <jsp...@affinegy.com<mailto:jsp...@affinegy.com>> wrote: Marcello, When we went through the triage issues we realized that the problem we had been discussing was already logged and fixed by Olga. It has already been merged into the repository by Way and is awaiting testing by Tyler. There is another potentially blocking Security 2.0 issue that has been found by Vincent. It appears to be in the XML processing, but he is investigating and will be logging an issue once he gathers enough information. Vincent or I will send out more information when it is available. Thanks, Josh [https://s3.amazonaws.com/ucwebapp.wisestamp.com/12898986-6b24-4785-b392-dfd3cd5cdd09/Affinegylogo201450px.format_png.resize_200x.png] Josh Spain Director of Engineering Affinegy, Inc. t.512-535-1700 x1006<tel:512-535-1700%20x1006> a. 1705 S Capital of Texas Hwy, Suite 310, Austin, TX 78746 USA Website<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Faffinegy.com&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856906201&sdata=W1NwdRjJX%2FT7ZCqr5uRVzs7vncoCz%2Fb5WrPQrmGoT%2B4%3D&reserved=0> Email<mailto:jsp...@affinegy.com> [https://s3.amazonaws.com/images.wisestamp.com/icons_32/linkedin.png]<https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.linkedin.com%2Fin%2Fjoshspain&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856906201&sdata=YlQXlMtzPQyXBBRIVd2UPebffWjN4q3dTn3p7%2BGDea8%3D&reserved=0> [https://s3.amazonaws.com/images.wisestamp.com/icons_32/twitter.png] <https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitter.com%2Faffinegy&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856906201&sdata=hH0LEi9Ko9ttXrHFqgQFZUe23eLLOkSk8O3ogYgqzfU%3D&reserved=0> [https://s3.amazonaws.com/images.wisestamp.com/icons/twitter.png]Latest Tweet:<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftwitter.com%2FAffinegy&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856906201&sdata=3UgmWU3mIu5xwkRPNgB37bgeXS%2Fx65Q2J3e4ZFHKg3Y%3D&reserved=0> Mighty Micro-Engines of IoT - https://t.co/ambxjzUKYE<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ft.co%2FambxjzUKYE&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856906201&sdata=NbbGYM00A5xEbKILlfBM27Mm7pcynUUeten%2ByNQo%2BVs%3D&reserved=0> Read More<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftwitter.com%2FAffinegy%2Fstatuses%2F799973507090108417&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856906201&sdata=jDYWrKNMainFwAWvsfvrisEKqHBfFGHsXvvZWHzlQAs%3D&reserved=0> [https://s3.amazonaws.com/images.wisestamp.com/email-apps/twitter_button/twitter-white.png]<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftwitter.com%2FAffinegy&data=02%7C01%7Ckkane%40microsoft.com%7C5f01470154ae423ddb4f08d4125adc1c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636153628856906201&sdata=3UgmWU3mIu5xwkRPNgB37bgeXS%2Fx65Q2J3e4ZFHKg3Y%3D&reserved=0>
_______________________________________________ Alljoyn-core mailing list Alljoyn-core@lists.alljoyn.org https://lists.alljoyn.org/mailman/listinfo/alljoyn-core