Skip to content

Binary data in binary format is not binary? #343

Closed

Description

@evankanderson

Describe the Bug
I'm trying to send a cloudevent with datacontenttype of image/jpeg and the actual payload being the bytes of the image. It seems like the current code in asData will always base-64 the content when fetching data. (Possibly twice, if I read the code at line 12 of http/index.js correctly -- once for event.data, and once for asData.)

Steps to Reproduce
I'd like the following code to work, but it appears that ArrayBuffer and Blob are not supported types for data:

  fetch(mediaUrl).then(async (resp) => {
    let newEvent = event.cloneWith({
      source: sourceUrl,
      type: responseEventType,
      datacontenttype: resp.headers.get("Content-Type")
    })
    // I want this. This definitely does not work, and an empty object gets encoded.
    newEvent.data = await resp.arrayBuffer();  // or .blob()
    const response = HTTP.binary(newEvent);
    res.status(200).set(response.headers).send(response.body);
  });

If I try to convert the arrayBuffer to a Uint32Array (newEvent.data = new Uint32Array(await resp.arrayBuffer());), I get the following exception:

TypeError: Cannot assign to read only property 'data_base64' of object '[object Object]'

Expected Behavior
I'd like the above code to work, or to be able to find an example of sending a binary file (i.e. an image or a proto or something) as an HTTP CloudEvent.

Additional context
I am terrible at Javascript, so feel free to point out I'm holding it wrong.

Activity

  1. lance commented on Sep 28, 2020

    @lance
    Member

    Hi @evankanderson can you please try the data assignment in the cloneWith() method?

    let newEvent = event.cloneWith({
          source: sourceUrl,
          type: responseEventType,
          datacontenttype: resp.headers.get("Content-Type"),
          data: new Uint32Array(await resp.arrayBuffer())
        });

    This will get around the assignment errors. But you are correct that when serializing an event, it will be encoded as base64. This is based on a reading of the JSON format spec.

    If the implementation determines that the type of data is Binary, the value MUST be represented as a JSON string expression containing the Base64 encoded binary value, and use the member name data_base64 to store it inside the JSON object.

    The assumption made in this module is that when serializing for HTTP, the event is converted into a JSON representation, and that data should be encoded as base64. But your use case clearly shows that this should not happen.

  2. self-assigned this
    on Sep 28, 2020
  3. added
    module/libRelated to the main source code
    module/transport/httpIssues related to the HTTP transport protocol implementation
    type/bugSomething isn't working
    version/4.xIssues related to the 4.0 release of this library
    and removed
    version/4.xIssues related to the 4.0 release of this library
    on Sep 28, 2020
  4. evankanderson commented on Sep 28, 2020

    @evankanderson
    Author

    (I'm trying to avoid the overheads of base64 in my image pipeline, and my next step was expecting an image in the POST body.)

    Thanks for the suggestion on assigning inside cloneWith, that seems to work. In general, it would be nice if I could assign a ReadableStream to data for the binary case, but I realze that's probably a larger feature request.

  5. lance commented on Sep 28, 2020

    @lance
    Member

    Thanks for the suggestion on assigning inside cloneWith, that seems to work.

    Sure thing. The idea behind cloneWith is that an event should be immutable. So that's why you were getting that particular error.

    it would be nice if I could assign a ReadableStream to data for the binary case, but I realze that's probably a larger feature request.

    It is. But I should have a fix soon so that the data attribute is unmodified.

  6. lance commented on Sep 29, 2020

    @lance
    Member

    Hi @evankanderson - I have a fix in this PR #344. If you can, would you mind giving it a try with your use case?

  7. lance commented on Sep 29, 2020

    @lance
    Member

    it would be nice if I could assign a ReadableStream to data for the binary case

    I think you should be able to do that with the changes in #344, but you could not convert it to a Message. You would need to do something like this.

      fetch(mediaUrl).then(async (resp) => {
        const newEvent = new CloudEvent({
          source: sourceUrl,
          type: responseEventType,
          datacontenttype: resp.headers.get("Content-Type"),
          data: resp.body
        })
        // Do whatever you need to do to read newEvent.data as a ReadableStream,
        // eventually setting a variable with the resultant value
        const data = ...
        const response = HTTP.binary(
          newEvent.cloneWith( { data } );
        );
        res.status(200).set(response.headers).send(response.body);
      });
  8. evankanderson commented on Sep 29, 2020

    @evankanderson
    Author

    I have a demo that I'm wrapping up, but I'll try figuring out how to patch this in on a branch probably tomorrow morning once I've got the demo recorded. 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

module/libRelated to the main source codemodule/transport/httpIssues related to the HTTP transport protocol implementationtype/bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions