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

Trailing slashes leads to 404 #818

Closed
VishnuSanal opened this issue May 13, 2024 · 1 comment · Fixed by #819
Closed

Trailing slashes leads to 404 #818

VishnuSanal opened this issue May 13, 2024 · 1 comment · Fixed by #819
Labels
bug Something isn't working

Comments

@VishnuSanal
Copy link
Contributor

VishnuSanal commented May 13, 2024

Bug Description

say, I have a route:

@app.get("/sync/str/")
def sync_str_get():
    return "sync str get"

now, when I do curl http://localhost:8080/sync/str/, everything works fine, but if I do a curl http://localhost:8080/sync/str, I get a "Not found".

this is the same case if it is vice versa too (slash not being present in the route & slash being present in the URL).

edit: I tried the same thing with flask, and that worked fine.

Steps to Reproduce

No response

Your operating system

Linux

Your Python version (python --version)

3.12

Your Robyn version

main branch

Additional Info

ofc, I would like to work on this issue, and I have been debugging it, that led me to const_router.Router#get_route.

fn get_route(&self, route_method: &HttpMethod, route: &str) -> Option<Response> {
        let table = self.routes.get(route_method)?;
        let route_map = table.read().ok()?;

        match route_map.at(route) {
            Ok(res) => Some(res.value.clone()),
            Err(_) => None,
        }
    }

I presume I should add/remove a trailing slash here after checks. @sansyrox please CMIIW. :)

edit: or, we can add both the routes (with & without the slash) in Server#add_route, since doing the above approach would introduce an overhead on every request, but this approach only once. what say?

@VishnuSanal VishnuSanal added the bug Something isn't working label May 13, 2024
@sansyrox
Copy link
Member

What I would suggest is to append a / to the URL without the / and then add it. It will make things a lot simpler

VishnuSanal added a commit to VishnuSanal/Robyn that referenced this issue May 14, 2024
VishnuSanal pushed a commit to VishnuSanal/Robyn that referenced this issue May 14, 2024
sansyrox added a commit that referenced this issue May 19, 2024
* fixes #818

* fix: trailing slash issue

* use `str#ends_with`

* something like this

* update

* fix: error[E0716]: temporary value dropped while borrowed

* use only one `#clone`, pass the function to the other

* change back to inline assignment itself

* test: add tests

* docs: add comment

* fix(ci): change string to char

* fix(ci): formatting -- run `cargo fmt`

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Sanskar Jethi <29942790+sansyrox@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants