Skip to content

fix: escape all matching portions of parts of cacher cache keys#486

Merged
wschurman merged 1 commit intomainfrom
wschurman/02-27-fix_escape_all_parts_of_the_local_memory_cacher_cache_key
Mar 2, 2026
Merged

fix: escape all matching portions of parts of cacher cache keys#486
wschurman merged 1 commit intomainfrom
wschurman/02-27-fix_escape_all_parts_of_the_local_memory_cacher_cache_key

Conversation

@wschurman
Copy link
Member

@wschurman wschurman commented Feb 28, 2026

Why

One more review suggestion from claude. This fixes an incorrect encoding of the cache keys where if a key portion included a : (delimiter) it could potentially conflict with another key (though admittedly very unlikely). This happened because we were erroneously suggesting use of replace instead of replaceAll.

Note that this lives in the application code where companion provider is created, so applications will need to fix independently of this upgrade. This just fixes the suggestions.

How

The fix is to use replaceAll.

Test Plan

Run new test.

Copy link
Member Author

wschurman commented Feb 28, 2026

@wschurman wschurman changed the title fix: escape all parts of the local memory cacher cache key fix: escape all parts of cacher cache keys Feb 28, 2026
@wschurman wschurman changed the title fix: escape all parts of cacher cache keys fix: escape all matching portions of parts of cacher cache keys Feb 28, 2026
@codecov
Copy link

codecov bot commented Feb 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (4126dcd) to head (37f2c00).

Additional details and impacted files
@@            Coverage Diff             @@
##              main      #486    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          110       110            
  Lines        16394     16394            
  Branches       900      1475   +575     
==========================================
  Hits         16394     16394            
Flag Coverage Δ
integration 24.69% <50.00%> (ø)
unittest 94.85% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@wschurman wschurman changed the base branch from wschurman/02-27-fix_fix_some_security_issues to graphite-base/486 February 28, 2026 01:59
@wschurman wschurman force-pushed the wschurman/02-27-fix_escape_all_parts_of_the_local_memory_cacher_cache_key branch from 9734940 to 5608f55 Compare February 28, 2026 02:00
@graphite-app graphite-app bot changed the base branch from graphite-base/486 to main February 28, 2026 02:00
@wschurman wschurman force-pushed the wschurman/02-27-fix_escape_all_parts_of_the_local_memory_cacher_cache_key branch 2 times, most recently from 2093ef8 to 1d447e9 Compare March 2, 2026 17:27
@wschurman wschurman force-pushed the wschurman/02-27-fix_escape_all_parts_of_the_local_memory_cacher_cache_key branch from 1d447e9 to 37f2c00 Compare March 2, 2026 17:32
@wschurman wschurman marked this pull request as ready for review March 2, 2026 17:37
@wschurman wschurman requested review from ide and quinlanj March 2, 2026 17:37
@wschurman wschurman merged commit 21f2f6f into main Mar 2, 2026
7 checks passed
@wschurman wschurman deleted the wschurman/02-27-fix_escape_all_parts_of_the_local_memory_cacher_cache_key branch March 2, 2026 18:02
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.

2 participants