fix: updating readElement to match underlying data structure

modules/serialize/src/deserialize_factory.ts

@@ -88,9 +88,7 @@ export function deserializeFactory<Suite extends AlgorithmSuite> (
      */
     if (22 + contextLength > dataView.byteLength) return false // not enough data
 
-    const encryptionContext = contextLength > 0
-      ? decodeEncryptionContext(messageBuffer.slice(22, 22 + contextLength))
-      : {}
+    const encryptionContext = decodeEncryptionContext(messageBuffer.slice(22, 22 + contextLength))
     const dataKeyInfo = deserializeEncryptedDataKeys(messageBuffer, 22 + contextLength)
 
     /* Check for early return (Postcondition): Not Enough Data. deserializeEncryptedDataKeys will return false if it does not have enough data.
@@ -176,23 +174,20 @@ export function deserializeFactory<Suite extends AlgorithmSuite> (
     /* Precondition: There must be at least 1 EncryptedDataKey element. */
     needs(encryptedDataKeysCount, 'No EncryptedDataKey found.')
 
-    const elementInfo = readElements(encryptedDataKeysCount * 3, buffer, startPos + 2)
+    const elementInfo = readElements(encryptedDataKeysCount, 3, buffer, startPos + 2)
     /* Check for early return (Postcondition): readElement will return false if there is not enough data.
      * I can only continue if I have at least the entire EDK section.
      */
     if (!elementInfo) return false
     const { elements, readPos } = elementInfo
 
-    let remainingKeyCount = encryptedDataKeysCount
-    const encryptedDataKeys = []
-    while (remainingKeyCount--) {
-      const [rawId, rawInfo] = elements.splice(0, 2)
-      const [providerId, providerInfo] = [rawId, rawInfo].map(toUtf8)
-      const [encryptedDataKey] = elements.splice(0, 1)
-      // The providerInfo is technically a binary field, so I must pass rawInfo
-      const edk = new EncryptedDataKey({ providerInfo, providerId, encryptedDataKey, rawInfo })
-      encryptedDataKeys.push(edk)
-    }
+    const encryptedDataKeys = elements.map(
+      ([rawId, rawInfo, encryptedDataKey], _) => {
+        const providerId = toUtf8(rawId)
+        const providerInfo = toUtf8(rawInfo)
+        return new EncryptedDataKey({ providerInfo, providerId, encryptedDataKey, rawInfo })
+      }
+    )
     Object.freeze(encryptedDataKeys)
     return { encryptedDataKeys, readPos }
   }
@@ -219,7 +214,7 @@ export function deserializeFactory<Suite extends AlgorithmSuite> (
       encodedEncryptionContext.byteLength
     )
     const pairsCount = dataView.getUint16(0, false) // big endian
-    const elementInfo = readElements(pairsCount * 2, encodedEncryptionContext, 2)
+    const elementInfo = readElements(pairsCount, 2, encodedEncryptionContext, 2)
     /* Postcondition: Since the encryption context has a length, it must have pairs.
      * Unlike the encrypted data key section, the encryption context has a length
      * element.  This means I should always pass the entire section.
@@ -230,9 +225,8 @@ export function deserializeFactory<Suite extends AlgorithmSuite> (
     /* Postcondition: The byte length of the encodedEncryptionContext must match the readPos. */
     needs(encodedEncryptionContext.byteLength === readPos, 'Overflow, too much data.')
 
