tomaka / rouille

Web framework in Rust
Apache License 2.0
1.09k stars 105 forks source link

Numbers in url #155

Closed zer0tonin closed 6 years ago

zer0tonin commented 6 years ago

Hello,

It looks like the application won't compile if the routes have hardcoded numbers in them. For example this works :

fn main() {
    println!("Listening!");

    rouille::start_server("0.0.0.0:8080", move |request| {

        router!(request, 
            (GET) (/) => {
                Response::text("Hello world!")
            },

            (GET) (/two) => {
                Response::text("Hello two!")
            },

            _ => Response::empty_404()
        )

    });
}

But this doesn't :

fn main() {
    rouille::start_server("0.0.0.0:8080", move |request| {

        router!(request, 
            (GET) (/) => {
                Response::text("Hello world!")
            },

            (GET) (/2) => {
                Response::text("Hello 2!")
            },

            _ => Response::empty_404()
        )

    });
}

It leads to the following error:

error: no rules expected the token `request_url`
  --> src/main.rs:11:9
   |
11 | /         router!(request,
12 | |             (GET) (/) => {
13 | |                 Response::text("Hello world!")
14 | |             },
...  |
20 | |             _ => Response::empty_404()
21 | |         )
   | |_________^
   |
   = note: this error originates in a macro outside of the current crate

error: Could not compile `rustit`.

Caused by:
  process didn't exit successfully: `rustc --crate-name rustit src/main.rs --crate-type bin --emit=
dep-info,link -C debuginfo=2 -C metadata=9a526171b8297007 -C extra-filename=-9a526171b8297007 --out
-dir /home/antoine/src/rust/rustit/target/debug/deps -L dependency=/home/antoine/src/rust/rustit/ta
rget/debug/deps --extern rouille=/home/antoine/src/rust/rustit/target/debug/deps/librouille-0f6c379
9a677a040.rlib -L native=/home/antoine/src/rust/Rustit/target/debug/build/brotli-sys-faf770ae9206b3
55/out -L native=/home/antoine/src/rust/Rustit/target/debug/build/miniz-sys-c9ca5bf082b97141/out` (
exit code: 101)

Is this normal behaviour? Is there anyway to get around this?

tomaka commented 6 years ago

The cause is that the macro expects an "ident", in other words a valid Rust variable name. I don't know a way to easily fix this :-/

bb010g commented 6 years ago

Could you also take an expr string literal?

jaemk commented 6 years ago

Edit: Looks like @bb010g beat me to it!

Would it be possible to modify the macro to take a string instead of a series of idents (maybe in a version bump in conjunction with other breaking changes (#154))? I've been meaning to take a look since I've run into something similar, allowing dots in urls.

Here's an example: I want to capture a "keys" parameter after /, but also handle specific exact file requests (for example favicon.ico and robots.txt) all in the router:

router!(request,                                                                                                                                                                                                                                                                
    (GET) (/favicon.ico)           => { handlers::match_asset_exact(request)? },
    (GET) (/robots.txt)            => { handlers::match_asset_exact(request)? },
    (GET) (/something_special.txt) => { handlers::match_asset_exact(request)? },
    (GET) (/{key: String})  => { ... },
    ... handle other static files (`/static/<file>`) ...

Currently I have to put "key parameter matching" under a prefix and serve the files with rouille::match_assets:

.
├── assets
│   ├── favicon.ico
│   ├── robots.txt
│   ├── something_special.txt
│   └── static/
(GET) (/a/{key: String}) => { ... },
_ => {
    let resp = rouille::match_assets(&request, "assets");
    if resp.is_success() { resp }
    else { bail!(DoesNotExist) }
}
tomaka commented 6 years ago

Could you also take an expr string literal?

You can't analyse the content of a string literal at compile-time, so a syntax like (GET) ("/users/{id}/name") isn't possible.

Something like (GET) ("/users/" {id} "/name") could work but it's not very nice nor friendly.

bb010g commented 6 years ago

(GET) (/ "2") doesn't look horrible, though, and you wouldn't be using strings that often. (Or is it only possible to take either exprs or idents, and not a combo of the two?)

jaemk commented 6 years ago

I don't think a mishmash of exprs and idents would be possible. Maybe a workaround could be something like:

(GET) ("/nothing/special") => { ... },
(GET) ("/special/{key}/name/{id}", {key: String, id: u32}) => { ... },

where the macro can branch on the number/format of arguments inside the second section.

Edit: It's much easier to parse (with a macro) as:

(GET) ("/special/{key}/name/{id}", key: String, id: u32) => { ... },
tomaka commented 6 years ago

The syntax (GET) ("/special/{key}/name/{id}", {key: String, id: u32}) => { ... }, also requires some runtime parsing which adds an overhead.

tomaka commented 6 years ago

However I think it should be possible to allow both idents and strings with two different macro paths.

jaemk commented 6 years ago

Right now runtime parsing is only required when params are present. If only a string is provided, then it just does a string comparison (I need to make a small adjustment to avoid creating extra Vecs).

jaemk commented 6 years ago

The alternative router syntax should be available now as of 2.0