[marksvc]

The apply middleware is the first of our code to see a client's op,
before JSON0 has checked it, and it read each component's path without
establishing that there is one. A component with no path made deepGet throw "path
is not iterable", and a component with a path but no instruction made the
speculative apply in opsWithDocs throw. In both cases the error came out of our
middleware rather than from the rejection JSON0 was about to make, so the client
got an error with no code in place of ERR_OT_OP_NOT_APPLIED. Nothing was
committed either way.

The middleware now checks that every component has both a path and an
instruction before reading any of them, and leaves the op untouched
when one does not. (Note that it's not checking to protect further processing
by ShareDB or future middleware, but to prevent crashing in the later
lines of handleApply.) The submit then carries on to JSON0, which
refuses the op as it refuses any op it cannot apply, so the client gets the error that a
malformed op has always produced.

---

Open in Devin Review

---

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

---

This change is 

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (task/sec1-1@a51915b). Learn more about missing BASE report.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@              Coverage Diff               @@
##             task/sec1-1    #4217   +/-   ##
==============================================
  Coverage               ?   81.20%           
==============================================
  Files                  ?      670           
  Lines                  ?    42920           
  Branches               ?     7116           
==============================================
  Hits                   ?    34853           
  Misses                 ?     6919           
  Partials               ?     1148           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[pmachapman]

@pmachapman reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on marksvc).

---

src/RealtimeServer/common/services/json-doc-service.ts line 61 at r1 (raw file):

    )
  );
}

This function returns true if an invalid instruction is snuck in with a valid, i.e.:

isComponentList([{p:['path'], invalid: 'x', li: 'y'}])

Or an invalid combination:

isComponentList([{p:['path'], oi: 'x', ld: 'y'}])

Are these ops you want to prevent with this function? (ShareDB will reject these with ERR_MESSAGE_BADLY_FORMED)

Code quote:

export function isComponentList(ops: unknown): ops is ShareDB.Op[] {
  return (
    Array.isArray(ops) &&
    ops.every(
      op =>
        op != null &&
        typeof op === 'object' &&
        Array.isArray(op.p) &&
        INSTRUCTIONS.some(instruction => instruction in op)
    )
  );
}