BlackSheep icon indicating copy to clipboard operation
BlackSheep copied to clipboard

BlackSheep does't work with a2wsgi

Open jordemort opened this issue 4 years ago • 5 comments

I attempted to use BlackSheep with https://github.com/abersheeran/a2wsgi so that I could use BlackSheep with https://pywebview.flowrl.com/ which expects a WSGI application.

It doesn't work because it seems that BlackSheep does not fully conform to the ASGI specification. Specifically, the raw_path field in an ASGI connection scope is optional, but BlackSheep treats it as required. a2wsgi does not provide this field, so when running a BlackSheep app under its ASGIMiddleware, a KeyError is thrown at https://github.com/Neoteroi/BlackSheep/blob/main/blacksheep/server/application.py#L632

I thought this would be easy to fix myself, and started to attempt to, but then I ran across this block of code: https://github.com/Neoteroi/BlackSheep/blob/main/blacksheep/server/application.py#L682-L689 - it looks like MountMixin depends on raw_path being present, and I'm not sure if it would be correct to rewrite it to use path.

jordemort avatar Sep 04 '21 17:09 jordemort

More information here: https://github.com/abersheeran/a2wsgi/issues/18

jordemort avatar Sep 04 '21 17:09 jordemort

Hi @jordemort

Thank you for raising up the question, we are welcome new contributors, and love them ❤️

Regarding your question, I'd wait for @RobertoPrevato answer, since I don't know the specific reasons why we have chosen raw_path instead of path.

But from my perspective, I don't see any risks, and use path instead of raw_path won't impact a lot of things, and the fix will be easy, but it also will require fixing tests.

And the last thing, since not all web servers provide raw_path information, would be correct to use path, in my opinion.

Sincerely, Mike

myusko avatar Sep 07 '21 14:09 myusko

Hi @jordemort, I apologize for replying so late. In the last months two major positive events happened in my family, and I dedicated all my time to my family.

This evening I started working on the fix to make blacksheep more ASGI compliant (handling scope without raw_path), and I tried to add integration tests running with a2wsgi.

Once I fix the error with raw_path, another error happens: AssertionError: Hop-by-hop header, 'transfer-encoding: chunked', not allowed

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/wsgiref/handlers.py", line 138, in run
    self.finish_response()
  File "/usr/local/lib/python3.8/wsgiref/handlers.py", line 183, in finish_response
    for data in self.result:
  File "{...}/site-packages/a2wsgi/asgi.py", line 179, in __call__
    start_response(f"{status} {HTTPStatus(status).phrase}", headers, None)
  File "/usr/local/lib/python3.8/wsgiref/handlers.py", line 249, in start_response
    assert not is_hop_by_hop(name),\
AssertionError: Hop-by-hop header, 'transfer-encoding: chunked', not allowed
127.0.0.1 - - [22/Oct/2021 21:24:40] "GET /pexels-photo-923360.jpeg HTTP/1.1" 500 59
A server error occurred.  Please contact the administrator.

wsgiref does not allow hop-by-hop headers, and blacksheep uses by default Transfer-Encoding: chunked when serving files that are bigger than a certain size, or in general when serving content from an asynchronous generator. Right now I don't know what is the best way to handle this, but one thing that can be investigated is whether we could support switching off Transfer-Encoding: chunked.

RobertoPrevato avatar Oct 22 '21 19:10 RobertoPrevato

@RobertoPrevato No need to apologize; spending time with your family is always the right choice! Thanks for looking into it - it looks like a WSGI specification thing, re: https://www.python.org/dev/peps/pep-0333/

Applications and middleware are forbidden from using HTTP/1.1 "hop-by-hop" features or headers, any equivalent features in HTTP/1.0, or any headers that would affect the persistence of the client's connection to the web server. These features are the exclusive province of the actual web server, and a server or gateway should consider it a fatal error for an application to attempt sending them, and raise an error if they are supplied to start_response(). (For more specifics on "hop-by-hop" features and headers, please see the Other HTTP Features section below.)

jordemort avatar Oct 22 '21 20:10 jordemort

Thank You! 😄 I'm looking into this, I'm trying to disable the Transfer-Encoding header when serving files from the file system (obtaining the expected file size from os file information and assuming that the size is correct os.stat). I think that not all proxy servers follow the principle described in the WSGI specification, regarding hop-by-hop headers: for example I published blacksheep apps in Azure and they work well (they also run behind proxy servers there).

Async iterables returning bytes of arbitrary size wouldn't be supported at that point, but it's better than the current situation.

RobertoPrevato avatar Oct 22 '21 20:10 RobertoPrevato