-    let count = pairsCount
-    while (count--) {
-      const [key, value] = elements.splice(0, 2).map(toUtf8)
+    for (let count = 0; count < pairsCount; count++) {
+      const [key, value] = elements[count].map(toUtf8)
       /* Postcondition: The number of keys in the encryptionContext must match the pairsCount.
        * If the same Key value is serialized...
        */

modules/serialize/src/read_element.ts

@@ -43,6 +43,7 @@ import { needs } from '@aws-crypto/material-management'
  */
 export function readElements (
   elementCount: number,
+  fieldsPerElement: number,
   buffer: Uint8Array,
   readPos: number = 0
 ) {
@@ -59,22 +60,27 @@ export function readElements (
   )
   const elements = []
 
-  /* Precondition: readElements readPos must be within the byte length of the buffer given. */
+  /* Precondition: readPos must be non-negative and within the byte length of the buffer given. */
   needs(readPos >= 0 && dataView.byteLength >= readPos, 'readPos out of bounds.')
 
-  /* Precondition: elementCount must not be negative. */
-  needs(elementCount >= 0, 'elementCount must be positive.')
+  /* Precondition: elementCount and fieldsPerElement must be non-negative. */
+  needs(elementCount >= 0 && fieldsPerElement >= 0, 'elementCount and fieldsPerElement must be positive.')
 
   while (elementCount--) {
-    /* Check for early return (Postcondition): Enough data must exist to read the Uint16 length value. */
-    if (readPos + 2 > dataView.byteLength) return false
-    const length = dataView.getUint16(readPos, false) // big endian
-    readPos += 2
-    /* Check for early return (Postcondition): Enough data must exist length of the value. */
-    if (readPos + length > dataView.byteLength) return false
-    const elementBinary = buffer.slice(readPos, readPos + length)
-    elements.push(elementBinary)
-    readPos += length
+    let element = []
+    let fieldCount = fieldsPerElement
+    while (fieldCount--) {
+      /* Check for early return (Postcondition): Enough data must exist to read the Uint16 length value. */
+      if (readPos + 2 > dataView.byteLength) return false
+      const length = dataView.getUint16(readPos, false) // big endian
+      readPos += 2
+      /* Check for early return (Postcondition): Enough data must exist length of the value. */
+      if (readPos + length > dataView.byteLength) return false
+      const fieldBinary = buffer.slice(readPos, readPos + length)
+      element.push(fieldBinary)
+      readPos += length
+    }
+    elements.push(element)
   }
   return { elements, readPos }
 }

modules/serialize/test/read_elements.test.ts

@@ -22,23 +22,14 @@ import { concatBuffers } from '../src/concat_buffers'
 import { Buffer } from 'buffer'
 import * as fixtures from './fixtures'
 
-describe('readElements', () => {
-  it('should be able to find one element', () => {
-    const len = Buffer.alloc(2)
-    len.writeUInt16BE(4, 0)
-    const data = new Uint8Array([...Buffer.from('data')])
-    const buff = concatBuffers(len, data)
-    const info = readElements(1, buff, 0)
-    if (info === false) throw new Error('Fail')
-    expect(info.elements).to.be.instanceof(Array)
-    expect(info.elements.length).to.eql(1)
-    expect(info.elements[0]).to.deep.equal(data)
-    expect(info.readPos).to.eql(6)
-  })
+function randomNat (limit : number) : number {
+  return Math.floor(Math.random() * limit)
+}
 
-  it('should be able to handle multiple elements', () => {
+describe('readElements', () => {
+  it('should be able to handle multiple elements containing multiple fields without padding', () => {
     const utf8DataStrings = [
-      'some utf8 information', '\u00bd + \u00bc = \u00be', 'to encode'
+      'here', 'is', 'some', 'utf8', 'information', '\u00bd + \u00bc = \u00be'
     ]
     const buffData = utf8DataStrings.map(str => new Uint8Array([...Buffer.from(str)]))
     const buff = concatBuffers(...buffData.map(bufStr => {
@@ -47,75 +38,93 @@ describe('readElements', () => {
       return concatBuffers(len, bufStr)
     }))
 
-    const info = readElements(3, buff, 0)
-
-    if (info === false) throw new Error('Fail')
-    expect(info.elements).to.be.instanceof(Array)
-    expect(info.elements.length).to.eql(3)
-    expect(info.elements[0]).to.deep.equal(buffData[0])
-    expect(info.elements[1]).to.deep.equal(buffData[1])
-    expect(info.elements[2]).to.deep.equal(buffData[2])
-    expect(Buffer.from(info.elements[0]).toString()).to.deep.equal(utf8DataStrings[0])
-    expect(Buffer.from(info.elements[1]).toString()).to.deep.equal(utf8DataStrings[1])
-    expect(Buffer.from(info.elements[2]).toString()).to.deep.equal(utf8DataStrings[2])
-    expect(info.readPos).to.eql(48)
+    /* The elements in the buffer can be arranged in several ways.
+     * For example, we can think of them as six elements with one
+     * field each, or as three elements with two fields each.
+     */
+    const dimensions = [[1, 6], [2, 3], [3, 2], [6, 1]]
+
+    dimensions.map(([elementCount, fieldsPerElement]) => {
+      const info = readElements(elementCount, fieldsPerElement, buff)
+
+      if (info === false) throw new Error('Fail')
+      let elements = info.elements
+      expect(elements).to.be.instanceof(Array)
+      expect(elements.length).to.eql(elementCount)
+
+      for (let eCount = 0; eCount < elementCount; eCount++) {
+        let element = info.elements[eCount]
+        expect(element).to.be.instanceof(Array)
+        expect(element.length).to.eql(fieldsPerElement)
+        for (let fCount = 0; fCount < fieldsPerElement; fCount++) {
+          let field = element[fCount]
+          expect(field).to.deep.equal(buffData[eCount * fieldsPerElement + fCount])
+          expect(Buffer.from(field).toString()).to.deep.equal(utf8DataStrings[eCount * fieldsPerElement + fCount])
+        }
+      }
+
+      expect(info.readPos).to.eql(buff.byteLength)
+    })
   })
 
-  it('should be able to handle multiple elements when not reading from the beginning', () => {
-    const utf8DataStrings = [
-      'some utf8 information', '\u00bd + \u00bc = \u00be', 'to encode'
-    ]
+  it('should be able to handle multiple elements containing multiple fields with various padding', () => {
+    let numberOfRuns = 16
+    const maxPaddingLength = 1024
+    const utf8DataStrings = [ 'here', 'is', 'some', 'utf8', 'information', '\u00bd + \u00bc = \u00be' ]
     const buffData = utf8DataStrings.map(str => new Uint8Array([...Buffer.from(str)]))
-    const padding = Buffer.alloc(10)
-    const buff = concatBuffers(padding, ...buffData.map(bufStr => {
+    const mainBuffer = concatBuffers(...buffData.map(bufStr => {
       const len = Buffer.alloc(2)
       len.writeUInt16BE(bufStr.byteLength, 0)
       return concatBuffers(len, bufStr)
     }))
 
-    const info = readElements(3, buff, padding.length)
-
-    if (info === false) throw new Error('Fail')
-    expect(info.elements).to.be.instanceof(Array)
-    expect(info.elements.length).to.eql(3)
-    expect(info.elements[0]).to.deep.equal(buffData[0])
-    expect(info.elements[1]).to.deep.equal(buffData[1])
-    expect(info.elements[2]).to.deep.equal(buffData[2])
-    expect(Buffer.from(info.elements[0]).toString()).to.deep.equal(utf8DataStrings[0])
-    expect(Buffer.from(info.elements[1]).toString()).to.deep.equal(utf8DataStrings[1])
-    expect(Buffer.from(info.elements[2]).toString()).to.deep.equal(utf8DataStrings[2])
-    expect(info.readPos).to.eql(58)
+    const dimensions = [[1, 6], [2, 3], [3, 2], [6, 1]]
+
+    while (numberOfRuns--) {
+      let leftPadding = Buffer.alloc(randomNat(maxPaddingLength))
+      let rightPadding = Buffer.alloc(randomNat(maxPaddingLength))
+      let buff = concatBuffers(leftPadding, mainBuffer, rightPadding)
+
+      dimensions.map(([elementCount, fieldsPerElement]) => {
+        const info = readElements(elementCount, fieldsPerElement, buff, leftPadding.byteLength)
+
+        if (info === false) throw new Error('Fail')
+        let elements = info.elements
+        expect(elements).to.be.instanceof(Array)
+        expect(elements.length).to.eql(elementCount)
+
+        for (let eCount = 0; eCount < elementCount; eCount++) {
+          let element = info.elements[eCount]
+          expect(element).to.be.instanceof(Array)
+          expect(element.length).to.eql(fieldsPerElement)
+          for (let fCount = 0; fCount < fieldsPerElement; fCount++) {
+            let field = element[fCount]
+            expect(field).to.deep.equal(buffData[eCount * fieldsPerElement + fCount])
+            expect(Buffer.from(field).toString()).to.deep.equal(utf8DataStrings[eCount * fieldsPerElement + fCount])
+          }
+        }
+
+        expect(info.readPos).to.eql(leftPadding.byteLength + mainBuffer.byteLength)
+      })
+    }
   })
 
-  it('should be able to handle multiple elements with padding on both sides', () => {
-    const utf8DataStrings = [
-      'some utf8 information', '\u00bd + \u00bc = \u00be', 'to encode'
-    ]
-    const buffData = utf8DataStrings.map(str => new Uint8Array([...Buffer.from(str)]))
-    const padding = Buffer.alloc(10)
-    const buff = concatBuffers(padding, ...buffData.map(bufStr => {
-      const len = Buffer.alloc(2)
-      len.writeUInt16BE(bufStr.byteLength, 0)
-      return concatBuffers(len, bufStr)
-    }), padding)
-
-    const info = readElements(3, buff, padding.length)
-
-    if (info === false) throw new Error('Fail')
-    expect(info.elements).to.be.instanceof(Array)
-    expect(info.elements.length).to.eql(3)
-    expect(info.elements[0]).to.deep.equal(buffData[0])
-    expect(info.elements[1]).to.deep.equal(buffData[1])
-    expect(info.elements[2]).to.deep.equal(buffData[2])
-    expect(Buffer.from(info.elements[0]).toString()).to.deep.equal(utf8DataStrings[0])
-    expect(Buffer.from(info.elements[1]).toString()).to.deep.equal(utf8DataStrings[1])
-    expect(Buffer.from(info.elements[2]).toString()).to.deep.equal(utf8DataStrings[2])
-    expect(info.readPos).to.eql(58)
+  it('Precondition: readPos must be non-negative and within the byte length of the buffer given.', () => {
+    const buff = new Uint8Array(32)
+    const readPosBeyondBuff = buff.byteLength + 1
+    expect(() => readElements(1, 1, buff, readPosBeyondBuff)).to.throw()
+  })
+
+  it('Precondition: elementCount and fieldsPerElement must be non-negative.', () => {
+    const buff = new Uint8Array(32)
+    expect(() => readElements(-1, 1, buff)).to.throw()
+    expect(() => readElements(1, -1, buff)).to.throw()
+    expect(() => readElements(-1, -1, buff)).to.throw()
   })
 
   it('Check for early return (Postcondition): Enough data must exist to read the Uint16 length value.; Check for early return (Postcondition): Enough data must exist length of the value.', () => {
     const utf8DataStrings = [
-      'some utf8 information', '\u00bd + \u00bc = \u00be', 'to encode'
+      'here', 'is', 'some', 'utf8', 'information', '\u00bd + \u00bc = \u00be'
     ]
     const buffData = utf8DataStrings.map(str => new Uint8Array([...Buffer.from(str)]))
     const buff = concatBuffers(...buffData.map(bufStr => {
@@ -124,35 +133,13 @@ describe('readElements', () => {
       return concatBuffers(len, bufStr)
     }))
 
-    // By testing every combination of byte length possible
-    // Both loop invariants are covered.
-    for (let i = 0; buff.byteLength > i; i++) {
-      const info = readElements(3, buff.slice(0, i), 0)
-      expect(info).to.eql(false)
-    }
-    // Engage pedantry...
-    const info = readElements(3, buff.slice(0, buff.byteLength), 0)
-    if (info === false) throw new Error('Fail')
-    expect(info.elements).to.be.instanceof(Array)
-    expect(info.elements.length).to.eql(3)
-    expect(info.elements[0]).to.deep.equal(buffData[0])
-    expect(info.elements[1]).to.deep.equal(buffData[1])
-    expect(info.elements[2]).to.deep.equal(buffData[2])
-    expect(Buffer.from(info.elements[0]).toString()).to.deep.equal(utf8DataStrings[0])
-    expect(Buffer.from(info.elements[1]).toString()).to.deep.equal(utf8DataStrings[1])
-    expect(Buffer.from(info.elements[2]).toString()).to.deep.equal(utf8DataStrings[2])
-    expect(info.readPos).to.eql(48)
-  })
-
-  it('Precondition: readElements readPos must be within the byte length of the buffer given.', () => {
-    const buff = new Uint8Array(32)
-    const readPosBeyondBuff = buff.byteLength + 1
-    expect(() => readElements(1, buff, readPosBeyondBuff)).to.throw()
-  })
+    /* Will return false when trying to read the length of the seventh element */
+    const infoFalse1 = readElements(1, 7, buff)
+    expect(infoFalse1).to.equal(false)
 
-  it('Precondition: elementCount must not be negative.', () => {
-    const buff = new Uint8Array(32)
-    expect(() => readElements(-1, buff)).to.throw()
+    /* Will return false when trying to read the sixth element */
+    const infoFalse2 = readElements(1, 6, buff.slice(0, buff.byteLength - 1))
+    expect(infoFalse2).to.equal(false)
   })
 
   it('ArrayBuffer for a Uint8Array or Buffer may be larger than the Uint8Array or Buffer that it is a view over is.', () => {
@@ -162,12 +149,12 @@ describe('readElements', () => {
      * read position past the length and count should succeed.
      */
     const buff = fixtures.basicEncryptionContext()
-    expect(readElements(4, buff, 0)).to.equal(false)
-    expect(readElements(4, buff, 4)).to.not.equal(false)
+    expect(readElements(4, 1, buff, 0)).to.equal(false)
+    expect(readElements(4, 1, buff, 4)).to.not.equal(false)
     /* Given this I can use this to construct a new view of part of the
      * ArrayBuffer to simulate a large ArrayBuffer that is sliced
      * into parts for efficiency. */
     const sharingArrayBuffer = new Uint8Array(buff.buffer, 4, buff.byteLength - 4)
-    expect(readElements(4, sharingArrayBuffer)).to.not.equal(false)
+    expect(readElements(4, 1, sharingArrayBuffer)).to.not.equal(false)
   })
 })