fix: do not alter an event's data attribute (#344)

* fix: do not alter an event's data attribute

When setting an event's data attribute we were trying to be really clever
and this is problematic. Instead, keep the data attribute unchanged. Per
the 1.0 specification, the data attribute is still inspected to determine
if it is binary, and if so, a data_base64 attribute is added with the
contents of the data property encoded as base64.

Fixes: https://github.com/cloudevents/sdk-javascript/issues/343

Signed-off-by: Lance Ball <lball@redhat.com>

src/event/cloudevent.ts

@@ -10,8 +10,6 @@ import {
 } from "./interfaces";
 import { validateCloudEvent } from "./spec";
 import { ValidationError, isBinary, asBase64, isValidType } from "./validation";
-import CONSTANTS from "../constants";
-import { isString } from "util";
 
 /**
  * An enum representing the CloudEvent specification version
@@ -92,7 +90,7 @@ export class CloudEvent implements CloudEventV1, CloudEventV03 {
     this.schemaurl = properties.schemaurl as string;
     delete properties.schemaurl;
 
-    this._setData(properties.data);
+    this.data = properties.data;
     delete properties.data;
 
     // sanity checking
@@ -125,25 +123,11 @@ export class CloudEvent implements CloudEventV1, CloudEventV03 {
   }
 
   get data(): unknown {
-    if (
-      this.datacontenttype === CONSTANTS.MIME_JSON &&
-      !(this.datacontentencoding === CONSTANTS.ENCODING_BASE64) &&
-      isString(this.#_data)
-    ) {
-      return JSON.parse(this.#_data as string);
-    } else if (isBinary(this.#_data)) {
-      return asBase64(this.#_data as Uint32Array);
-    }
     return this.#_data;
   }
 
   set data(value: unknown) {
-    this._setData(value);
-  }
-
-  private _setData(value: unknown): void {
     if (isBinary(value)) {
-      this.#_data = value;
       this.data_base64 = asBase64(value as Uint32Array);
     }
     this.#_data = value;
@@ -158,7 +142,7 @@ export class CloudEvent implements CloudEventV1, CloudEventV03 {
   toJSON(): Record<string, unknown> {
     const event = { ...this };
     event.time = new Date(this.time as string).toISOString();
-    event.data = this.data;
+    event.data = !isBinary(this.data) ? this.data : undefined;
     return event;
   }
 
@@ -175,7 +159,6 @@ export class CloudEvent implements CloudEventV1, CloudEventV03 {
     try {
       return validateCloudEvent(this);
     } catch (e) {
-      console.error(e.errors);
       if (e instanceof ValidationError) {
         throw e;
       } else {

src/message/http/index.ts

@@ -2,16 +2,17 @@ import { CloudEvent, CloudEventV03, CloudEventV1, CONSTANTS, Mode, Version } fro
 import { Message, Headers } from "..";
 
 import { headersFor, sanitize, v03structuredParsers, v1binaryParsers, v1structuredParsers } from "./headers";
-import { asData, isBase64, isString, isStringOrObjectOrThrow, ValidationError } from "../../event/validation";
+import { isStringOrObjectOrThrow, ValidationError } from "../../event/validation";
-import { Base64Parser, JSONParser, MappedParser, Parser, parserByContentType } from "../../parsers";
+import { JSONParser, MappedParser, Parser, parserByContentType } from "../../parsers";
 
 // implements Serializer
 export function binary(event: CloudEvent): Message {
   const contentType: Headers = { [CONSTANTS.HEADER_CONTENT_TYPE]: CONSTANTS.DEFAULT_CONTENT_TYPE };
   const headers: Headers = { ...contentType, ...headersFor(event) };
-  let body = asData(event.data, event.datacontenttype as string);
+  let body = event.data;
-  if (typeof body === "object") {
+  if (typeof event.data === "object" && !(event.data instanceof Uint32Array)) {
-    body = JSON.stringify(body);
+    // we'll stringify objects, but not binary data
+    body = JSON.stringify(event.data);
   }
   return {
     headers,
@@ -21,6 +22,10 @@ export function binary(event: CloudEvent): Message {
 
 // implements Serializer
 export function structured(event: CloudEvent): Message {
+  if (event.data_base64) {
+    // The event's data is binary - delete it
+    event = event.cloneWith({ data: undefined });
+  }
   return {
     headers: {
       [CONSTANTS.HEADER_CONTENT_TYPE]: CONSTANTS.DEFAULT_CE_CONTENT_TYPE,
@@ -89,7 +94,7 @@ function getMode(headers: Headers): Mode {
  * @param {Record<string, unknown>} body the HTTP request body
  * @returns {Version} the CloudEvent specification version
  */
