Added a transformer that injects byte sequences

[blanu]

This transformer has a list of byte sequences to add and another list to remove.
Each sequence also has an index into the packet stream, an offset into the packet, and a target packet length.
So for instance, packet #4 could have a byte sequence injected at the 17th byte of a packet of length 118.
An injectable packet is constructed by placing the byte sequence at the specified offset in the packet and then filling in the rest with random bytes in order to generate a packet of the target length.
As the index of the packet is also specified, injected packets will always be sent in a specified order, interleaves with real data packets.

On the receiving end, packets may have been reordered. Therefore, the receiver determines which packets to remove by matching byte sequences to offsets and ignoring the index. Since there are no constraints on the injected byte sequences, false positives are possible. For instance, a packet could be injected where the first byte is 0. This is a common value for the first byte, so based on sequence matching a non-injected packet may be dropped. In this case, SCTP retransmission should be sufficient to recover. False positives are unlikely to occur in practice using real settings for the byte sequences to inject.

[trevj]

Review status: 0 of 1 files reviewed at latest revision, 9 unresolved discussions, all commit checks successful.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 9 [r1] (raw file):}
Is this the object expected, in serialised form, by ByteSequenceShaper.configure?

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 60 [r1] (raw file):}
This isn't an extremely helpful comment and this variable is modified by >1 function. What exactly does it represent?

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 67 [r1] (raw file):}
I just remembered it's good practice to have unimplemented methods raise an error (fail fast, and all that), e.g.:

public setKey = (key:ArrayBuffer) : void => {
  throw new Error('setKey unimplemented');
}

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 76 [r1] (raw file):}
Confused by this; does this expect a serialised SequenceConfig?

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 87 [r1] (raw file):}
Logging and throwing is generally a code smell: just throw the error and let the caller decide how/if to handle it.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 92 [r1] (raw file):}
Don't swallow errors; remove this catch, allowing the caller decide how to handle an error.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 100 [r1] (raw file):}
style: this brace can fit on the previous line

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 113 [r1] (raw file):}
You're calling this.outputIndex_++ in both the if and else blocks. Suggest you move it to just before this if/else block.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 145 [r1] (raw file):}
Prefer static functions when possible, to reduce state. This looks like it can be static, as well as the functions it calls.

---

Comments from the review on Reviewable.io

[blanu]

Review status: 0 of 1 files reviewed at latest revision, 9 unresolved discussions, all commit checks successful.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 9 [r1] (raw file):}
Yes, I have added additional comments to this effect.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 60 [r1] (raw file):}
I have added additional comments to address this.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 67 [r1] (raw file):}
Okay I have made this change.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 76 [r1] (raw file):}
Okay I changed this.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 87 [r1] (raw file):}
Okay I changed this.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 92 [r1] (raw file):}
Okay I changed this.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 100 [r1] (raw file):}
Okay I have made this change.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 113 [r1] (raw file):}
Okay I have rewritten this.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 145 [r1] (raw file):}
Okay I have made this change.

---

Comments from the review on Reviewable.io

[trevj]

Just a couple of things, then 👍.

---

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 37 [r2] (raw file):}
I think you had this right the first time, in that it doesn't need to be exported.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 152 [r2] (raw file):}
spaces: i < config.addSequences

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 165 [r2] (raw file):}
It's more grokkable as an object if you put each field on its own line, i.e.

return {
  index: model.index,
  offset: model.offset,
  // ...
};

---

Comments from the review on Reviewable.io

[blanu]

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 31 [r2] (raw file):}
It needs to be exported in order for the deserializer methods to be changed from private to static. Static functions are public and so are exported with the class, so all of their argument and return types must be exported as well.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 150 [r2] (raw file):}
Okay I have made this change.

---

^{src/fancy-transformers/byteSequenceShaper.ts, line 163 [r2] (raw file):}
Okay I have made this change.

---

Comments from the review on Reviewable.io

[trevj]

Still 👍 .

---

Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from the review on Reviewable.io