Skip to content

chore: refactor bin handler to be struct#21917

Merged
spikecurtis merged 1 commit intomainfrom
spike/internal-1300-handler-is-struct
Feb 6, 2026
Merged

chore: refactor bin handler to be struct#21917
spikecurtis merged 1 commit intomainfrom
spike/internal-1300-handler-is-struct

Conversation

@spikecurtis
Copy link
Contributor

@spikecurtis spikecurtis commented Feb 4, 2026

relates to: coder/internal#1300

Refactors the bin handler to be a struct instead of a handlerfunc. The reason we want this is because we are going to introduce a cache of compressed files, so we need somewhere to put this cache.

Copy link
Contributor

@coder-tasks coder-tasks bot left a comment

Choose a reason for hiding this comment

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

Code Review

Reviewed refactoring of bin handler from function to struct. The refactor is clean and prepares for future caching functionality. Found one potential path traversal issue that should be addressed.

Summary

Strengths:

  • Clean refactor with clear separation of concerns
  • All existing tests pass
  • Good preparation for adding compressed file cache
  • Maintains backward compatibility

⚠️ Issues Found:

  1. Path prefix check is insufficient - may allow path traversal

Details

The refactor successfully converts binHandler from a function to a struct, which is a logical change given the upcoming need to add caching state. The code maintains all existing functionality while preparing for future enhancements.

However, there's a security concern with the path prefix check that should be addressed before merge.

@spikecurtis spikecurtis force-pushed the spike/internal-1300-handler-is-struct branch from 0320b65 to 8251b2e Compare February 4, 2026 10:11
@spikecurtis spikecurtis force-pushed the spike/internal-1300-refactor-site-bin branch 2 times, most recently from 1613bcb to ac49605 Compare February 4, 2026 10:19
@spikecurtis spikecurtis force-pushed the spike/internal-1300-handler-is-struct branch from 8251b2e to 4429923 Compare February 4, 2026 10:19
@spikecurtis spikecurtis force-pushed the spike/internal-1300-refactor-site-bin branch from ac49605 to f184efa Compare February 4, 2026 11:50
@spikecurtis spikecurtis force-pushed the spike/internal-1300-handler-is-struct branch from 4429923 to af3fb2f Compare February 4, 2026 11:50
@spikecurtis spikecurtis force-pushed the spike/internal-1300-refactor-site-bin branch from f184efa to e838f7e Compare February 4, 2026 11:50
@spikecurtis spikecurtis force-pushed the spike/internal-1300-handler-is-struct branch from af3fb2f to 9ec9c39 Compare February 4, 2026 11:50
@spikecurtis spikecurtis force-pushed the spike/internal-1300-refactor-site-bin branch 2 times, most recently from 678c91a to a5bf33c Compare February 6, 2026 06:13
@spikecurtis spikecurtis force-pushed the spike/internal-1300-handler-is-struct branch from 9ec9c39 to 6bb53c2 Compare February 6, 2026 06:13
@spikecurtis spikecurtis changed the base branch from spike/internal-1300-refactor-site-bin to graphite-base/21917 February 6, 2026 06:29
@spikecurtis spikecurtis force-pushed the spike/internal-1300-handler-is-struct branch from 6bb53c2 to b478cc5 Compare February 6, 2026 06:30
@graphite-app graphite-app bot changed the base branch from graphite-base/21917 to main February 6, 2026 06:31
@spikecurtis spikecurtis force-pushed the spike/internal-1300-handler-is-struct branch from b478cc5 to 2816283 Compare February 6, 2026 06:31
@spikecurtis spikecurtis merged commit 110dcbb into main Feb 6, 2026
27 of 29 checks passed
Copy link
Contributor Author

Merge activity

@spikecurtis spikecurtis deleted the spike/internal-1300-handler-is-struct branch February 6, 2026 06:41
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