[ffumero2003]

Context

Several structures that hold credentials print them in toString(): B2AccountAuthorization (authorization token), B2FileSseForRequest (SSE-C customer key), B2WebhookConfiguration (HMAC signing secret), B2WebhookCustomHeader (header values) and B2HeadersImpl (Authorization and X-Bz-Server-Side-Encryption-Customer-Key). Applications that log these objects, or exceptions and debug output that include them, write those values in clear text.

Changes

• toString() replaces the sensitive values with [redacted] (a null value still prints as null). Non-secret fields, including the SSE-C key MD5, are unchanged. B2HeadersImpl matches header names case-insensitively.
• B2EventNotificationRule.COMPARATOR previously ordered rules by the target configuration's toString(). With the redaction, two rules that differ only in a secret would compare equal and one would be dropped from a TreeSet. Webhook targets are now compared field by field (url, custom headers, signing secret, maxEventsPerBatch). Any other target type falls back to toString() ordering, as before, and never throws.
• CHANGELOG.md: an [Unreleased] entry for both behavior changes.

No public API or serialized form changes; JSON output is unaffected.

Verification

Claim	Test
Each structure's toString() omits its secret and keeps non-secret fields	B2AccountAuthorizationTest, B2FileSseForRequestTest, B2WebhookConfigurationTest, B2CustomWebhookHeaderTest, B2HeadersImplTest
Header redaction is case-insensitive	B2HeadersImplTest
Rules differing only in a redacted value stay distinct and both survive in a TreeSet; compareTo is consistent with equals	B2EventNotificationRuleTest
Unknown target configurations are ordered without throwing	B2EventNotificationRuleTest.testCompareToOrdersUnknownTargetConfigurationsWithoutThrowing

Each fix was checked against a deliberate break: restoring the comparator's toString() ordering fails three tests; removing the header redaction fails B2HeadersImplTest; restoring an exception for unknown targets fails 1 of 702 core tests.

$ ./gradlew build
BUILD SUCCESSFUL
54 actionable tasks

[sophiecarreras]

Thanks Felipe. Review from Sofia, with Claude's help. No blocking bug found; this is a comment, not a request for changes on the code, but the CHANGELOG needs a fix.

What was verified (by running, on head ccdb191, JDK 17, offline): 7 test classes pass: B2EventNotificationRuleTest, B2HeadersImplTest, B2AccountAuthorizationTest, B2WebhookConfigurationTest, B2FileSseForRequestTest, B2CustomWebhookHeaderTest, B2AccountAuthorizationCacheTest. Not verified: that the tests fail without the fix. My revert attempt did not compile, so it proved nothing; this rests on reading the tests (they assert the secret is absent and [redacted] present). The deliberate-break results in the PR body are Felipe's, not re-run.

Please change

1. CHANGELOG is incomplete. B2EventNotificationRule.java:89 now orders and dedupes by maxEventsPerBatch. The old toString never included it, so rules differing only in batch size used to collapse in a TreeSet and no longer do. Also, existing rules are reordered because the order is no longer string-based (url, then headers, then secret). Callers who persist or diff ordered rules will see it. Please state both.

Nits

• B2EventNotificationRule.java:109-120: the instanceof check admits subclasses of B2WebhookConfiguration, while equals uses getClass(). A subclass compared to a webhook uses field comparison, to another type uses class name: transitivity breaks and compareTo can be 0 while equals is false. Use getClass() equality.
• Unknown target types fall back to Object.toString (class@hash), so they order by identity. Pre-existing, not a regression.
• B2WebhookCustomHeader.java:20-22: compareTo ignores name case, equals does not; both throw NPE on a null value though the constructor accepts null. The new header-set comparison inherits this.
• B2HeadersImpl.java:41-44: the redaction list is Authorization and the SSE-C key header only. I found no other secret header in B2Headers.

Checked by reading, no issue: B2UploadUrlResponse, B2UploadPartUrlResponse, B2DownloadAuthorization, B2CreateKeyRequest, B2ApplicationKey, B2CreatedApplicationKey have no toString; B2FileSseForResponse prints only the MD5; JSON output is unchanged; toString was only used as behavior in the old comparator.

Needs a human: other modules (httpClientCore, samples, ITs) that print or assert on these toString outputs; whether anyone relies on the old ordering or the collapse; the full :b2-sdk-core:test in CI (only 7 classes were run); other logging paths in the HTTP client that dump headers (not audited outside core).

[sophiecarreras]

Re-review of the new head adede03 (Sofia, with Claude's help). No blocking findings from this review.

Both items I asked for are fixed:

• CHANGELOG now says rules differing only in maxEventsPerBatch no longer collapse, and that the order of existing rules can change.
• The comparator uses getClass() == B2WebhookConfiguration.class, and new tests cover the TreeSet size and compareTo/equals consistency for a webhook subclass.

Run: B2EventNotificationRuleTest, B2HeadersImplTest, B2AccountAuthorizationTest, B2WebhookConfigurationTest, B2CustomWebhookHeaderTest and B2FileSseForRequestTest at adede03; they pass. Not run: the full :b2-sdk-core:test and the other modules (CI is still pending), and I did not re-verify that the tests fail without the fix.

Still open, not blockers: B2WebhookCustomHeader compareTo ignores header-name case while equals does not, and both throw NPE on a null value the constructor accepts. A human should still check other modules that print or assert on these toString outputs.

I'm leaving this as a comment, not an approval, so the approval stays with the owners.

[sophiecarreras]

Thanks for this, it looks good to me. I went through every toString() in the SDK and these five structures are the ones that print a credential: the account authorization token, the SSE-C key, the webhook signing secret and custom header values, and the Authorization and SSE-C key headers. I did not find other leaks. The redaction handles null and header-name case correctly and leaves equals, hashCode and serialization unchanged. The rule comparator is a total order consistent with equals, and it no longer collapses rules that differ only by batch size or only by a secret in sorted sets. I reverted the production changes locally and the new tests fail as expected; the full test suite and check_code pass with them applied.

One note: callers that persist or diff ordered rules may see a different order, which the CHANGELOG already mentions. Optional nit: the [redacted] constant is repeated in five classes, so a shared helper would be tidier.

[sophiecarreras]

Thanks for this, it looks good to me. I went through every toString() in the SDK and these five structures are the ones that print a credential: the account authorization token, the SSE-C key, the webhook signing secret and custom header values, and the Authorization and SSE-C key headers. I did not find other leaks. The redaction handles null and header-name case correctly and leaves equals, hashCode and serialization unchanged. The rule comparator is a total order consistent with equals, and it no longer collapses rules that differ only by batch size or only by a secret in sorted sets. I reverted the production changes locally and the new tests fail as expected; the full test suite and check_code pass with them applied.

One note: callers that persist or diff ordered rules may see a different order, which the CHANGELOG already mentions. Optional nit: the [redacted] constant is repeated in five classes, so a shared helper would be tidier.