Issues/91 change certstore caching 2#98
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the updated CertStore caching semantics requested in #91 so that a successful server update fully replaces the locally cached certificate set (removing certificates that no longer appear in the server response).
Changes:
- Change CertStore update logic to rebuild cached certificates solely from the latest server response (instead of merging with previously cached entries).
- Add targeted update tests covering cache replacement, expired-certificate filtering, and response de-duplication.
- Adjust configuration fallback-certificate behavior/tests (including removal of deprecated
fallbackCertificateAPI).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| library/src/main/java/com/wultra/android/sslpinning/CertStore.kt | Updates cache-update logic to replace cached certificates with server-provided set and keeps filtering/dedup behavior. |
| library/src/test/java/com/wultra/android/sslpinning/CertStoreUpdateTest.kt | Adds regression tests validating the new replace-cache behavior and related response handling. |
| library/src/main/java/com/wultra/android/sslpinning/CertStoreConfiguration.kt | Changes fallback certificate defaulting behavior and removes deprecated fallbackCertificate API. |
| library/src/test/java/com/wultra/android/sslpinning/CertStoreConfigurationTest.java | Updates expectations for fallback certificates being an empty array rather than null. |
Comments suppressed due to low confidence (1)
library/src/main/java/com/wultra/android/sslpinning/CertStoreConfiguration.kt:209
- The deprecated
fallbackCertificateproperty + builder method were removed even though the project version is still 1.x (library/gradle.properties currently declares 1.6.0-SNAPSHOT). This is a source/binary breaking change for any consumers still using the deprecated API; the file itself previously indicated removal should happen in 2.0. Consider keeping the deprecated API as a shim that forwards tofallbackCertificates(or bump the major version / document the breaking change explicitly).
@Suppress("DeprecatedCallableAddReplaceWith", "unused")
class Builder(
val serviceUrl: URL,
val publicKey: ByteArray
) {
var expectedCommonNames: Array<String>? = null
private set
var identifier: String? = null
private set
var fallbackCertificates: Array<GetFingerprintResponse.Entry>? = null
private set
var periodicUpdateIntervalMillis: Long = TimeUnit.DAYS.toMillis(7)
private set
var expirationUpdateThresholdMillis: Long = TimeUnit.DAYS.toMillis(14)
private set
var executorService: ExecutorService? = null
private set
var sslValidationStrategy: SslValidationStrategy? = null
private set
/**
* Set expected common names.
*/
fun expectedCommonNames(expectedCommonNames: Array<String>?) = apply {
this.expectedCommonNames = expectedCommonNames
}
/**
* Set identifier.
*
* Necessary for multiple instances of [CertStore].
* If not set the identifier is "default".
*/
fun identifier(identifier: String?) = apply {
this.identifier = identifier
}
/**
* Fallback certificate fingerprints.
* Useful for situations when no fingerprints have been loaded from the server yet.
* Only certificate fingerprints are accepted here; domain bypass configuration
* (DomainsConfig) is not supported in fallback data and is intentionally ignored.
*/
fun fallbackCertificates(fallbackCertificates: Array<GetFingerprintResponse.Entry>?) = apply {
this.fallbackCertificates = fallbackCertificates
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
972e61f to
6dac9df
Compare
6dac9df to
4bfaae2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #91