Skip to content

Support for lack of content-type header in binary mode parsing #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
aliok opened this issue May 4, 2020 · 3 comments · Fixed by #118
Closed

Support for lack of content-type header in binary mode parsing #117

aliok opened this issue May 4, 2020 · 3 comments · Fixed by #118
Assignees
Labels
type/bug Something isn't working version/1.x.backport Changes that should be backported to the 1.x release line version/1.x Issues related to version 1.x of this library

Comments

@aliok
Copy link
Contributor

aliok commented May 4, 2020

I have a very simple binary receiver:

const v1 = require("cloudevents-sdk/v1");
const receiver = new v1.BinaryHTTPReceiver();

const express = require('express')
const app = express()
const port = 8080

app.use((req, res, next) => {
    let data = "";

    //req.setEncoding("utf8");
    req.on("data", function(chunk) {
        data += chunk;
    });

    req.on("end", function() {
        req.body = data;
        next();
    });
});

app.post("/", function(req, res) {
    console.log("HEADERS===========");
    console.log(req.headers);
    console.log("BODY===========");
    console.log(req.body);

    try {
        const myevent = receiver.parse(req.body, req.headers);
        // pretty print
        console.log(new Date() +  " Accepted event. Type: " + myevent.getType() + ", data: " + JSON.stringify(myevent.getData(), null));

        res.status(201).json(myevent.format());
    } catch (err) {
        console.log("ERROR===========");
        console.error(err);
        console.log("===========");
        res.status(415)
            .header("Content-Type", "application/json")
            .send(JSON.stringify(err));
    }
});

app.listen(port, () => console.log(`Example app listening at http://localhost:${port}`))

When I send a request without Content-Type header, I see an error in the logs:

curl -X POST \
     -d'{"hello":"world"}' \
     -H'Content-Type:' \
     -H'ce-specversion:1.0' \
     -H'ce-type:com.github.pull.create' \
     -H'ce-source:https://github.com/cloudevents/spec/pull/123' \
     -H'ce-id:45c83279-c8a1-4db6-a703-b3768db93887' \
     -H'ce-time:2019-11-06T11:17:00Z' \
     http://localhost:8080

Logs:

HEADERS===========
{
  host: 'localhost:8080',
  'user-agent': 'curl/7.66.0',
  accept: '*/*',
  'ce-specversion': '1.0',
  'ce-type': 'com.github.pull.create',
  'ce-source': 'https://github.com/cloudevents/spec/pull/123',
  'ce-id': '45c83279-c8a1-4db6-a703-b3768db93887',
  'ce-time': '2019-11-06T11:17:00Z',
  'content-length': '17'
}
BODY===========
{"hello":"world"}
ERROR===========
{ message: 'invalid content type', errors: [ undefined ] }
===========

I understand that this is a supported case in Golang SDK as lack of content type means application/json (maybe I am wrong).
Although I couldn't find anything like that in the CloudEvents spec.

My use case is, that I have a Knative Kafka event source that's sending binary cloudevents without any Content-Type set.

@aliok
Copy link
Contributor Author

aliok commented May 4, 2020

cc @lance @slinkydeveloper

@lance lance self-assigned this May 4, 2020
@lance lance added type/bug Something isn't working version/1.x.backport Changes that should be backported to the 1.x release line version/1.x Issues related to version 1.x of this library labels May 4, 2020
@lance
Copy link
Member

lance commented May 4, 2020

@aliok you're right - the spec doesn't address how to handle an HTTP bound event without a Content-Type header in binary mode: https://github.com/cloudevents/spec/blob/v1.0/http-protocol-binding.md#311-http-content-type. I opened an issue on the spec repository about this and will apply a fix here.

lance added a commit to lance/sdk-javascript that referenced this issue May 4, 2020
The Knative Kafka event source does not include a `Content-Type` header when
sending binary events. The CE HTTP binding specification doesn't address how
a receiver should handle this situation.

This commit makes `application/json` the default.

Fixes: cloudevents#117
Ref: cloudevents/spec#614

Signed-off-by: Lance Ball <[email protected]>
@lance lance closed this as completed in #118 May 5, 2020
lance added a commit that referenced this issue May 5, 2020
)

The Knative Kafka event source does not include a `Content-Type` header when
sending binary events. The CE HTTP binding specification doesn't address how
a receiver should handle this situation.

This commit makes `application/json` the default.

Fixes: #117
Ref: cloudevents/spec#614

Signed-off-by: Lance Ball <[email protected]>
@aliok
Copy link
Contributor Author

aliok commented May 5, 2020

Thanks @lance

grant pushed a commit to grant/sdk-javascript that referenced this issue May 6, 2020
…loudevents#118)

The Knative Kafka event source does not include a `Content-Type` header when
sending binary events. The CE HTTP binding specification doesn't address how
a receiver should handle this situation.

This commit makes `application/json` the default.

Fixes: cloudevents#117
Ref: cloudevents/spec#614

Signed-off-by: Lance Ball <[email protected]>
Signed-off-by: Grant Timmerman <[email protected]>
grant added a commit that referenced this issue May 6, 2020
* fix: make application/json the default content type in binary mode (#118)

The Knative Kafka event source does not include a `Content-Type` header when
sending binary events. The CE HTTP binding specification doesn't address how
a receiver should handle this situation.

This commit makes `application/json` the default.

Fixes: #117
Ref: cloudevents/spec#614

Signed-off-by: Lance Ball <[email protected]>
Signed-off-by: Grant Timmerman <[email protected]>

* refactor: remove ext folder

Signed-off-by: Grant Timmerman <[email protected]>

* Revert "fix: make application/json the default content type in binary mode (#118)"

This reverts commit 9ccfaf2.

Signed-off-by: Grant Timmerman <[email protected]>

Co-authored-by: Lance Ball <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something isn't working version/1.x.backport Changes that should be backported to the 1.x release line version/1.x Issues related to version 1.x of this library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants