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: #343

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

Author: lance

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 { Base64Parser, JSONParser, MappedParser, Parser, parserByContentType } from "../../parsers";
+import { isStringOrObjectOrThrow, ValidationError } from "../../event/validation";
+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);
-  if (typeof body === "object") {
-    body = JSON.stringify(body);
+  let body = event.data;
+  if (typeof event.data === "object" && !(event.data instanceof Uint32Array)) {
+    // 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,30 +148,26 @@ 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.
   if (eventObj.datacontenttype === CONSTANTS.MIME_JSON && eventObj.datacontentencoding === CONSTANTS.ENCODING_BASE64) {
     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
-  if (eventObj.data_base64) {
-    const parser = new Base64Parser();
-    eventObj.data = JSON.parse(parser.parse(eventObj.data_base64 as string));
+  // data_base64 is a property that only exists on V1 events. For V03 events,
+  // there will be a .datacontentencoding property, and the .data property
+  // itself will be encoded as base64
+  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/ce.png

@@ -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/cloud_event_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) }),
   );
 }