Repository: creachadair/jrpc2 (go)

Issue:
jhttp: Requests with invalid ID structure are incorrectly flagged as notifications
The bridge parses requests with jrpc2.ParseRequests. This correctly detects invalid request ID structure (e.g., `{"bogus":true}`), but this error is not correctly propagated. In particular, ParseRequests does not check for validation errors, and the ID field is `nil` when the structure was invalid.

This causes the bridge to treat the invalid request as a notification, as if its ID were not set at all. It is forwarded to the server, but the server does not see the problem since the ID on that request is not set.

Either a request needs to surface validation errors, or the bridge needs to use a smarter parsing structure.

Pull request description:
Rework the jrpc2.ParseRequests function.
Add a new ParsedRequest type and return that instead of Request.
The latter is intended for consumption by Handler methods, and is hard to use
effectively in other contexts.  The new type exposes validation errors that
occur during parsing and allows the caller to detect specific problems with
each individual request in a batch separately.

This change also allows us to consolidate the checking of version errors, and
to remove most of the special case checks inside the client and server.

- Update the jhttp.Bridge to use the new ParsedRequest type.

- Update test cases. In cases where we really needed a jrpc2.Request, use the
  newly introduced internal/testutil package, which uses the server plumbing to
  render a request.

Fixes #80.

Patch:
diff --git a/client.go b/client.go
index 339c9ff..b0f6655 100644
--- a/client.go
+++ b/client.go
@@ -6,7 +6,6 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
-	"fmt"
 	"io"
 	"strconv"
 	"sync"
@@ -150,22 +149,18 @@ func (c *Client) deliver(rsp *jmessage) {
 	}
 
 	id := string(fixID(rsp.ID))
