rmi: restrict parser to java.rmi.registry.Registry + add Decoder

[phith0n]

Summary

Three sharpening passes on the rmi package. Together they shrink the public API, tighten the scope, and collapse the implementation onto a single primitive.

1. Scope: Registry-only

Restrict the parser from "general JRMP + Registry decoding" to java.rmi.registry.Registry only.

• readCall rejects any Call whose header fails the Registry dispatch triple (ObjID == REGISTRY_ID, methodHash == RegistryInterfaceHash, op ∈ [0..4]) at parse time with a specific error.
• readCallArgs drops its sentinel fallback — the only peek-past-last-arg code path is gone. Registry Calls over a live TCP reader return the instant their own bytes arrive, with no risk of deadlock when the client waits for the server's reply.
• Every non-Registry Remote interface has its own method-hash table we don't carry; the ambiguity machinery (optional Decoded, sentinel peeks, fall-through error cases) was the single largest source of complexity. Registry coverage alone serves the project's use case (intercepting rmiregistry traffic).

2. API: two entry points, chosen by input shape

• rmi.FromBytes(data []byte) — bytes already in memory (.bin captures, io.ReadAll of an HTTP body). Loops until io.EOF and returns the whole Transmission. Not safe with a live net.Conn: the loop would deadlock on the first post-last-frame peek when the peer keeps the connection open waiting for a reply (i.e. every synchronous RPC).
• rmi.Decoder (NewDecoder + Opening + Next) — one frame per Next() call, the only viable choice for a live connection. Caller applies SetReadDeadline on the underlying net.Conn between calls to bound how long to wait for the next frame.

The earlier PR (#19) introduced FromStream(io.Reader) alongside these. That turned out to be a trap: its signature implied "any reader works" but in practice net.Conn deadlocks the message loop. Removed in favor of the sharper two-way split — []byte is unambiguously bounded, Decoder is explicitly frame-by-frame.

3. Implementation: single primitive, thin wrapper

FromBytes is now a thin wrapper that drives Decoder to EOF. Before, both had parallel "PeekN → EOF check → readMessage" loops. Now Decoder is the only place that loop lives, and FromBytes is ~15 lines:

func FromBytes(data []byte) (*Transmission, error) {
    d := NewDecoder(bytes.NewReader(data))
    opening, err := d.Opening()
    if err != nil { return nil, err }
    t := &Transmission{
        Handshake:      opening.Handshake,
        Acknowledge:    opening.Acknowledge,
        ClientEndpoint: opening.ClientEndpoint,
    }
    for {
        msg, err := d.Next()
        if errors.Is(err, io.EOF) { return t, nil }
        if err != nil { return nil, err }
        t.Messages = append(t.Messages, msg)
    }
}

Return-side caveat (documented, not fixed)

ReturnData still uses a sentinel peek for its 0-vs-1 payload count — bind/rebind/unbind return void (0 payload), list/lookup/Exception return 1. Without call/response correlation (which direction-agnostic parsing can't do), the sentinel is the only viable strategy. It blocks between frames on a live reader until the next flag byte arrives or the reader's deadline fires. Decoder's doc comments and CLAUDE.md call this out explicitly.

Documentation

Four runnable godoc examples in rmi/example_test.go, each with verified // Output: blocks:

• ExampleFromBytes — parse a real rmiregistry capture (testcases/rmi/jdk17/lookup-c2s.bin), navigate Handshake / ClientEndpoint / Decoded method + args.
• ExampleNewDecoder — frame-by-frame idiom (Opening() + for { Next() }).
• ExampleDecoder_liveConnection — the net.Conn pattern, with SetReadDeadline placement and the error-handling contract.

CLAUDE.md's rmi section rewritten: "Scope: Registry only" opens the section; "Arg/payload-reading strategy" documents the two clean paths (Call exact-count, Return sentinel); "Live-TCP ergonomics" explains why FromBytes deadlocks on a typical session and shows the Decoder usage pattern.

Test plan

• go test ./... — all tests green across 2 JDK fixture dirs (jdk17, jdk8)
  • TestDecoderRegistryCallReturnsWithoutPeek — feeds exact Registry Call bytes into a never-closed io.Pipe with a 2s deadlock guard. Proves the exact-count fast path returns without peeking past the last arg.
  • TestDecoderChunkedDelivery — feeds a real capture through io.Pipe in 7-byte chunks, proves Decoder stitches chunks across frame-internal boundaries.
  • TestCallRejectsNonRegistry (table-driven) — covers non-zero ObjNum, wrong interface hash, unknown op-index.
  • TestBackToBackRegistryCallsNoSeparator — two Calls concatenated without any intervening bytes, regression guard for the exact-count property.
• go vet ./... / go build ./...
• CI: lint + 15 tests jobs (Go 1.18–1.22 × Linux/macOS/Windows) all passing.

Commits

40117dd	rmi: restrict parser to java.rmi.registry.Registry + add Decoder
db5a08d	rmi: add runnable godoc examples + fix gofmt on Decoder doc
ba58371	rmi: drop FromStream, simplify to FromBytes + Decoder
ac3ac3a	rmi: make FromBytes a thin wrapper over Decoder

Supersedes #19

That PR tried to expand scope (support any frame type in streaming). The non-Registry machinery turned out to be more ambiguity than it removed — closed and superseded by this one going the other direction.

🤖 Generated with Claude Code

[phith0n]

Code review

Found 2 issues, both stale doc comments that contradict the code after the multi-commit refactor:

1. rmi/message.go:15-18 — readMessage doc says "the per-reader arg strategies (exact-count-for-Registry vs sentinel) are chosen internally based on each frame's header". That phrasing implies Calls can take either strategy, but after commit 40117dd the sentinel fallback for Calls was removed: readCallArgs always reads exact count. Only Returns still use a sentinel. The comment should say something like "Calls use exact count; Returns use a sentinel peek for 0-vs-1 payload."

zkar/rmi/message.go

Lines 14 to 20 in ac3ac3a

	// readMessage peeks the next byte and dispatches to the matching reader.
	// Works identically for buffered and streaming input — the per-reader arg
	// strategies (exact-count-for-Registry vs sentinel) are chosen internally
	// based on each frame's header, not on the input source.
	func readMessage(outer *commons.Stream) (Message, error) {
	next, err := outer.PeekN(1)

2. rmi/call.go:24-27 — the CallMessage type doc states "Decoded is always non-nil for a successfully parsed CallMessage", but the actual decoder path at call.go:154-156 swallows decoder errors (if decoded, derr := registryDecoders[op](args); derr == nil { call.Decoded = decoded }), so a well-formed-but-semantically-bad Call (e.g. the name arg isn't a TC_STRING) parses successfully with Decoded == nil. The inline comment in ToString at call.go:46-48 correctly documents this nil possibility — so the type doc is the one to fix.

zkar/rmi/call.go

Lines 23 to 35 in ac3ac3a

	//
	// Decoded is always non-nil for a successfully parsed CallMessage. Its
	// Args slice is the human-oriented view (scalar values inlined, stub
	// subtrees referenced by handler). ObjectArgs and Raw expose the raw
	// TCContent tree for callers that want to walk the embedded stream.
	type CallMessage struct {
	ObjID ObjID
	Operation int32
	MethodHash int64
	Raw *serz.Serialization
	ObjectArgs []*serz.TCContent
	Decoded *DecodedCall
	}

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}