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

Blueprint with url_prefix that contains "search" in path and container <path:path> in route does not match any requests #2130

Closed
tylersalminen opened this issue May 5, 2021 · 4 comments

Comments

@tylersalminen
Copy link

tylersalminen commented May 5, 2021

ISSUE->>>>

If Blueprint with url_prefix that contains "search" in path and container path:path in route does not match any requests
I confirmed the compiled regex used in the server does match my request uri:

path regex
^/prism/api/v1/elasticsearch/rest/(?P[^/]+)/(?P[^/]?.*?)$

request uri
/api/v1/elastic/rest/Elasticsearch_Prism/logs*/_search

I am getting a NotFound error with this uri and the route defined below if and only if i have 2 distinct routes, both with similar url prefixs, but with different route patterns. I do NOT get a NotFound error if i change the url_prefix of the first item to "/api/v1/elastic/rest". if i change the second route to also be "/api/v1/elastic/" the former stops working again. It took me forever to figure out what was going wrong here especially because i reuse this pattern a lot throughout my server but this particular route inexplicable was not working.

HOW TO REPRODUCE ->>>>

sanic = "==21.3.4"

route code that does not work

from sanic import Blueprint, Sanic

ELASTICSEARCH_REST_API = Blueprint("ELASTICSEARCH_REST_API", url_prefix="/api/v1/elasticsearch/rest")

@ELASTICSEARCH_REST_API.route("/<instance:string>/<path:path>",
    methods=["GET", "DELETE", "POST", "PUT", "PATCH"],
    stream=True):

...

ELASTICSEARCH_QUERYCELLAUDITHISTORY = Blueprint("ELASTICSEARCH_QUERYCELLAUDITHISTORY", url_prefix="/api/v1/elasticsearch")

@ELASTICSEARCH_QUERYCELLAUDITHISTORY.route("/<instance:string>/hardcodedpath", methods=["POST"])
async def route_query_cell_audit_history(request, instance):

...

route code that DOES work

from sanic import Blueprint, Sanic

ELASTICSEARCH_REST_API = Blueprint("ELASTICSEARCH_REST_API", url_prefix="/api/v1/elastic/rest")

@ELASTICSEARCH_REST_API.route("/<instance:string>/<path:path>",
    methods=["GET", "DELETE", "POST", "PUT", "PATCH"],
    stream=True):

route code that also works
note that the following also works, if i change the second route's hardcoded path part to the url prefix which leads me to believe the part of the second route is somehow matching to the /rest portion of the url prefix of the first route

ELASTICSEARCH_QUERYCELLAUDITHISTORY = Blueprint("ELASTICSEARCH_QUERYCELLAUDITHISTORY", url_prefix="/api/v1/elasticsearch/hardcodedpath")

@ELASTICSEARCH_QUERYCELLAUDITHISTORY.route("/<instance:string>", methods=["POST"])
async def route_query_cell_audit_history(request, instance):
@ahopkins
Copy link
Member

<path:path> is a pretty broad expansion that forces a regex. You have another path that is very close to it that is non-regex. You are probably correct that these are crossing paths and causing the NotFound. A potential "hacky" solution would be to force your other endpoint to also use regex:

@ELASTICSEARCH_QUERYCELLAUDITHISTORY.route("/<instance:^[^/]+$>/hardcodedpath", methods=["POST"])

I have not tested this use case, but I believe this would probably resolve it when merged without resorting to the above: sanic-org/sanic-routing#37

@tylersalminen
Copy link
Author

Thanks for the response ahopkins,

FYI I was converting a Flask application that did work with the former path syntax. For anyone else having this issue what I did to workaround this was to move the "hardcoded" part of the path before any route parameters.

so instead of the following 2 "conflicting" paths:

"/api/v1/elasticsearch/<instance:string>/queryCellAuditHistory"
"/api/v1/elasticsearch/rest/<instance:string>/<path:path>"

I instead did the following:

"/api/v1/elasticsearch/queryCellAuditHistory/<instance:string>"
"/api/v1/elasticsearch/rest/<instance:string>/<path:path>"

this a little less than ideal in my case as I would rather have the final path be the actual endpoint being used so it is easier to read in Chrome's network tab in it's debug tools (it always shows the last "folder" in the uri in the requests list, which for the queryCellAuditHistory will now be an arbitrary route variable). However, since I have the luxury of being able to change the structure of these routes ATM I went ahead with the later changes and everything works as expected now

@ahopkins
Copy link
Member

Thanks for sharing @tylersalminen

For your reference, tested with the new version of sanic-router:

@app.route("/api/v1/elasticsearch/<instance:string>/queryCellAuditHistory")
async def handler1(request, **kwargs):
    return json({"handler": 1, **kwargs})

@app.route("/api/v1/elasticsearch/rest/<instance:string>/<path:path>")
async def handler2(request, **kwargs):
    return json({"handler": 2, **kwargs})
╭─adam@thedirthouse ~  
╰─$ curl localhost:8888/api/v1/elasticsearch/foobar/queryCellAuditHistory
{"handler":1,"instance":"foobar"}
╭─adam@thedirthouse ~  
╰─$ curl localhost:8888/api/v1/elasticsearch/rest/foobar/path/to/something    
{"handler":2,"instance":"foobar","path":"path\/to\/something"}

@tylersalminen
Copy link
Author

awesome I will check it out thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants