Skip to content

M1I06: ForkSharedProxy#62

Merged
s2x merged 1 commit into
mainfrom
feature/m1i06-forksharedproxy
May 1, 2026
Merged

M1I06: ForkSharedProxy#62
s2x merged 1 commit into
mainfrom
feature/m1i06-forksharedproxy

Conversation

@s2x
Copy link
Copy Markdown
Contributor

@s2x s2x commented May 1, 2026

Summary

Item Detail
Proxy ForkSharedProxy
Socket option SO_REUSEADDR only (no SO_REUSEPORT)
Bind 0.0.0.0:$port
Listen SOMAXCONN
isSupported() Always true
accept() Delegates to $socket->accept()

Acceptance criteria

  • Implements SocketProxyInterface
  • isSupported() always returns true
  • Uses only SO_REUSEADDR (no SO_REUSEPORT)
  • Test passes on all platforms

Closes #8

Copy link
Copy Markdown
Contributor Author

@s2x s2x left a comment

Choose a reason for hiding this comment

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

Review Summary — Round 1

# Severity File:Line Finding
No findings

What Checked

  • Code smells & logic errors: clean, mirrors ReusePortProxy pattern correctly
  • Security & leaks: no secrets, PII, or injection vectors
  • Issue #8 compliance: all 4 acceptance criteria met
  • Tests: 4/4 pass, 6 assertions

Follow-up Observations

  • The $protocol parameter in SocketProxyInterface::createSocket() goes unused by both ForkSharedProxy and ReusePortProxy (both hardcode SOCK_STREAM/SOL_TCP). It will gain meaning when TLS/UDP proxies arrive in later milestones — no action needed now.
  • No PHPDoc annotations on methods — consistent with existing ReusePortProxy style.

What's Fine ✔️

  • Implementation is a minimal, correct subset of ReusePortProxy omitting only SO_REUSEPORT
  • Error handling pattern (@socket_create + === false check + custom exception) matches existing code
  • Test structure identical to ReusePortProxyTest — covers interface check, socket creation, accept with client connection, and isSupported()
  • Bind 0.0.0.0:$port, listen SOMAXCONN exactly as specified

@s2x s2x merged commit 8922ed0 into main May 1, 2026
5 checks passed
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.

M1I06: ForkSharedProxy

1 participant