M1I13: WebSocketFrame

[s2x]

Summary

Revision	Changes
1	Initial implementation of RFC 6455 WebSocket frame encoder/decoder

• Added WebSocketFrame value object with encode(), decode(), and frameSize() methods
• Handles all payload length variants (7-bit, 16-bit, 64-bit)
• Supports masking/unmasking for client→server frames
• Returns null for incomplete frames (no silent truncation)
• 23 unit tests covering encode, decode, frameSize, masking, extended lengths

Acceptance criteria

• encode() produces valid RFC 6455 frames with correct length encoding
• decode() returns null for data shorter than required frame size (incomplete)
• decode() returns WebSocketFrame for complete frames with correct payload
• frameSize() reports total frame size including header
• Masked payloads correctly XOR-unmasked
• 16-bit and 64-bit extended payload lengths handled

Closes #13

[s2x]

Review Summary — Round 1

#	Severity	File:Line	Finding
1	MEDIUM	src/Server/Protocol/WebSocketFrame.php:59	readUint64 may return negative int for 64-bit extended length with MSB set
2	LOW	src/Server/Protocol/WebSocketFrame.php:148	Magic number 2 for offset in frameSize()

Follow-up Observations

• Consider adding opcode validation in decode() — RFC 6455 reserves opcodes 0x3–0x7 and 0xB–0xF. Rejecting them would catch malformed frames early. File separately.
• encode() is fragmentation-unaware (always sets FIN=1, no $fin parameter). Acceptable for single-frame messages but worth planning for multi-frame support.
• XOR unmasking uses byte-by-byte .= concatenation — consider a more efficient approach (e.g. str_repeat‑based) if large WebSocket frames become common.
• readUint64 returns null for 64‑bit values > PHP_INT_MAX on 32‑bit PHP (returns float, fails is_int). Unlikely to matter for a server‑side tool, but worth a comment.
• Missing PHPDoc blocks on the class and public methods (opcode values, RFC 6455 references).

What's Fine ✔️

• All 6 acceptance criteria from the milestone doc are satisfied with test coverage
• encode()/decode()/frameSize() correctly handle all three payload length variants (≤125, 16‑bit, 64‑bit)
• Masked payloads correctly XOR‑unmasked
• Incomplete frames correctly return null at every boundary (empty, 1‑byte, short payload, short extended length field)
• Implementation matches the milestone plan — single file created, no unexpected side effects
• 23 unit tests with good coverage of edge cases

[s2x]

RESOLVED — Fixed in 5f8e12c: added guard to readUint64 and 2 regression tests.

[s2x]

RESOLVED — Fixed in 5f8e12c: replaced literal with tracked variable in frameSize(), consistent with decode().

[s2x]

Review Summary — Round 2

All findings from Round 1 resolved. No new issues found.

#	Severity	Finding	Status
1	MEDIUM	readUint64 negative overflow via MSB	FIXED — $value >= 0 guard + 2 regression tests
2	LOW	Literal 2 offset in frameSize()	FIXED — tracked $offset variable, consistent with decode()

Follow-up Observations

• Opcode validation: The class accepts any 4-bit opcode (0x0–0xF). RFC 6455 only defines 0x0, 0x1, 0x2, 0x8, 0x9, 0xA. Reserved opcodes (0x3–0x7, 0xB–0xF) could be rejected in a future iteration.
• encode() FIN fragmentation: encode() always sets FIN=1. No continuation frames. By design per current spec.
• XOR unmasking perf: Byte-by-byte loop could be optimized with bulk string XOR for large payloads. Separate issue candidate.

What's Fine

• All 6 acceptance criteria satisfied
• 25 tests covering all paths
• 7/16/64-bit payload lengths handled correctly
• Incomplete frame detection returns null at every boundary
• Masking XOR logic correct
• PHP 8.2 compat, PHPStan clean, CI green