pvillard31 commented on code in PR #11249:
URL: https://github.com/apache/nifi/pull/11249#discussion_r3242765564
##########
nifi-extension-bundles/nifi-snmp-bundle/nifi-snmp-processors/src/main/java/org/apache/nifi/snmp/utils/UsmUserDeserializer.java:
##########
@@ -73,12 +73,19 @@ public UsmUser deserialize(JsonParser jp,
DeserializationContext ctxt) throws IO
"authentication protocol is specified.");
}
+ OctetString localizationEngineID = null;
+ final JsonNode localizationEngineIDNode =
node.get("localizationEngineID");
+ if (localizationEngineIDNode != null) {
+ localizationEngineID = new
OctetString(localizationEngineIDNode.asText());
Review Comment:
Should `localizationEngineID` be hex-decoded via
`OctetString.fromHexString(...)` rather than wrapped with `new
OctetString(...asText())`, given that SNMP engineIDs are conventionally
exchanged as hex strings and `new OctetString("8000000001020304")` yields 16
ASCII bytes instead of the 8-byte engineID `0x80 0x00 0x00 0x00 0x01 0x02 0x03
0x04` that the authoritative engine actually advertises?
##########
nifi-extension-bundles/nifi-snmp-bundle/nifi-snmp-processors/src/test/resources/usm_users.json:
##########
@@ -4,13 +4,15 @@
"authProtocol":"HMAC384SHA512",
"authPassphrase":"abc12345",
"privProtocol":"AES192",
- "privPassphrase":"abc12345"
+ "privPassphrase":"abc12345",
+ "localizationEngineID":"8000000001020304"
Review Comment:
Could the JSON example in
`nifi-snmp-processors/.../ListenTrapSNMP/additionalDetails.md` be extended to
include the optional `localizationEngineID` field (with its expected format),
since both USM JSON properties' descriptions point users there for the file
format?
##########
nifi-extension-bundles/nifi-snmp-bundle/nifi-snmp-processors/src/test/java/org/apache/nifi/snmp/utils/JsonUsmReaderTestBase.java:
##########
@@ -39,14 +39,17 @@ public class JsonUsmReaderTestBase {
new OID("1.3.6.1.6.3.10.1.1.7"),
new OctetString("abc12345"),
new OID("1.3.6.1.4.1.4976.2.2.1.1.1"),
- new OctetString("abc12345")
+ new OctetString("abc12345"),
+ new OctetString("8000000001020304")
Review Comment:
Could one of the fixture users be left without a `localizationEngineID` in
both `usm_users.json` and `expectedUsmUsers`, so the test locks in the contract
that the field is optional and that an absent value produces a `UsmUser` with
`null` localizationEngineID?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