-function getVersion(mode: Mode, headers: Headers, body: string | Record<string, string>) {
+function getVersion(mode: Mode, headers: Headers, body: string | Record<string, string> | unknown) {
   if (mode === Mode.BINARY) {
     // Check the headers for the version
     const versionHeader = headers[CONSTANTS.CE_HEADERS.SPEC_VERSION];
@@ -129,8 +134,6 @@ function parseBinary(message: Message, version: Version): CloudEvent {
     throw new ValidationError(`invalid spec version ${headers[CONSTANTS.CE_HEADERS.SPEC_VERSION]}`);
   }
 
-  body = isString(body) && isBase64(body) ? Buffer.from(body as string, "base64").toString() : body;
-
   // Clone and low case all headers names
   const sanitizedHeaders = sanitize(headers);
 
@@ -145,22 +148,18 @@ function parseBinary(message: Message, version: Version): CloudEvent {
     }
   }
 
-  let parsedPayload;
-
-  if (body) {
-    const parser = parserByContentType[eventObj.datacontenttype as string];
-    if (!parser) {
-      throw new ValidationError(`no parser found for content type ${eventObj.datacontenttype}`);
-    }
-    parsedPayload = parser.parse(body);
-  }
-
   // Every unprocessed header can be an extension
   for (const header in sanitizedHeaders) {
     if (header.startsWith(CONSTANTS.EXTENSIONS_PREFIX)) {
       eventObj[header.substring(CONSTANTS.EXTENSIONS_PREFIX.length)] = headers[header];
     }
   }
+
+  const parser = parserByContentType[eventObj.datacontenttype as string];
+  if (parser && body) {
+    body = parser.parse(body as string);
+  }
+
   // At this point, if the datacontenttype is application/json and the datacontentencoding is base64
   // then the data has already been decoded as a string, then parsed as JSON. We don't need to have
   // the datacontentencoding property set - in fact, it's incorrect to do so.
@@ -168,7 +167,7 @@ function parseBinary(message: Message, version: Version): CloudEvent {
     delete eventObj.datacontentencoding;
   }
 
-  return new CloudEvent({ ...eventObj, data: parsedPayload } as CloudEventV1 | CloudEventV03, false);
+  return new CloudEvent({ ...eventObj, data: body } as CloudEventV1 | CloudEventV03, false);
 }
 
 /**
@@ -201,7 +200,7 @@ function parseStructured(message: Message, version: Version): CloudEvent {
   const contentType = sanitizedHeaders[CONSTANTS.HEADER_CONTENT_TYPE];
   const parser: Parser = contentType ? parserByContentType[contentType] : new JSONParser();
   if (!parser) throw new ValidationError(`invalid content type ${sanitizedHeaders[CONSTANTS.HEADER_CONTENT_TYPE]}`);
-  const incoming = { ...(parser.parse(payload) as Record<string, unknown>) };
+  const incoming = { ...(parser.parse(payload as string) as Record<string, unknown>) };
 
   const eventObj: { [key: string]: unknown } = {};
   const parserMap: Record<string, MappedParser> = version === Version.V1 ? v1structuredParsers : v03structuredParsers;
@@ -220,10 +219,12 @@ function parseStructured(message: Message, version: Version): CloudEvent {
     eventObj[key] = incoming[key];
   }
 
-  // ensure data content is correctly decoded
+  // data_base64 is a property that only exists on V1 events. For V03 events,
-  if (eventObj.data_base64) {
+  // there will be a .datacontentencoding property, and the .data property
-    const parser = new Base64Parser();
+  // itself will be encoded as base64
-    eventObj.data = JSON.parse(parser.parse(eventObj.data_base64 as string));
+  if (eventObj.data_base64 || eventObj.datacontentencoding === CONSTANTS.ENCODING_BASE64) {
+    const data = eventObj.data_base64 || eventObj.data;
+    eventObj.data = new Uint32Array(Buffer.from(data as string, "base64"));
     delete eventObj.data_base64;
     delete eventObj.datacontentencoding;
   }

src/message/index.ts

@@ -28,7 +28,7 @@ export interface Headers {
  */
 export interface Message {
   headers: Headers;
-  body: string;
+  body: string | unknown;
 }
 
 /**

src/transport/receiver.ts

@@ -17,7 +17,7 @@ export const Receiver = {
    */
   accept(headers: Headers, body: string | Record<string, unknown> | undefined | null): CloudEvent {
     const cleanHeaders: Headers = sanitize(headers);
-    const cleanBody = body ? (typeof body === "object" ? JSON.stringify(body) : body) : "";
+    const cleanBody = body ? (typeof body === "object" ? JSON.stringify(body) : body) : undefined;
     const message: Message = {
       headers: cleanHeaders,
       body: cleanBody,

test/integration/cloud_event_test.ts

@@ -1,6 +1,10 @@
+import path from "path";
+import fs from "fs";
+
 import { expect } from "chai";
 import { CloudEvent, ValidationError, Version } from "../../src";
 import { CloudEventV03, CloudEventV1 } from "../../src/event/interfaces";
+import { asBase64 } from "../../src/event/validation";
 
 const type = "org.cncf.cloudevents.example";
 const source = "http://unit.test";
@@ -14,6 +18,9 @@ const fixture: CloudEventV1 = {
   data: `"some data"`,
 };
 
+const imageData = new Uint32Array(fs.readFileSync(path.join(process.cwd(), "test", "integration", "ce.png")));
+const image_base64 = asBase64(imageData);
+
 describe("A CloudEvent", () => {
   it("Can be constructed with a typed Message", () => {
     const ce = new CloudEvent(fixture);
@@ -151,6 +158,15 @@ describe("A 1.0 CloudEvent", () => {
     expect(ce.data).to.be.true;
   });
 
+  it("can be constructed with binary data", () => {
+    const ce = new CloudEvent({
+      ...fixture,
+      data: imageData,
+    });
+    expect(ce.data).to.equal(imageData);
+    expect(ce.data_base64).to.equal(image_base64);
+  });
+
   it("can be constructed with extensions", () => {
     const extensions = {
       extensionkey: "extension-value",

test/integration/emitter_factory_test.ts

@@ -50,12 +50,12 @@ function superagentEmitter(message: Message, options?: Options): Promise<unknown
   for (const key of Object.getOwnPropertyNames(message.headers)) {
     post.set(key, message.headers[key]);
   }
-  return post.send(message.body);
+  return post.send(message.body as string);
 }
 
 function gotEmitter(message: Message, options?: Options): Promise<unknown> {
   return Promise.resolve(
-    got.post(sink, { headers: message.headers, body: message.body, ...((options as unknown) as Options) }),
+    got.post(sink, { headers: message.headers, body: message.body as string, ...((options as unknown) as Options) }),
   );
 }
 

test/integration/message_test.ts

@@ -1,3 +1,6 @@
+import path from "path";
+import fs from "fs";
+
 import { expect } from "chai";
 import { CloudEvent, CONSTANTS, Version } from "../../src";
 import { asBase64 } from "../../src/event/validation";
@@ -16,7 +19,6 @@ const data = {
 
 // Attributes for v03 events
 const schemaurl = "https://cloudevents.io/schema.json";
-const datacontentencoding = "base64";
 
 const ext1Name = "extension1";
 const ext1Value = "foobar";
@@ -27,6 +29,11 @@ const ext2Value = "acme";
 const dataBinary = Uint32Array.from(JSON.stringify(data), (c) => c.codePointAt(0) as number);
 const data_base64 = asBase64(dataBinary);
 
+// Since the above is a special case (string as binary), let's test
+// with a real binary file one is likely to encounter in the wild
+const imageData = new Uint32Array(fs.readFileSync(path.join(process.cwd(), "test", "integration", "ce.png")));
+const image_base64 = asBase64(imageData);
+
 describe("HTTP transport", () => {
   it("Can detect invalid CloudEvent Messages", () => {
     // Create a message that is not an actual event
@@ -45,6 +52,7 @@ describe("HTTP transport", () => {
       new CloudEvent({
         source: "/message-test",
         type: "example",
+        data,
       }),
     );
     expect(HTTP.isEvent(message)).to.be.true;
@@ -102,7 +110,7 @@ describe("HTTP transport", () => {
 
     it("Binary Messages can be created from a CloudEvent", () => {
       const message: Message = HTTP.binary(fixture);
-      expect(JSON.parse(message.body)).to.deep.equal(data);
+      expect(message.body).to.equal(JSON.stringify(data));
       // validate all headers
       expect(message.headers[CONSTANTS.HEADER_CONTENT_TYPE]).to.equal(datacontenttype);
       expect(message.headers[CONSTANTS.CE_HEADERS.SPEC_VERSION]).to.equal(Version.V1);
@@ -120,7 +128,7 @@ describe("HTTP transport", () => {
       const message: Message = HTTP.structured(fixture);
       expect(message.headers[CONSTANTS.HEADER_CONTENT_TYPE]).to.equal(CONSTANTS.DEFAULT_CE_CONTENT_TYPE);
       // Parse the message body as JSON, then validate the attributes
-      const body = JSON.parse(message.body);
+      const body = JSON.parse(message.body as string);
       expect(body[CONSTANTS.CE_ATTRIBUTES.SPEC_VERSION]).to.equal(Version.V1);
       expect(body[CONSTANTS.CE_ATTRIBUTES.ID]).to.equal(id);
       expect(body[CONSTANTS.CE_ATTRIBUTES.TYPE]).to.equal(type);
@@ -144,20 +152,47 @@ describe("HTTP transport", () => {
       expect(event).to.deep.equal(fixture);
     });
 
-    it("Supports Base-64 encoded data in structured messages", () => {
+    it("Converts binary data to base64 when serializing structured messages", () => {
-      const event = fixture.cloneWith({ data: dataBinary });
+      const event = fixture.cloneWith({ data: imageData, datacontenttype: "image/png" });
-      expect(event.data_base64).to.equal(data_base64);
+      expect(event.data).to.equal(imageData);
       const message = HTTP.structured(event);
-      const eventDeserialized = HTTP.toEvent(message);
+      const messageBody = JSON.parse(message.body as string);
-      expect(eventDeserialized.data).to.deep.equal({ foo: "bar" });
+      expect(messageBody.data_base64).to.equal(image_base64);
     });
 
-    it("Supports Base-64 encoded data in binary messages", () => {
+    it("Converts base64 encoded data to binary when deserializing structured messages", () => {
-      const event = fixture.cloneWith({ data: dataBinary });
+      const message = HTTP.structured(fixture.cloneWith({ data: imageData, datacontenttype: "image/png" }));
-      expect(event.data_base64).to.equal(data_base64);
-      const message = HTTP.binary(event);
       const eventDeserialized = HTTP.toEvent(message);
-      expect(eventDeserialized.data).to.deep.equal({ foo: "bar" });
+      expect(eventDeserialized.data).to.deep.equal(imageData);
+      expect(eventDeserialized.data_base64).to.equal(image_base64);
+    });
+
+    it("Does not parse binary data from structured messages with content type application/json", () => {
+      const message = HTTP.structured(fixture.cloneWith({ data: dataBinary }));
+      const eventDeserialized = HTTP.toEvent(message);
+      expect(eventDeserialized.data).to.deep.equal(dataBinary);
+      expect(eventDeserialized.data_base64).to.equal(data_base64);
+    });
+
+    it("Converts base64 encoded data to binary when deserializing binary messages", () => {
+      const message = HTTP.binary(fixture.cloneWith({ data: imageData, datacontenttype: "image/png" }));
+      const eventDeserialized = HTTP.toEvent(message);
+      expect(eventDeserialized.data).to.deep.equal(imageData);
+      expect(eventDeserialized.data_base64).to.equal(image_base64);
+    });
+
+    it("Keeps binary data binary when serializing binary messages", () => {
+      const event = fixture.cloneWith({ data: dataBinary });
+      expect(event.data).to.equal(dataBinary);
+      const message = HTTP.binary(event);
+      expect(message.body).to.equal(dataBinary);
+    });
+
+    it("Does not parse binary data from binary messages with content type application/json", () => {
+      const message = HTTP.binary(fixture.cloneWith({ data: dataBinary }));
+      const eventDeserialized = HTTP.toEvent(message);
+      expect(eventDeserialized.data).to.deep.equal(dataBinary);
+      expect(eventDeserialized.data_base64).to.equal(data_base64);
     });
   });
 
@@ -196,7 +231,7 @@ describe("HTTP transport", () => {
       const message: Message = HTTP.structured(fixture);
       expect(message.headers[CONSTANTS.HEADER_CONTENT_TYPE]).to.equal(CONSTANTS.DEFAULT_CE_CONTENT_TYPE);
       // Parse the message body as JSON, then validate the attributes
-      const body = JSON.parse(message.body);
+      const body = JSON.parse(message.body as string);
       expect(body[CONSTANTS.CE_ATTRIBUTES.SPEC_VERSION]).to.equal(Version.V03);
       expect(body[CONSTANTS.CE_ATTRIBUTES.ID]).to.equal(id);
       expect(body[CONSTANTS.CE_ATTRIBUTES.TYPE]).to.equal(type);
@@ -220,20 +255,35 @@ describe("HTTP transport", () => {
       expect(event).to.deep.equal(fixture);
     });
 
-    it("Supports Base-64 encoded data in structured messages", () => {
+    it("Converts binary data to base64 when serializing structured messages", () => {
-      const event = fixture.cloneWith({ data: dataBinary, datacontentencoding });
+      const event = fixture.cloneWith({ data: imageData, datacontenttype: "image/png" });
-      expect(event.data_base64).to.equal(data_base64);
+      expect(event.data).to.equal(imageData);
       const message = HTTP.structured(event);
-      const eventDeserialized = HTTP.toEvent(message);
+      const messageBody = JSON.parse(message.body as string);
-      expect(eventDeserialized.data).to.deep.equal({ foo: "bar" });
+      expect(messageBody.data_base64).to.equal(image_base64);
     });
 
-    it("Supports Base-64 encoded data in binary messages", () => {
+    it("Converts base64 encoded data to binary when deserializing structured messages", () => {
-      const event = fixture.cloneWith({ data: dataBinary, datacontentencoding });
+      // Creating an event with binary data automatically produces base64 encoded data
-      expect(event.data_base64).to.equal(data_base64);
+      // which is then set as the 'data' attribute on the message body
-      const message = HTTP.binary(event);
+      const message = HTTP.structured(fixture.cloneWith({ data: imageData, datacontenttype: "image/png" }));
       const eventDeserialized = HTTP.toEvent(message);
-      expect(eventDeserialized.data).to.deep.equal({ foo: "bar" });
+      expect(eventDeserialized.data).to.deep.equal(imageData);
+      expect(eventDeserialized.data_base64).to.equal(image_base64);
+    });
+
+    it("Converts base64 encoded data to binary when deserializing binary messages", () => {
+      const message = HTTP.binary(fixture.cloneWith({ data: imageData, datacontenttype: "image/png" }));
+      const eventDeserialized = HTTP.toEvent(message);
+      expect(eventDeserialized.data).to.deep.equal(imageData);
+      expect(eventDeserialized.data_base64).to.equal(image_base64);
+    });
+
+    it("Keeps binary data binary when serializing binary messages", () => {
+      const event = fixture.cloneWith({ data: dataBinary });
+      expect(event.data).to.equal(dataBinary);
+      const message = HTTP.binary(event);
+      expect(message.body).to.equal(dataBinary);
     });
   });
 });

test/integration/spec_03_tests.ts

@@ -168,12 +168,6 @@ describe("CloudEvents Spec v0.3", () => {
 
         expect(typeof cloudevent.data).to.equal("string");
       });
-
-      it("should convert data with stringified json to a json object", () => {
-        cloudevent = cloudevent.cloneWith({ datacontenttype: Constants.MIME_JSON });
-        cloudevent.data = JSON.stringify(data);
-        expect(cloudevent.data).to.deep.equal(data);
-      });
     });
 
     describe("'subject'", () => {

test/integration/spec_1_tests.ts

@@ -168,11 +168,6 @@ describe("CloudEvents Spec v1.0", () => {
       cloudevent = cloudevent.cloneWith({ datacontenttype: dct });
     });
 
-    it("should convert data with stringified json to a json object", () => {
-      cloudevent = cloudevent.cloneWith({ datacontenttype: Constants.MIME_JSON, data: JSON.stringify(data) });
-      expect(cloudevent.data).to.deep.equal(data);
-    });
-
     it("should be ok when type is 'Uint32Array' for 'Binary'", () => {
       const dataString = ")(*~^my data for ce#@#$%";