Copilot commented on code in PR #133:
URL:
https://github.com/apache/fineract-consumer-facing/pull/133#discussion_r3942561905
##########
consumer/src/main/java/org/apache/fineract/consumer/infrastructure/stepup/service/StepUpTokenService.java:
##########
@@ -46,27 +46,22 @@ public class StepUpTokenService {
private final JwtIssuer jwtIssuer;
private final JwtDecoder jwtDecoder;
- public String actionFingerprint(String endpoint, Long fromAccountId, Long
toAccountId, BigDecimal amount) {
- String canonical =
- endpoint + "|" + fromAccountId + "|" + toAccountId + "|" +
amount.stripTrailingZeros().toPlainString();
- return hash(canonical);
- }
-
- public String actionFingerprint(
- String endpoint, Long fromAccountId, Long toAccountId, String
toAccountType, BigDecimal amount) {
- String canonical = endpoint + "|" + fromAccountId + "|" + toAccountId
+ "|" + toAccountType + "|"
- + amount.stripTrailingZeros().toPlainString();
- return hash(canonical);
- }
-
- public String actionFingerprint(String endpoint, Long fromAccountId, Long
toAccountId) {
- String canonical = endpoint + "|" + fromAccountId + "|" + toAccountId;
- return hash(canonical);
+ public String actionFingerprint(String endpoint, Object... parts) {
+ StringBuilder canonical = new StringBuilder(endpoint).append('|');
+ for (int i = 0; i < parts.length; i++) {
Review Comment:
`new StringBuilder(endpoint)` will throw a NullPointerException if
`endpoint` is ever null. Previously, the canonical string was built via
concatenation and would have produced the literal "null" instead, so this is a
behavior change that contradicts the "behavior-preserving" goal.
##########
consumer/src/main/java/org/apache/fineract/consumer/infrastructure/stepup/service/StepUpTokenService.java:
##########
@@ -46,27 +46,22 @@ public class StepUpTokenService {
private final JwtIssuer jwtIssuer;
private final JwtDecoder jwtDecoder;
- public String actionFingerprint(String endpoint, Long fromAccountId, Long
toAccountId, BigDecimal amount) {
- String canonical =
- endpoint + "|" + fromAccountId + "|" + toAccountId + "|" +
amount.stripTrailingZeros().toPlainString();
- return hash(canonical);
- }
-
- public String actionFingerprint(
- String endpoint, Long fromAccountId, Long toAccountId, String
toAccountType, BigDecimal amount) {
- String canonical = endpoint + "|" + fromAccountId + "|" + toAccountId
+ "|" + toAccountType + "|"
- + amount.stripTrailingZeros().toPlainString();
- return hash(canonical);
- }
-
- public String actionFingerprint(String endpoint, Long fromAccountId, Long
toAccountId) {
- String canonical = endpoint + "|" + fromAccountId + "|" + toAccountId;
- return hash(canonical);
+ public String actionFingerprint(String endpoint, Object... parts) {
+ StringBuilder canonical = new StringBuilder(endpoint).append('|');
+ for (int i = 0; i < parts.length; i++) {
+ if (i > 0) {
+ canonical.append('|');
+ }
+ canonical.append(fingerprintPart(parts[i]));
+ }
+ return hash(canonical.toString());
}
- public String actionFingerprint(String endpoint, String... parts) {
- String canonical = endpoint + "|" + String.join("|", parts);
- return hash(canonical);
+ private String fingerprintPart(Object part) {
+ if (part instanceof BigDecimal decimal) {
+ return decimal.stripTrailingZeros().toPlainString();
+ }
+ return String.valueOf(part);
}
Review Comment:
The new `Object...` signature makes it easy for callers to pass arbitrary
objects whose `toString()` is not stable across JVM instances (e.g., default
`Object#toString` includes an identity hash). In a scaled deployment,
initiate/confirm may hit different instances, so fingerprints could become
non-deterministic unless unsupported types are rejected.
--
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]
