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

Routing hierarchy confusion with CORs #343

Open
CMCDragonkai opened this issue Nov 7, 2020 · 0 comments
Open

Routing hierarchy confusion with CORs #343

CMCDragonkai opened this issue Nov 7, 2020 · 0 comments

Comments

@CMCDragonkai
Copy link

CMCDragonkai commented Nov 7, 2020

We just encountered a strange CORS bug that that we found a solution to. Our solution is this git commit:

diff --git a/prism_orchestrator_server/router.py b/prism_orchestrator_server/router.py
index 58dfd83..34262f7 100644
--- a/prism_orchestrator_server/router.py
+++ b/prism_orchestrator_server/router.py
@@ -106,6 +106,15 @@ def register_routes(container):
     visual_classes = router.add_resource("/visual-classes/")
     visual_classes.add_route("GET", resources["visual_classes"].index)
 
+    visual_class_merge = router.add_resource("/visual-classes/merge")
+    visual_class_merge.add_route("PUT", resources["visual_classes"].merge)
+
+    visual_class_move = router.add_resource("/visual-classes/move")
+    visual_class_move.add_route("PUT", resources["visual_classes"].move)
+
+    visual_class_extract = router.add_resource("/visual-classes/extract")
+    visual_class_extract.add_route("PUT", resources["visual_classes"].extract)
+
     visual_class = router.add_resource("/visual-classes/{visual_class_id}")
     visual_class.add_route("GET", resources["visual_classes"].get)
 
@@ -119,15 +128,6 @@ def register_routes(container):
     )
     visual_class_history.add_route("GET", resources["visual_classes"].get_history)
 
-    visual_class_merge = router.add_resource("/visual-classes/merge")
-    visual_class_merge.add_route("PUT", resources["visual_classes"].merge)
-
-    visual_class_move = router.add_resource("/visual-classes/move")
-    visual_class_move.add_route("PUT", resources["visual_classes"].move)
-
-    visual_class_extract = router.add_resource("/visual-classes/extract")
-    visual_class_extract.add_route("PUT", resources["visual_classes"].extract)
-
     router.add_route("GET", "/instances", redirect_perm("/instances/"))
     router.add_route("POST", "/instances", redirect_perm("/instances/"))
     instances = router.add_resource("/instances/")

Basically the problem was that /visual-classes/merge, /visual-classes/move and /visual-classes/extract routes were defined after /visual-classes/{visual_class_id}, and while aiohttp was able to handle this routing order with no problems, it appears that aiohttp-cors cannot.

That is aiohttp-cors fixes the GET for /visual-classes/{visual_class_id} and later when I try to enable CORS for /visual-classes/merge for PUT, it doesn't work, because it thinks that GET cors options have already been set for /visual-classes/{visual_class_id}.

After some debugging. The only solution was to move the more specific routes on top of the generic route. And then CORS worked beautifully.

I think this is a bug in aiohttp-cors, and it's particularly nasty because it's very silent, and when you think aiohttp routing works fine, but cors doesn't, it can be difficult to realize this.

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

1 participant