Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

The Module API docs' DemoResource crashes #10837

Closed
cvwright opened this issue Sep 16, 2021 · 5 comments
Closed

The Module API docs' DemoResource crashes #10837

cvwright opened this issue Sep 16, 2021 · 5 comments
Labels
S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@cvwright
Copy link
Contributor

Description

The Module API docs include an example module, which is extremely helpful for a new module developer.

However, the DemoResource's render_GET function causes a TypeError during JSON encoding and fails to return the intended response.

Steps to reproduce

  • Install the DemoModule.
    • I put the example code in synapse/demo_module/__init__.py
    • Copy the example config verbatim into homeserver.yaml
  • Start Synapse:
$ python3 -m synapse.app.homeserver --config-path homeserver.yaml
  • Load the demo URL _synapse/client/demo/hello?name=world in a browser or use cURL/wget/etc

Expected to see the simple JSON response. Got "Processing Failed" instead.

2021-09-16 11:44:50,905 - twisted - 271 - CRITICAL - sentinel - 
Traceback (most recent call last):
  File "/home/user/synapse/env/lib/python3.7/site-packages/twisted/web/server.py", line 229, in process
    self.render(resrc)
  File "/home/user/synapse/synapse/http/site.py", line 241, in render
    Request.render(self, resrc)
  File "/home/user/synapse/env/lib/python3.7/site-packages/twisted/web/server.py", line 294, in render
    body = resrc.render(self)
  File "/home/user/synapse/env/lib/python3.7/site-packages/twisted/web/resource.py", line 263, in render
    return m(request)
  File "/home/user/synapse/demo_module/__init__.py", line 17, in render_GET
    return json.dumps({"hello": name})
  File "/usr/lib/python3.7/json/__init__.py", line 231, in dumps
    return _default_encoder.encode(obj)
  File "/usr/lib/python3.7/json/encoder.py", line 199, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/usr/lib/python3.7/json/encoder.py", line 257, in iterencode
    return _iterencode(o, 0)
  File "/usr/lib/python3.7/json/encoder.py", line 179, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type bytes is not JSON serializable

Version information

  • Homeserver: My own development server

If not matrix.org:

  • Version: 1.42.0

  • Install method: Git clone

  • Platform: Debian 10, x86, Python 3.7.3

Proposed Fix

Since the problem seems to be with the JSON encoder failing to handle bytes, we can simply decode the string before we put it into the JSON. Also, it seems that we need to encode the JSON string back to bytes before we return.

    def render_GET(self, request: Request):
        name = request.args.get(b"name")[0].decode() 
        request.setHeader(b"Content-Type", b"application/json") 
        return json.dumps({"hello": name}).encode()
@DMRobertson
Copy link
Contributor

Thanks for spotting! Would you be up for making this change into a pull request?

I'm guessing the example you mention is the one here? I wonder if that was written a while back with Python 2 or a particularly old twisted version in mind.

Also, it seems that we need to encode the JSON string back to bytes before we return.

Indeed: twisted expects render_METHOD to return a bytes type if it doesn't write the result asynchronously. See also https://twistedmatrix.com/documents/21.2.0/api/twisted.web.resource.Resource.html#render

(cc @babolivier because modules.)

@cvwright
Copy link
Contributor Author

Glad to be able to help! And yes, that's the example that I was looking at.

Would you be up for making this change into a pull request?

Sure thing. I'll try to submit the PR tomorrow if I can find some time.

Thanks for the quick response, and for confirming that it's not just something weird I'm doing on my end.

I'm loving the Module API so far btw.

@cvwright
Copy link
Contributor Author

So it turns out that the documentation I was reading had already been updated to be even more detailed, and there's no more DemoModule or DemoResource. But it appears that the same issue exists in the new IsUserEvilResource. So the linked PR fixes it there.

@anoadragon453 anoadragon453 added S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Sep 17, 2021
@richvdh
Copy link
Member

richvdh commented Sep 20, 2021

fixed by #10845. @anoadragon453 please can you make sure that PRs and issues are correctly linked before merging.

@anoadragon453
Copy link
Member

@richvdh Oops, yep can do.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

4 participants