cloudevents.http.from_http binary incorrect error messages

[klueken]

Expected Behavior

When using cloudevents.http.from_http(headers, body), an event that is missing any of the required fields should result in a cloud_exceptions.MissingRequiredFields exception with a message that indicates which field is missing.

Actual Behavior

When given a Binary Cloud Event that is missing a required field ce-id, ce-source, or ce-type, it will return a MissingRequiredFields error with the incorrect error message Failed to find specversion in HTTP request.

Steps to Reproduce the Problem

from cloudevents.http import from_http
from cloudevents.exceptions import MissingRequiredFields

# Correctly does not result in an error if all required fields are present
def test_from_http():
    event = from_http({"ce-specversion": "1.0", "ce-id":"123", "ce-type": "test-type", "ce-source": "test-source"}, "{}")
    assert event["id"] == "123"

# Returns an incorrect error message
def test_from_http_missing_id_binary():
    try:
        event = from_http({"ce-specversion": "1.0", "ce-type": "test-type", "ce-source": "test-source"}, "{}")
        assert 1 == 2
    except MissingRequiredFields as e:
        assert "Failed to find specversion in HTTP request" == str(e)

# Returns the appropriate message
def test_from_http_missing_id_structured():
    try:
        event = from_http({}, "{\"specversion\": \"1.0\", \"type\": \"test-type\", \"source\": \"test-source\"}")
        assert 1 == 2
    except MissingRequiredFields as e:
        assert "Missing required attributes: {'id'}" == str(e)

The code flow is as follows:

1. it checks is_binary(headers)

   sdk-python/cloudevents/http/http_methods.py

   Line 46 in b83bfc5

   if is_binary(headers):

2. It calls binary_parser.can_read

   sdk-python/cloudevents/http/event_type.py

   Lines 6 to 16 in b83bfc5

   	def is_binary(headers: typing.Dict[str, str]) -> bool:
   	"""Uses internal marshallers to determine whether this event is binary
   	:param headers: the HTTP headers
   	:type headers: typing.Dict[str, str]
   	:returns bool: returns a bool indicating whether the headers indicate
   	a binary event type
   	"""
   	headers = {key.lower(): value for key, value in headers.items()}
   	content_type = headers.get("content-type", "")
   	binary_parser = binary.BinaryHTTPCloudEventConverter()
   	return binary_parser.can_read(content_type=content_type, headers=headers)

3. it calls has_binary_headers

   sdk-python/cloudevents/sdk/converters/binary.py

   Lines 29 to 35 in b83bfc5

   	def can_read(
   	self,
   	content_type: str = None,
   	headers: typing.Dict[str, str] = {"ce-specversion": None},
   	) -> bool:
   	return has_binary_headers(headers)

4. has_binary_headers checks for the presence of all required fields, which in this test case is false because it is missing ce-id

   sdk-python/cloudevents/sdk/converters/util.py

   Lines 4 to 10 in b83bfc5

   	def has_binary_headers(headers: typing.Dict[str, str]) -> bool:
   	return (
   	"ce-specversion" in headers
   	and "ce-source" in headers
   	and "ce-type" in headers
   	and "ce-id" in headers
   	)

5. it then falls through and tries to get the specversion here

   sdk-python/cloudevents/http/http_methods.py

   Line 57 in b83bfc5

   specversion = raw_ce.get("specversion", None)

6. specversion is never set and the error gets thrown here

   sdk-python/cloudevents/http/http_methods.py

   Lines 64 to 67 in b83bfc5

   	if specversion is None:
   	raise cloud_exceptions.MissingRequiredFields(
   	"Failed to find specversion in HTTP request"
   	)

I think that the solution might be as simple as changing all of the ands to ors in the has_binary_headers method in step 4.

Specifications

• Platform: Mac OS
• Python Version: 3.9.4

[sp-schoen]

I would also like to see this fixed. It would greatly improve the user experience. What I would also consider doing in this step is adding an attribute to the MissingRequiredFields exception that specifies which fields are actually missing. This would make it easier to handle the exception by another application.

[bbtong]

We are seeing this issue, and debugging was a headache because of it. It looks like the PR went stale with no outcome from a WG meeting -- is this true?