Skip to content

fix: bound verified certificate cache size - #304

Open
visheshupadhyay wants to merge 2 commits into
apple:mainfrom
visheshupadhyay:fix/bound-verified-certificate-cache
Open

visheshupadhyay wants to merge 2 commits into
apple:mainfrom
visheshupadhyay:fix/bound-verified-certificate-cache

Conversation

@visheshupadhyay

Copy link
Copy Markdown

Fixes #303

@dxnnystyles dxnnystyles left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a small eviction edge case here. when the cache is already at capacity, refreshing an existing certificate chain still removes another entry before put replaces the existing key. checking whether the chain is already present before evicting would preserve that entry. it'd also be worth covering this case in the cache tests.

@visheshupadhyay

Copy link
Copy Markdown
Author

I’ve updated the cache insertion logic to skip eviction when the incoming certificate chain is already present, so a refresh replaces only that existing entry.

I also added regression coverage that fills the cache to capacity, refreshes an existing chain, and verifies that the cache remains at 32 entries without removing any unrelated chain. The full test suite passes.

@dxnnystyles dxnnystyles left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the update! checked the revised eviction logic + regression test. refreshing an existing chain now leaves the other cached entries intact, and new insertions still respect the 32-entry limit. looks good 👍

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]:Bound the verified certificate cache when all entries are fresh

2 participants