#93 Respond with CORS headers
Merged 4 years ago by pingou. Opened 4 years ago by karsten.
karsten/mdapi cors_headers  into  master

file modified
+8 -1
@@ -2,6 +2,7 @@ 

  import logging.config

  

  from aiohttp import web

+ from aiohttp.web import middleware

  

  from mdapi import CONFIG

  from mdapi.views import (
@@ -21,6 +22,12 @@ 

          get_pkg_changelog

  )

  

+ @middleware

+ async def add_cors_headers(request, handler):

+     resp = await handler(request)

+     resp.headers['Access-Control-Allow-Origin'] = '*'

+     resp.headers['Access-Control-Allow-Methods'] = 'GET'

+     return resp

  

  async def init_app():

      """ Creates the aiohttp application.
@@ -30,7 +37,7 @@ 

      logging.basicConfig()

      logging.config.dictConfig(CONFIG.get("LOGGING") or {"version": 1})

  

-     app = web.Application()

+     app = web.Application(middlewares=[add_cors_headers])

  

      app.add_routes([

          web.get('/', index),

As we query get_src_pkg from src.fp.o with a request for json,
we need to respond with json plus some additional headers to
tell modern browsers that this cross-origin HTTP request
should be allowed.

Signed-off-by: Karsten Hopp karsten@redhat.com

Code looks fine in itself however, this is only going to work for one endpoint no?

Maybe we could use a decorator to make this work with all the endpoints we want?

1 new commit added

  • Use a decorator to add CORS headers to all replies
4 years ago

This likely needs to be async

Did you see https://github.com/aio-libs/aiohttp-cors it looks like it may also do what we want. What do you think?

yes, I've had a look at it and it also can achieve the same. I wonder if it isn't overkill for our purposes

Since all the views on mdapi are susceptible of being cross origin called, why not go with a global postrequest implementation that add the headers to all the responses instead of a decorator that we'll forgot to add if we expand mdapi?

I don't think this'll be a big issue. Whoever is going to add a new endpoint will be looking at the existing ones anyway

I agree with @karsten that people will be looking at the existing endpoints, however @jlanda 's idea is simply safer.

aiohttp's middleware seems to be the way to do that:
https://docs.aiohttp.org/en/latest/web_advanced.html#middlewares

rebased onto b470b06

4 years ago

I've updated the PR and use aiohttp middleware now

It looks good, but something looks wrong about the commits. I see a commit from Clément and your commit seems to have both the removal of the previous attempt as well as the new one.

Maybe it needs a rebase against master?

2 new commits added

  • Respond with CORS headers
  • Remove blocking call to sqlite by using aiosqlite module.
4 years ago

rebased onto 04306f0

4 years ago

From https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Headers

The CORS-safelisted request headers, Accept, Accept-Language, Content-Language, Content-Type are always allowed and don't need to be listed by this header necessarily. However, note that additional restrictions apply with these headers which you circumvent by listing these headers in an Access-Control-Allow-Headers header as well.

So as I understand there is no need of this header

This is looking good!
Let's address @jlanda's and get this in :)

1 new commit added

  • fix typo
4 years ago

1 new commit added

  • remove Access-Control-Allow-Headers
4 years ago

I've removed the typo and Access-Control-Allow-Headers

Pull-Request has been merged by pingou

4 years ago
Metadata