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

method '*' treated differently from others #241

Open
wumpus opened this issue May 23, 2019 · 9 comments
Open

method '*' treated differently from others #241

wumpus opened this issue May 23, 2019 · 9 comments

Comments

@wumpus
Copy link

wumpus commented May 23, 2019

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
Copy link
Member

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

@wumpus
Copy link
Author

wumpus commented May 31, 2019

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
Copy link
Member

@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
Copy link
Author

wumpus commented Jun 16, 2019

Ping.

I think the basic issue is:

  • CORS preflight request is a OPTIONS
  • Lazy coders create routes to method='*' in middleware like aiohttp-graphl
  • aiohttp-cors treats method='*' specially because it includes OPTIONS
  • it would be nice if the middleware still worked

@amaksymov
Copy link

amaksymov commented Jul 6, 2019

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

@jogc
Copy link

jogc commented Jul 21, 2019

Same problem here

@wumpus
Copy link
Author

wumpus commented Jul 22, 2019

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

@jogc
Copy link

jogc commented Jul 22, 2019

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
Copy link
Author

wumpus commented Jul 23, 2019

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:

graphql-python/aiohttp-graphql@master...wumpus:master

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

No branches or pull requests

4 participants