aio-libs / aiohttp-cors

CORS support for aiohttp
Apache License 2.0
208 stars 57 forks source link

method '*' treated differently from others #241

Open wumpus opened 5 years ago

wumpus commented 5 years ago

I'm trying to use aiohttp-cors with aiohttp-graphl, which registers a route with the '*' method. I get

ValueError: <PlainResource 'graphql' /graphql> already has a '*' handler for all methods

Here is a small reproducer showing that '*' is different:

import asyncio
import aiohttp
import aiohttp_cors
from aiohttp.web_runner import GracefulExit

async def handler(request):
    return aiohttp.web.Response(text="Hello!")

app = aiohttp.web.Application()

cors = aiohttp_cors.setup(app)

routes = [{
    'method': 'GET',
    'path': '/test',
    'handler': handler,
    'name': 'test-good'
    }, {
    'method': '*',  # ValueError: <PlainResource 'another-route'  /another-route> already has a '*' handler for all methods                                           
    'path': '/another-route',
    'handler': handler,
    'name': 'another-route'
    }, ]

for route in routes:
    print('creating a route for method', route['method'])
    cors.add(
        app.router.add_route(
            method=route['method'],
            path=route['path'],
            handler=route['handler'],
            name=route['name']
        )
    )

web.run_app(app, host='localhost', port=8081)

which prints

creating a route for method GET
creating a route for method *
Traceback (most recent call last):
...
ValueError: <PlainResource 'another-route'  /another-route> already has a '*' handler for all methods

Reading the aiohttp-cors code I can see that _is_web_view() is False and then causes this confusing message? But only for method '*' and not GET.

I hacked the aiohttp-cors code to create separate routes for all the methods other than OPTIONS, and my program now runs successfully. But obviously that's not a fix.

asvetlov commented 5 years ago

If you provide a patch I'm happy to review it.

wumpus commented 5 years ago

I suspect it's a bug that aiohttp-cors works great for GET and throws a configuration error for method '*'.

I don't see any tests for method '*' actually working. There are tests in tests/unit/test_cors_config related to webviews and mixins. That appears to be the only tests related to method '*'.

aiohttp-graphl is an example of middleware that expects method='*' to work

asvetlov commented 5 years ago

@wumpus sorry. Please ping me in, say, 10 days. Ok? I have no free time to work on the issue personally now (review only maybe) but really want to get CORS plugin working. Last months worked hard on CPython, added several important changes to asyncio. Have to finish this job before returning back to aio-libs projects.

wumpus commented 5 years ago

Ping.

I think the basic issue is:

amaksymov commented 5 years ago

Hi, @asvetlov ! I encountered the same error. Are there any solutions?

jogc commented 5 years ago

Same problem here

wumpus commented 5 years ago

@andrii-maksymov and @jogc what middleware are you using?

jogc commented 5 years ago

this is what i do, there is no other middleware used

aiohttp_jinja2.setup(app,
    loader = jinja2.FileSystemLoader(os.path.dirname(os.path.abspath(__file__)) + '/templates'),
    extensions = [ 'pypugjs.ext.jinja.PyPugJSExtension' ])

aiohttp_graphql.GraphQLView.attach(app, schema = schema,
    graphiql = True)

cors = aiohttp_cors.setup(app, defaults={
    'http://127.0.0.1:3000': aiohttp_cors.ResourceOptions(
        allow_credentials=True,
        expose_headers='*',
        allow_headers=('Content-Type',),
    )
})
wumpus commented 5 years ago

Thanks @jogc so you're using exactly the same middleware that I am.

Here is my patch that works around the problem in aiohttp_graphql, by handling every method other than OPTIONS:

https://github.com/graphql-python/aiohttp-graphql/compare/master...wumpus:master