cloudevents / cloudevents/sdk-python

cloudevents.http.from_http binary incorrect error messages

Open
#139 2 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
342
Forks
65
PR merge metrics
No merged PRs in 30d

Description

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) https://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/cloudevents/http/http_methods.py#L46
  2. It calls binary_parser.can_read https://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/cloudevents/http/event_type.py#L6-L16
  3. it calls has_binary_headers https://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/cloudevents/sdk/converters/binary.py#L29-L35
  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 https://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/cloudevents/sdk/converters/util.py#L4-L10
  5. it then falls through and tries to get the specversion here https://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/cloudevents/http/http_methods.py#L57
  6. specversion is never set and the error gets thrown here https://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/cloudevents/http/http_methods.py#L64-L67

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in cloudevents/http/http_methods.py and trace the binary parsing path through cloudevents/http/event_type.py, cloudevents/sdk/converters/binary.py, and cloudevents/sdk/converters/util.py. Reproduce the missing-field cases shown in the issue and verify that missing ce-id, ce-source, or ce-type reports the corresponding missing field instead of specversion.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.