Skip to content
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

Improve error message when unexpected request.data encountered during parsing #111

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amberylx
Copy link
Contributor

@amberylx amberylx commented Apr 30, 2018

Ran into a scenario when sending a POST request to the gateway from a client-side javascript application, where request.body was of type string instead of type object. This unexpected data type is accepted initially, but is not handled or caught at any point in the request cycle, the result being that the final request that is passed to the backend Django view has an empty request.POST body.

Walkthrough of request cycle:

  1. The client-side javascript application makes a request to gateway using the requests library, and passes an options object with {body: <JSON API Resource Identifier Object>}
  2. When the request hits gateway here: https://github.com/ZeroCater/gateway/blob/master/src/handleRoute.js#L30, this.request.body is of type string (but the code "expects" an object)
  3. gateway creates an event to be passed into the message bus here: https://github.com/ZeroCater/gateway/blob/master/src/handleRoute.js#L52 with kwargs where body is of type `string
  4. The created event is picked up by microservice_events_listener.py
  5. microservice_events_listener.py routes the request to the appropriate job task in microservice_events.py
  6. microservice_events.py calls event_client.handle_request_event()
  7. Which calls create_django_request() here: https://github.com/ZeroCater/zc_events/blob/master/zc_events/client.py#L352
  8. Which attaches the string body to request.raw_body here: https://github.com/ZeroCater/zc_events/blob/master/zc_events/django_request.py#L45
  9. Then the job task picks up the request object and parses the information in it via zc_common.remote_resource.parsers here: https://github.com/ZeroCater/zc_common/blob/master/zc_common/remote_resource/parsers.py#L62
  10. The conditional checking for raw_body here https://github.com/ZeroCater/zc_common/blob/master/zc_common/remote_resource/parsers.py#L63 is True, so result is set to the string body
  11. Then since we are expecting a body of type Object we do result.get('data') here: https://github.com/ZeroCater/zc_common/blob/master/zc_common/remote_resource/parsers.py#L72, but it raises an AttributeErrorsince result is actually a string
  12. This error gets absorbed somewhere, I think here: https://github.com/encode/django-rest-framework/blob/master/rest_framework/request.py#L349
  13. Ultimately, request.data is set to an empty dictionary.

Essentially, data goes in on one end, then gets silently swallowed and discarded somewhere deep down along a long request cycle chain.

This PR detects the discrepancy in data type, and raises and appropriate error with explanatory message.

@amberylx
Copy link
Contributor Author

amberylx commented May 1, 2018

@amberylx
Copy link
Contributor Author

amberylx commented May 1, 2018

Note: This PR is not strictly required for functionality in https://trello.com/c/xACK2j2O/1181-fetch-company-stats-by-page, but was uncovered during implementation.

@amberylx
Copy link
Contributor Author

amberylx commented May 8, 2018

sdf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant