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

Dynamic Route Not Matching Expected URL Patterns /sitemap_*.xml or /sitemap_:id.xml #210

Open
binhvq opened this issue Nov 30, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@binhvq
Copy link

binhvq commented Nov 30, 2024

The current route definition app.get("/sitemap_*", sitemaps) isn't catching requests like /sitemap_823.xml. This happens because the asterisk (*) in the route isn't a proper match for such patterns.

@ArthoPacini
Copy link

From the docs:

Routes are matched in order of specificity, not by the order you register them:

- Highest priority - static routes, think “/hello/this/is/static”.
- Middle priority - parameter routes, think “/candy/:kind”, where value of :kind is retrieved by req.get_parameter(0).
- Lowest priority - wildcard routes, think “/hello/*”. In other words, the more specific a route is, the earlier it will match. This allows you to define wildcard routes that match a wide range of URLs and then “carve” out more specific behavior from that.

“any” routes, those who match any HTTP method, will match with lower priority than routes which specify their specific HTTP method (such as GET) if and only if the two routes otherwise are equally specific.

Can you give some reproducible example? How are your routes setup?

@binhvq
Copy link
Author

binhvq commented Dec 3, 2024

I use this
app.get("/static/:path", static) app.get("/*", catch_all) app.get("/robots.txt", robots) app.get("/sitemap_*", sitemaps)

@cirospaciari
Copy link
Owner

cirospaciari commented Dec 3, 2024

I use this app.get("/static/:path", static) app.get("/*", catch_all) app.get("/robots.txt", robots) app.get("/sitemap_*", sitemaps)

this need to be the last one:

app.get("/*", catch_all) 

@binhvq
Copy link
Author

binhvq commented Dec 3, 2024

simple setup

from socketify import App, AppOptions

def make_app(app:App):
    app.get("/sitemap_*", lambda res, req: res.end("Hello World socketify from Python!"))

if __name__ == "__main__":
    app = App()
    make_app(app)
    app.listen(3000, lambda config: print("Listening on port http://localhost:%d now\n" % config.port))
    app.run()

GET /sitemap_* => Response Hello World socketify from Python!
GET /sitemap_dynamic => Empty Response

@cirospaciari
Copy link
Owner

Looks like uWS only support exactly matching "*" and not partial like /sitemap_*
https://github.com/uNetworking/uWebSockets/blob/master/src/HttpRouter.h#L185

An workaround would be /:path and manually check if the route starts with "/sitemap_"
Will need some improvements on the HttpRouter to support this.

@cirospaciari cirospaciari added enhancement New feature or request and removed need repro labels Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants