fix: make application/json the default content type in binary mode #118
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 <lball@redhat.com>
grant
approved these changes
May 4, 2020
helio-frota
approved these changes
May 5, 2020
grant
pushed a commit
to grant/sdk-javascript
that referenced
this pull request
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 <lball@redhat.com> Signed-off-by: Grant Timmerman <timmerman+devrel@google.com>
grant
added a commit
to grant/sdk-javascript
that referenced
this pull request
May 6, 2020
… mode (cloudevents#118)" This reverts commit 9ccfaf2.
grant
added a commit
to grant/sdk-javascript
that referenced
this pull request
May 6, 2020
… mode (cloudevents#118)" This reverts commit 9ccfaf2. Signed-off-by: Grant Timmerman <timmerman+devrel@google.com>
grant
added a commit
that referenced
this pull request
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 <lball@redhat.com> Signed-off-by: Grant Timmerman <timmerman+devrel@google.com> * refactor: remove ext folder Signed-off-by: Grant Timmerman <timmerman+devrel@google.com> * Revert "fix: make application/json the default content type in binary mode (#118)" This reverts commit 9ccfaf2. Signed-off-by: Grant Timmerman <timmerman+devrel@google.com> Co-authored-by: Lance Ball <lball@redhat.com>
lance
added a commit
to lance/sdk-javascript
that referenced
this pull request
May 7, 2020
The fix provided in cloudevents#118 only included tests for `receiver.check()`, and the change in that case was to add the `application/json` content type to the cleansed headers if to type was specified. However, `receiver.parse()` did not receive the benefit of this change. It calls `this.check()` but then sanitizes the original headers again, and the missing content-type was not re-inserted into the newly sanitized headers. This commit, modifies the code so that `receiver.check()` does not insert the content-type, but does allow the validation check to pass if no content-type header exists. When `receiver.parse()` is called, and the headers are sanitized again - and this time used to look up parser implementation, the default `application/json` content-is applied if no content-type header exists. I've also removed a redundant call to `receiver.check()` in receiver_binary_1.js and simplified the usage of `Constants` in the test. Signed-off-by: Lance Ball <lball@redhat.com>
lance
added a commit
that referenced
this pull request
May 9, 2020
* fix: ensure binary events can handle no content-type header The fix provided in #118 only included tests for `receiver.check()`, and the change in that case was to add the `application/json` content type to the cleansed headers if to type was specified. However, `receiver.parse()` did not receive the benefit of this change. It calls `this.check()` but then sanitizes the original headers again, and the missing content-type was not re-inserted into the newly sanitized headers. This commit, modifies the code so that `receiver.check()` does not insert the content-type, but does allow the validation check to pass if no content-type header exists. When `receiver.parse()` is called, and the headers are sanitized again - and this time used to look up parser implementation, the default `application/json` content-is applied if no content-type header exists. I've also removed a redundant call to `receiver.check()` in receiver_binary_1.js and simplified the usage of `Constants` in the test. Signed-off-by: Lance Ball <lball@redhat.com> * chore: clean up header sniffing Signed-off-by: Lance Ball <lball@redhat.com>
lance
added a commit
to lance/sdk-javascript
that referenced
this pull request
May 9, 2020
…nts#134) * fix: ensure binary events can handle no content-type header The fix provided in cloudevents#118 only included tests for `receiver.check()`, and the change in that case was to add the `application/json` content type to the cleansed headers if to type was specified. However, `receiver.parse()` did not receive the benefit of this change. It calls `this.check()` but then sanitizes the original headers again, and the missing content-type was not re-inserted into the newly sanitized headers. This commit, modifies the code so that `receiver.check()` does not insert the content-type, but does allow the validation check to pass if no content-type header exists. When `receiver.parse()` is called, and the headers are sanitized again - and this time used to look up parser implementation, the default `application/json` content-is applied if no content-type header exists. I've also removed a redundant call to `receiver.check()` in receiver_binary_1.js and simplified the usage of `Constants` in the test. Signed-off-by: Lance Ball <lball@redhat.com> * chore: clean up header sniffing Signed-off-by: Lance Ball <lball@redhat.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Knative Kafka event source does not include a
Content-Typeheader whensending binary events. The CE HTTP binding specification doesn't address how
a receiver should handle this situation.
This commit makes
application/jsonthe default.Fixes: #117
Ref: cloudevents/spec#614
Signed-off-by: Lance Ball lball@redhat.com