-	if p := c.pending[id]; p == nil {
+	p := c.pending[id]
+	if p == nil {
 		c.log("Discarding response for unknown ID %q", id)
-	} else if !c.versionOK(rsp.V) {
-		delete(c.pending, id)
-		p.ch <- &jmessage{
-			ID: rsp.ID,
-			E: &Error{
-				Code:    code.InvalidRequest,
-				Message: fmt.Sprintf("incorrect version marker %q", rsp.V),
-			},
-		}
+		return
+	}
+	// Remove the pending request from the set and deliver its response.
+	// Determining whether it's an error is the caller's responsibility.
+	delete(c.pending, id)
+	if rsp.err != nil {
+		p.ch <- &jmessage{ID: rsp.ID, E: rsp.err}
 		c.log("Invalid response for ID %q", id)
 	} else {
-		// Remove the pending request from the set and deliver its response.
-		// Determining whether it's an error is the caller's responsibility.
-		delete(c.pending, id)
 		p.ch <- rsp
 		c.log("Completed request for ID %q", id)
 	}
@@ -422,8 +417,6 @@ func (c *Client) stop(err error) {
 	c.ch = nil
 }
 
-func (c *Client) versionOK(v string) bool { return v == Version }
-
 // marshalParams validates and marshals params to JSON for a request.  The
 // value of params must be either nil or encodable as a JSON object or array.
 func (c *Client) marshalParams(ctx context.Context, method string, params interface{}) (json.RawMessage, error) {
diff --git a/error.go b/error.go
index 7e95246..67bb443 100644
--- a/error.go
+++ b/error.go
@@ -61,6 +61,9 @@ var errEmptyBatch = &Error{Code: code.InvalidRequest, Message: "empty request ba
 // errInvalidParams is the error reported for invalid request parameters.
 var errInvalidParams = &Error{Code: code.InvalidParams, Message: code.InvalidParams.String()}
 
+// errTaskNotExecuted is the internal sentinel error for an unassigned task.
+var errTaskNotExecuted = new(Error)
+
 // ErrConnClosed is returned by a server's push-to-client methods if they are
 // called after the client connection is closed.
 var ErrConnClosed = errors.New("client connection is closed")
diff --git a/jhttp/bridge.go b/jhttp/bridge.go
index f637e24..2d627dc 100644
--- a/jhttp/bridge.go
+++ b/jhttp/bridge.go
@@ -40,7 +40,7 @@ import (
 // headers. Use jhttp.HTTPRequest to retrieve the request from the context.
 type Bridge struct {
 	local    server.Local
-	parseReq func(*http.Request) ([]*jrpc2.Request, error)
+	parseReq func(*http.Request) ([]*jrpc2.ParsedRequest, error)
 	getter   *Getter
 }
 
@@ -77,12 +77,8 @@ func (b Bridge) serveInternal(w http.ResponseWriter, req *http.Request) error {
 	// The HTTP request requires a response, but the server will not reply if
 	// all the requests are notifications. Check whether we have any calls
 	// needing a response, and choose whether to wait for a reply based on that.
-	//
-	// Note that we are forgiving about a missing version marker in a request,
-	// since we can't tell at this point whether the server is willing to accept
-	// messages like that.
 	jreq, err := b.parseHTTPRequest(req)
-	if err != nil && err != jrpc2.ErrInvalidVersion {
+	if err != nil {
 		return err
 	}
 
@@ -93,50 +89,67 @@ func (b Bridge) serveInternal(w http.ResponseWriter, req *http.Request) error {
 	// To do this, we keep track of the inbound ID for each request so that we
 	// can map the responses back. This takes advantage of the fact that the
 	// *jrpc2.Client detangles batch order so that responses come back in the
-	// same order (modulo notifications) even if the server response did not
+	// same order (omitting notifications) even if the server response did not
 	// preserve order.
+	//
+	// Requests that are already known to be invalid are converted to error
+	// responses directly. Besides preventing the server from doing the same
+	// error check a second time, this avoids the issue that a remapped ID may
+	// obcure an invalid request ID (see #80).
+	var results []json.RawMessage
 
 	// Generate request specifications for the client.
-	var inboundID []string                // for requests
-	spec := make([]jrpc2.Spec, len(jreq)) // requests & notifications
-	for i, req := range jreq {
-		spec[i] = jrpc2.Spec{
-			Method: req.Method(),
-			Notify: req.IsNotification(),
+	var inboundID []string // for calls
+	var spec []jrpc2.Spec  // requests & notifications
+	for _, req := range jreq {
+		if req.Error != nil {
+			// Filter out statically invalid requests.
+			msg, err := marshalError(req)
+			if err != nil {
+				return err
+			}
+			results = append(results, msg)
+			continue
 		}
-		if req.HasParams() {
-			var p json.RawMessage
-			req.UnmarshalParams(&p)
-			spec[i].Params = p
-		}
-		if !spec[i].Notify {
-			inboundID = append(inboundID, req.ID())
+
+		spec = append(spec, jrpc2.Spec{
+			Method: req.Method,
+			Notify: req.ID == "",
+			Params: req.Params,
+		})
+		if req.ID != "" {
+			inboundID = append(inboundID, req.ID)
 		}
 	}
 
-	// Attach the HTTP request to the client context, so the encoder can see it.
-	ctx := context.WithValue(req.Context(), httpReqKey{}, req)
-	rsps, err := b.local.Client.Batch(ctx, spec)
-	if err != nil {
-		return err
+	if len(spec) != 0 {
+		// Attach the HTTP request to the client context, so the encoder can see it.
+		ctx := context.WithValue(req.Context(), httpReqKey{}, req)
+		rsps, err := b.local.Client.Batch(ctx, spec)
+		if err != nil {
+			return err
+		}
+		for i, rsp := range rsps {
+			// Map the responses back to their original IDs and marshal to JSON.
+			rsp.SetID(inboundID[i])
+			msg, err := json.Marshal(rsp)
+			if err != nil {
+				return err
+			}
+			results = append(results, msg)
+		}
 	}
 
-	// If all the requests were notifications, report success without responses.
-	if len(rsps) == 0 {
+	// If all the requests were notifications and there were no invalid ones,
+	// report success without responses.
+	if len(results) == 0 {
 		w.WriteHeader(http.StatusNoContent)
 		return nil
 	}
-
-	// Otherwise, map the responses back to their original IDs, and marshal the
-	// response back into the body.
-	for i, rsp := range rsps {
-		rsp.SetID(inboundID[i])
-	}
-
-	return b.encodeResponses(rsps, w)
+	return b.encodeResponses(results, w)
 }
 
-func (b Bridge) parseHTTPRequest(req *http.Request) ([]*jrpc2.Request, error) {
+func (b Bridge) parseHTTPRequest(req *http.Request) ([]*jrpc2.ParsedRequest, error) {
 	if b.parseReq != nil {
 		return b.parseReq(req)
 	}
@@ -147,7 +160,7 @@ func (b Bridge) parseHTTPRequest(req *http.Request) ([]*jrpc2.Request, error) {
 	return jrpc2.ParseRequests(body)
 }
 
-func (b Bridge) encodeResponses(rsps []*jrpc2.Response, w http.ResponseWriter) error {
+func (b Bridge) encodeResponses(rsps []json.RawMessage, w http.ResponseWriter) error {
 	// If there is only a single reply, send it alone; otherwise encode a batch.
 	// Per the spec (https://www.jsonrpc.org/specification#batch), this is OK;
 	// we are not required to respond to a batch with an array:
@@ -155,11 +168,11 @@ func (b Bridge) encodeResponses(rsps []*jrpc2.Response, w http.ResponseWriter) e
 	//   The Server SHOULD respond with an Array containing the corresponding
 	//   Response objects
 	//
-	data, err := marshalResponses(rsps)
-	if err != nil {
-		return err
+	if len(rsps) == 1 {
+		writeJSON(w, http.StatusOK, rsps[0])
+	} else {
+		writeJSON(w, http.StatusOK, rsps)
 	}
-	writeJSON(w, http.StatusOK, json.RawMessage(data))
 	return nil
 }
 
@@ -215,7 +228,7 @@ type BridgeOptions struct {
 	//
 	// Setting this hook disables the default requirement that the request
 	// method be POST and the content-type be application/json.
-	ParseRequest func(*http.Request) ([]*jrpc2.Request, error)
+	ParseRequest func(*http.Request) ([]*jrpc2.ParsedRequest, error)
 
 	// If non-nil, this function is used to parse a JSON-RPC method name and
 	// parameters from the URL of an HTTP GET request. If this function reports
@@ -241,7 +254,7 @@ func (o *BridgeOptions) serverOptions() *jrpc2.ServerOptions {
 	return o.Server
 }
 
-func (o *BridgeOptions) parseRequest() func(*http.Request) ([]*jrpc2.Request, error) {
+func (o *BridgeOptions) parseRequest() func(*http.Request) ([]*jrpc2.ParsedRequest, error) {
 	if o == nil {
 		return nil
 	}
@@ -267,10 +280,17 @@ func HTTPRequest(ctx context.Context) *http.Request {
 	return nil
 }
 
-// marshalResponses encodes a batch of JSON-RPC responses into JSON.
-func marshalResponses(rsps []*jrpc2.Response) ([]byte, error) {
-	if len(rsps) == 1 {
-		return json.Marshal(rsps[0])
+// marshalError encodes an error response for an invalid request.
+func marshalError(req *jrpc2.ParsedRequest) ([]byte, error) {
+	v, err := json.Marshal(req.Error)
+	if err != nil {
+		return nil, err
+	}
+
+	// If the ID is empty, set the response ID to null.
+	id := req.ID
+	if id == "" {
+		id = "null"
 	}
-	return json.Marshal(rsps)
+	return []byte(fmt.Sprintf(`{"jsonrpc":"2.0","id":%s,"error":%s}`, id, string(v))), nil
 }
diff --git a/json.go b/json.go
index 141de86..83f0e0e 100644
--- a/json.go
+++ b/json.go
@@ -9,35 +9,37 @@ import (
 	"github.com/creachadair/jrpc2/code"
 )
 
-// ErrInvalidVersion is returned by ParseRequests if one or more of the
-// requests in the input has a missing or invalid version marker.
-var ErrInvalidVersion error = &Error{Code: code.InvalidRequest, Message: "incorrect version marker"}
-
 // ParseRequests parses a single request or a batch of requests from JSON.
-//
-// If msg is syntactically valid apart from one or more of the requests having
-// a missing or invalid JSON-RPC version, ParseRequests returns ErrInvalidVersion
-// along with the parsed results.
-func ParseRequests(msg []byte) ([]*Request, error) {
-	var req jmessages
-	if err := req.parseJSON(msg); err != nil {
+// This function reports an error only if msg is not valid JSON. The caller
+// must check the individual results for their validity.
+func ParseRequests(msg []byte) ([]*ParsedRequest, error) {
+	var reqs jmessages
+	if err := reqs.parseJSON(msg); err != nil {
 		return nil, err
 	}
 	var err error
-	out := make([]*Request, len(req))
-	for i, req := range req {
-		if req.V != Version {
-			err = ErrInvalidVersion
-		}
-		out[i] = &Request{
-			id:     fixID(req.ID),
-			method: req.M,
-			params: req.P,
+	out := make([]*ParsedRequest, len(reqs))
+	for i, req := range reqs {
+		out[i] = &ParsedRequest{
+			ID:     string(fixID(req.ID)),
+			Method: req.M,
+			Params: req.P,
+			Error:  req.err,
 		}
 	}
 	return out, err
 }
 
+// A ParsedRequest is the parsed form of a request message. If a request is
+// valid, its Error field is nil. Otherwise, the Error field describes why the
+// request is invalid, and the other fields may be incomplete or missing.
+type ParsedRequest struct {
+	ID     string
+	Method string
+	Params json.RawMessage
+	Error  *Error
+}
+
 // jmessages is either a single protocol message or an array of protocol
 // messages.  This handles the decoding of batch requests in JSON-RPC 2.0.
 type jmessages []*jmessage
@@ -111,8 +113,8 @@ type jmessage struct {
 	// and R. Specifically, if M != "" then E and R must both be unset. This is
 	// checked during parsing.
 
-	batch bool  // this message was part of a batch
-	err   error // if not nil, this message is invalid and err is why
+	batch bool   // this message was part of a batch
+	err   *Error // if not nil, this message is invalid and err is why
 }
 
 // isValidID reports whether v is a valid JSON encoding of a request ID.
@@ -129,8 +131,13 @@ func isValidID(v json.RawMessage) bool {
 	return false // anything else is garbage
 }
 
+// isValidVersion reports whether v is a valid JSON-RPC version string.
+func isValidVersion(v string) bool { return v == Version }
+
 func (j *jmessage) fail(code code.Code, msg string) {
-	j.err = &Error{Code: code, Message: msg}
+	if j.err == nil {
+		j.err = &Error{Code: code, Message: msg}
+	}
 }
 
 func (j *jmessage) toJSON() ([]byte, error) {
@@ -221,6 +228,11 @@ func (j *jmessage) parseJSON(data []byte) error {
 		}
 	}
 
+	// Report an error for an invalid version marker
+	if !isValidVersion(j.V) {
+		j.fail(code.InvalidRequest, "invalid version marker")
+	}
+
 	// Report an error if request/response fields overlap.
 	if j.M != "" && (j.E != nil || j.R != nil) {
 		j.fail(code.InvalidRequest, "mixed request and reply fields")
diff --git a/server.go b/server.go
index 837d6f7..f4d275a 100644
--- a/server.go
+++ b/server.go
@@ -283,17 +283,13 @@ func (s *Server) checkAndAssign(next jmessages) tasks {
 
 	// Phase 1: Check for errors and duplicate request IDs.
 	for _, req := range next {
-		if req.err != nil {
-			// keep the existing error
-		} else if !s.versionOK(req.V) {
-			req.err = ErrInvalidVersion
-		}
-
 		fid := fixID(req.ID)
 		t := &task{
 			hreq:  &Request{id: fid, method: req.M, params: req.P},
 			batch: req.batch,
-			err:   req.err,
+		}
+		if req.err != nil {
+			t.err = req.err
 		}
 		id := string(fid)
 		if old := dup[id]; old != nil {
@@ -749,8 +745,6 @@ func (s *Server) cancel(id string) bool {
 	return ok
 }
 
-func (s *Server) versionOK(v string) bool { return v == Version }
-
 // A task represents a pending method invocation received by the server.
 type task struct {
 	m Handler // the assigned handler (after assignment)
@@ -787,7 +781,7 @@ func (ts tasks) responses(rpcLog RPCLogger) jmessages {
 		}
 		if task.m == nil {
 			// No method was ever assigned for this task, so it was never run.
-			rsp.err = errors.New("task not executed")
+			rsp.err = errTaskNotExecuted
 		}
 		if task.err == nil {
 			rsp.R = task.val


Test patch:
diff --git a/examples_test.go b/examples_test.go
index 05552e2..cd63232 100644
--- a/examples_test.go
+++ b/examples_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/creachadair/jrpc2"
 	"github.com/creachadair/jrpc2/code"
 	"github.com/creachadair/jrpc2/handler"
+	"github.com/creachadair/jrpc2/internal/testutil"
 	"github.com/creachadair/jrpc2/server"
 )
 
@@ -98,10 +99,9 @@ func ExampleClient_Batch() {
 
 func ExampleRequest_UnmarshalParams() {
 	const msg = `{"jsonrpc":"2.0", "id":101, "method":"M", "params":{"a":1, "b":2, "c":3}}`
-
-	reqs, err := jrpc2.ParseRequests([]byte(msg))
+	req, err := testutil.ParseRequest(msg)
 	if err != nil {
-		log.Fatalf("ParseRequests: %v", err)
+		log.Fatalf("Parsing %#q failed: %v", msg, err)
 	}
 
 	var t, u struct {
@@ -110,7 +110,7 @@ func ExampleRequest_UnmarshalParams() {
 	}
 
 	// By default, unmarshaling ignores unknown fields (here, "c").
-	if err := reqs[0].UnmarshalParams(&t); err != nil {
+	if err := req.UnmarshalParams(&t); err != nil {
 		log.Fatalf("UnmarshalParams: %v", err)
 	}
 	fmt.Printf("t.A=%d, t.B=%d\n", t.A, t.B)
@@ -118,21 +118,21 @@ func ExampleRequest_UnmarshalParams() {
 	// To implement strict field checking, there are several options:
 	//
 	// Solution 1: Use the jrpc2.StrictFields helper.
-	err = reqs[0].UnmarshalParams(jrpc2.StrictFields(&t))
+	err = req.UnmarshalParams(jrpc2.StrictFields(&t))
 	if code.FromError(err) != code.InvalidParams {
 		log.Fatalf("UnmarshalParams strict: %v", err)
 	}
 
 	// Solution 2: Implement a DisallowUnknownFields method.
 	var p strictParams
-	err = reqs[0].UnmarshalParams(&p)
+	err = req.UnmarshalParams(&p)
 	if code.FromError(err) != code.InvalidParams {
 		log.Fatalf("UnmarshalParams strict: %v", err)
 	}
 
 	// Solution 3: Decode the raw message separately.
 	var tmp json.RawMessage
-	reqs[0].UnmarshalParams(&tmp) // cannot fail
+	req.UnmarshalParams(&tmp) // cannot fail
 	dec := json.NewDecoder(bytes.NewReader(tmp))
 	dec.DisallowUnknownFields()
 	if err := dec.Decode(&u); err == nil {
diff --git a/handler/example_test.go b/handler/example_test.go
index 898b23a..e774a37 100644
--- a/handler/example_test.go
+++ b/handler/example_test.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/creachadair/jrpc2"
 	"github.com/creachadair/jrpc2/handler"
+	"github.com/creachadair/jrpc2/internal/testutil"
 )
 
 func ExampleCheck() {
@@ -111,11 +112,9 @@ func ExamplePositional_array() {
 }
 
 func mustParseReq(s string) *jrpc2.Request {
-	reqs, err := jrpc2.ParseRequests([]byte(s))
+	req, err := testutil.ParseRequest(s)
 	if err != nil {
-		log.Fatalf("ParseRequests: %v", err)
-	} else if len(reqs) == 0 {
-		log.Fatal("ParseRequests: empty result")
+		log.Fatalf("ParseRequest: %v", err)
 	}
-	return reqs[0]
+	return req
 }
diff --git a/handler/handler_test.go b/handler/handler_test.go
index 6f057e8..38819fe 100644
--- a/handler/handler_test.go
+++ b/handler/handler_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/creachadair/jrpc2"
 	"github.com/creachadair/jrpc2/code"
 	"github.com/creachadair/jrpc2/handler"
+	"github.com/creachadair/jrpc2/internal/testutil"
 	"github.com/google/go-cmp/cmp"
 )
 
@@ -105,7 +106,7 @@ func TestFuncInfo_wrapDecode(t *testing.T) {
 	}
 	ctx := context.Background()
 	for _, test := range tests {
-		req := mustParseRequest(t,
+		req := testutil.MustParseRequest(t,
 			fmt.Sprintf(`{"jsonrpc":"2.0","id":1,"method":"x","params":%s}`, test.p))
 		got, err := test.fn(ctx, req)
 		if err != nil {
@@ -165,7 +166,7 @@ func TestNewStrict(t *testing.T) {
 	}
 	fn := handler.NewStrict(func(ctx context.Context, arg *arg) error { return nil })
 
-	req := mustParseRequest(t, `{
+	req := testutil.MustParseRequest(t, `{
    "jsonrpc": "2.0",
    "id":      100,
    "method":  "f",
@@ -199,7 +200,7 @@ func TestNew_pointerRegression(t *testing.T) {
 		t.Logf("Got argument struct: %+v", got)
 		return nil
 	})
-	req := mustParseRequest(t, `{
+	req := testutil.MustParseRequest(t, `{
    "jsonrpc": "2.0",
    "id":      "foo",
    "method":  "bar",
@@ -245,7 +246,7 @@ func TestPositional_decode(t *testing.T) {
 		{`{"jsonrpc":"2.0","id":15,"method":"add","params":[1,2,3]}`, 0, true}, // too many
 	}
 	for _, test := range tests {
-		req := mustParseRequest(t, test.input)
+		req := testutil.MustParseRequest(t, test.input)
 		got, err := call(context.Background(), req)
 		if !test.bad {
 			if err != nil {
@@ -438,17 +439,6 @@ func TestObjUnmarshal(t *testing.T) {
 	}
 }
 
-func mustParseRequest(t *testing.T, text string) *jrpc2.Request {
-	t.Helper()
-	req, err := jrpc2.ParseRequests([]byte(text))
-	if err != nil {
-		t.Fatalf("ParseRequests: %v", err)
-	} else if len(req) != 1 {
-		t.Fatalf("Wrong number of requests: got %d, want 1", len(req))
-	}
-	return req[0]
-}
-
 // stringByte is a byte with a custom JSON encoding. It expects a string of
 // decimal digits 1 and 0, e.g., "10011000" == 0x98.
 type stringByte byte
diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go
new file mode 100644
index 0000000..7c38bfc
--- /dev/null
+++ b/internal/testutil/testutil.go
@@ -0,0 +1,68 @@
+// Copyright (C) 2022 Michael J. Fromberger. All Rights Reserved.
+
+// Package testutil defines internal support code for writing tests.
+package testutil
+
+import (
+	"context"
+	"testing"
+
+	"github.com/creachadair/jrpc2"
+	"github.com/creachadair/jrpc2/channel"
+)
+
+// ParseRequest parses a single JSON request object.
+func ParseRequest(s string) (_ *jrpc2.Request, err error) {
+	// Check syntax.
+	if _, err := jrpc2.ParseRequests([]byte(s)); err != nil {
+		return nil, err
+	}
+
+	cch, sch := channel.Direct()
+	rs := newRequestStub()
+	srv := jrpc2.NewServer(rs, nil).Start(sch)
+	defer func() {
+		cch.Close()
+		serr := srv.Wait()
+		if err == nil {
+			err = serr
+		}
+	}()
+	if err := cch.Send([]byte(s)); err != nil {
+		return nil, err
+	}
+	req := <-rs.reqc
+	if !rs.isNote {
+		cch.Recv()
+	}
+	return req, nil
+}
+
+// MustParseRequest calls ParseRequest and fails t if it reports an error.
+func MustParseRequest(t *testing.T, s string) *jrpc2.Request {
+	t.Helper()
+
+	req, err := ParseRequest(s)
+	if err != nil {
+		t.Fatalf("Parsing %#q failed: %v", s, err)
+	}
+	return req
+}
+
+func newRequestStub() *requestStub {
+	return &requestStub{reqc: make(chan *jrpc2.Request, 1)}
+}
+
+type requestStub struct {
+	reqc   chan *jrpc2.Request
+	isNote bool
+}
+
+func (r *requestStub) Assign(context.Context, string) jrpc2.Handler { return r }
+
+func (r *requestStub) Handle(_ context.Context, req *jrpc2.Request) (interface{}, error) {
+	defer close(r.reqc)
+	r.isNote = req.IsNotification()
+	r.reqc <- req
+	return nil, nil
+}
diff --git a/internal/testutil/testutil_test.go b/internal/testutil/testutil_test.go
new file mode 100644
index 0000000..e154ca3
--- /dev/null
+++ b/internal/testutil/testutil_test.go
@@ -0,0 +1,28 @@
+// Copyright (C) 2022 Michael J. Fromberger. All Rights Reserved.
+
+package testutil_test
+
+import (
+	"testing"
+
+	"github.com/creachadair/jrpc2/internal/testutil"
+)
+
+func TestParseRequest(t *testing.T) {
+	t.Run("Invalid", func(t *testing.T) {
+		req, err := testutil.ParseRequest(`{this is invalid}`)
+		if err == nil {
+			t.Errorf("ParseRequest: got %+v, wanted error", req)
+		} else {
+			t.Logf("Invalid OK: %v", err)
+		}
+	})
+	t.Run("Call", func(t *testing.T) {
+		req := testutil.MustParseRequest(t, `{"jsonrpc":"2.0","id":1,"method":"OK"}`)
+		t.Logf("Call OK: %+v", req)
+	})
+	t.Run("Notification", func(t *testing.T) {
+		req := testutil.MustParseRequest(t, `{"jsonrpc":"2.0","id":null,"method":"OK"}`)
+		t.Logf("Note OK: %+v", req)
+	})
+}
diff --git a/internal_test.go b/internal_test.go
index 8daa00d..c846111 100644
--- a/internal_test.go
+++ b/internal_test.go
@@ -19,40 +19,47 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 )
 
+var errInvalidVersion = &Error{Code: code.InvalidRequest, Message: "invalid version marker"}
+
 func TestParseRequests(t *testing.T) {
 	tests := []struct {
 		input string
-		want  []*Request
+		want  []*ParsedRequest
 		err   error
 	}{
 		// An empty batch is valid and produces no results.
 		{`[]`, nil, nil},
 
 		// An empty single request is invalid but returned anyway.
-		{`{}`, []*Request{{}}, ErrInvalidVersion},
+		{`{}`, []*ParsedRequest{{Error: errInvalidVersion}}, nil},
+
+		// Structurally invalid JSON reports an error.
+		{`[`, nil, errInvalidRequest},
+		{`{}}`, nil, errInvalidRequest},
+		{`[{}, ttt]`, nil, errInvalidRequest},
 
 		// A valid notification.
-		{`{"jsonrpc":"2.0", "method": "foo", "params":[1, 2, 3]}`, []*Request{{
-			method: "foo",
-			params: json.RawMessage(`[1, 2, 3]`),
+		{`{"jsonrpc":"2.0", "method": "foo", "params":[1, 2, 3]}`, []*ParsedRequest{{
+			Method: "foo",
+			Params: json.RawMessage(`[1, 2, 3]`),
 		}}, nil},
 
 		// A valid request, with nil parameters.
-		{`{"jsonrpc":"2.0", "method": "foo", "id":10332, "params":null}`, []*Request{{
-			id: json.RawMessage("10332"), method: "foo",
+		{`{"jsonrpc":"2.0", "method": "foo", "id":10332, "params":null}`, []*ParsedRequest{{
+			ID: "10332", Method: "foo",
 		}}, nil},
 
 		// A valid mixed batch.
 		{`[ {"jsonrpc": "2.0", "id": 1, "method": "A", "params": {}},
-          {"jsonrpc": "2.0", "params": [5], "method": "B"} ]`, []*Request{
-			{method: "A", id: json.RawMessage(`1`), params: json.RawMessage(`{}`)},
-			{method: "B", params: json.RawMessage(`[5]`)},
+          {"jsonrpc": "2.0", "params": [5], "method": "B"} ]`, []*ParsedRequest{
+			{Method: "A", ID: "1", Params: json.RawMessage(`{}`)},
+			{Method: "B", Params: json.RawMessage(`[5]`)},
 		}, nil},
 
 		// An invalid batch.
-		{`[{"id": 37, "method": "complain", "params":[]}]`, []*Request{
-			{method: "complain", id: json.RawMessage(`37`), params: json.RawMessage(`[]`)},
-		}, ErrInvalidVersion},
+		{`[{"id": 37, "method": "complain", "params":[]}]`, []*ParsedRequest{
+			{Method: "complain", ID: "37", Params: json.RawMessage(`[]`), Error: errInvalidVersion},
+		}, nil},
 
 		// A broken request.
 		{`{`, nil, Errorf(code.ParseError, "invalid request value")},
@@ -67,7 +74,7 @@ func TestParseRequests(t *testing.T) {
 			continue
 		}
 
-		diff := cmp.Diff(test.want, got, cmp.AllowUnexported(Request{}), cmpopts.EquateEmpty())
+		diff := cmp.Diff(test.want, got, cmpopts.EquateEmpty())
 		if diff != "" {
 			t.Errorf("ParseRequests(%#q): wrong result (-want, +got):\n%s", test.input, diff)
 		}
@@ -113,17 +120,18 @@ func TestRequest_UnmarshalParams(t *testing.T) {
 			xy{X: 23}, `{"x":23, "z":"wat"}`, code.NoError},
 	}
 	for _, test := range tests {
-		req, err := ParseRequests([]byte(test.input))
-		if err != nil {
+		var reqs jmessages
+		if err := reqs.parseJSON([]byte(test.input)); err != nil {
 			t.Errorf("Parsing request %#q failed: %v", test.input, err)
-		} else if len(req) != 1 {
-			t.Fatalf("Wrong number of requests: got %d, want 1", len(req))
+		} else if len(reqs) != 1 {
+			t.Fatalf("Wrong number of requests: got %d, want 1", len(reqs))
 		}
+		req := &Request{id: reqs[0].ID, method: reqs[0].M, params: reqs[0].P}
 
 		// Allocate a zero of the expected type to unmarshal into.
 		target := reflect.New(reflect.TypeOf(test.want)).Interface()
 		{
-			err := req[0].UnmarshalParams(target)
+			err := req.UnmarshalParams(target)
 			if got := code.FromError(err); got != test.code {
 				t.Errorf("UnmarshalParams error: got code %d, want %d [%v]", got, test.code, err)
 			}
@@ -139,7 +147,7 @@ func TestRequest_UnmarshalParams(t *testing.T) {
 		}
 
 		// Check that the parameter string matches.
-		if got := req[0].ParamString(); got != test.pstring {
+		if got := req.ParamString(); got != test.pstring {
 			t.Errorf("ParamString(%#q): got %q, want %q", test.input, got, test.pstring)
 		}
 	}
diff --git a/jhttp/jhttp_test.go b/jhttp/jhttp_test.go
index fd77672..ef9cbc4 100644
--- a/jhttp/jhttp_test.go
+++ b/jhttp/jhttp_test.go
@@ -110,6 +110,20 @@ func TestBridge(t *testing.T) {
 		}
 	})
 
+	// Verify that an invalid ID is not swallowed by the remapping process (see #80).
+	t.Run("PostInvalidID", func(t *testing.T) {
+		got := mustPost(t, hsrv.URL, `{
+        "jsonrpc": "2.0",
+        "id": ["this is totally bogus"],
+        "method": "Test1"
+      }`, http.StatusOK)
+
+		const exp = `{"jsonrpc":"2.0","id":null,"error":{"code":-32600,"message":"invalid request ID"}}`
+		if got != exp {
+			t.Errorf("POST body: got %#q, want %#q", got, exp)
+		}
+	})
+
 	// Verify that a notification returns an empty success.
 	t.Run("PostNotification", func(t *testing.T) {
 		got := mustPost(t, hsrv.URL, `{
@@ -131,7 +145,7 @@ func TestBridge_parseRequest(t *testing.T) {
 	const wantReply = `{"jsonrpc":"2.0","id":100,"result":0}`
 
 	b := jhttp.NewBridge(testService, &jhttp.BridgeOptions{
-		ParseRequest: func(req *http.Request) ([]*jrpc2.Request, error) {
+		ParseRequest: func(req *http.Request) ([]*jrpc2.ParsedRequest, error) {
 			action := req.Header.Get("x-test-header")
 			if action == "fail" {
 				return nil, errors.New("parse hook reporting failure")
diff --git a/jrpc2_test.go b/jrpc2_test.go
index b3d0b2b..061dd60 100644
--- a/jrpc2_test.go
+++ b/jrpc2_test.go
@@ -607,11 +607,11 @@ func TestServer_nonLibraryClient(t *testing.T) {
 	}{
 		// Missing version marker (and therefore wrong).
 		{`{"id":0}`,
-			`{"jsonrpc":"2.0","id":0,"error":{"code":-32600,"message":"incorrect version marker"}}`},
+			`{"jsonrpc":"2.0","id":0,"error":{"code":-32600,"message":"invalid version marker"}}`},
 
 		// Version marker is present, but wrong.
 		{`{"jsonrpc":"1.5","id":1}`,
-			`{"jsonrpc":"2.0","id":1,"error":{"code":-32600,"message":"incorrect version marker"}}`},
+			`{"jsonrpc":"2.0","id":1,"error":{"code":-32600,"message":"invalid version marker"}}`},
 
 		// No method was specified.
 		{`{"jsonrpc":"2.0","id":2}`,