carllerche / tower-web

A fast, boilerplate free, web framework for Rust
MIT License
980 stars 51 forks source link

Panic when using non-ASCII in a route #138

Open phrohdoh opened 5 years ago

phrohdoh commented 5 years ago

This very well may be a use-case that you decide not to support but I figured I'd report it regardless.

#[get("/£")]
#[content_type("plain")]
fn money_plz(&self) -> Result<String, ()> {
    Ok("$".into())
}
➜  server git:(master) ✗ RUST_BACKTRACE=1 cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.14s
     Running `target/debug/grassdoor`
Listening on http://127.0.0.1:8080
thread 'main' panicked at 'byte index 2 is not a char boundary; it is inside '£' (bytes 1..3) of `/£`', libcore/str/mod.rs:2108:5
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: <std::panicking::begin_panic::PanicPayload<A> as core::panic::BoxMeUp>::get
             at libstd/panicking.rs:475
   5: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:390
   6: std::panicking::try::do_call
             at libstd/panicking.rs:325
   7: core::ptr::drop_in_place
             at libcore/panicking.rs:77
   8: core::str::run_utf8_validation
             at libcore/str/mod.rs:2108
   9: <usize as core::iter::range::Step>::add_usize
             at /Users/travis/build/rust-lang/rust/src/libcore/str/mod.rs:1891
  10: core::ptr::drop_in_place
             at /Users/travis/build/rust-lang/rust/src/libcore/option.rs:386
  11: core::str::traits::<impl core::slice::SliceIndex<str> for core::ops::range::Range<usize>>::index
             at /Users/travis/build/rust-lang/rust/src/libcore/str/mod.rs:1891
  12: core::str::traits::<impl core::ops::index::Index<core::ops::range::Range<usize>> for str>::index
             at /Users/travis/build/rust-lang/rust/src/libcore/str/mod.rs:1667
  13: core::str::traits::<impl core::ops::index::Index<core::ops::range::RangeFull> for str>::index
             at /Users/taryn/.cargo/registry/src/github.com-1ecc6299db9ec823/tower-web-0.3.1/src/routing/path.rs:18
  14: <tower_web::routing::route::Route<T>>::path
             at /Users/taryn/.cargo/registry/src/github.com-1ecc6299db9ec823/tower-web-0.3.1/src/routing/route.rs:39
  15: grassdoor::services::web::__IMPL_WEB_1_FOR_WebService::<impl tower_web::routing::resource::IntoResource<S, B> for grassdoor::services::web::WebService>::routes
             at ./<derive_resource macros>:2
  16: tower_web::util::tuple::<impl tower_web::routing::resource::IntoResource<S, B> for (T0, T1, T2)>::routes
             at /Users/taryn/.cargo/registry/src/github.com-1ecc6299db9ec823/tower-web-0.3.1/src/util/tuple.rs:3995
  17: <tower_web::service::builder::ServiceBuilder<T, S, C, M>>::build_new_service
             at /Users/taryn/.cargo/registry/src/github.com-1ecc6299db9ec823/tower-web-0.3.1/src/service/builder.rs:463
  18: <tower_web::service::builder::ServiceBuilder<T, S, C, M>>::run
             at /Users/taryn/.cargo/registry/src/github.com-1ecc6299db9ec823/tower-web-0.3.1/src/service/builder.rs:519
  19: grassdoor::main
             at src/main.rs:42
  20: std::rt::lang_start::{{closure}}
             at /Users/travis/build/rust-lang/rust/src/libstd/rt.rs:74
  21: std::panicking::try::do_call
             at libstd/rt.rs:59
             at libstd/panicking.rs:310
  22: panic_unwind::dwarf::eh::read_encoded_pointer
             at libpanic_unwind/lib.rs:105
  23: <std::sync::mutex::Mutex<T>>::new
             at libstd/panicking.rs:289
             at libstd/panic.rs:392
             at libstd/rt.rs:58
  24: std::rt::lang_start
             at /Users/travis/build/rust-lang/rust/src/libstd/rt.rs:74
  25: grassdoor::main

Using tower-web version 0.3.1.

carllerche commented 5 years ago

Yeah, that seems bad 👍

carllerche commented 5 years ago

Uris cannot actually contain non-ascii characters. These characters would be percent encoded.

So, tower-web could transparently percent encode these cases?

Abogical commented 5 years ago

Uris cannot actually contain non-ascii characters. These characters would be percent encoded.

So, tower-web could transparently percent encode these cases?

Should this be done in the http crate?

lnicola commented 5 years ago

Filed #206 to fix the panic, although the route probably won't match due to the percent-encoding. See https://github.com/carllerche/tower-web/issues/142 and the linked thread for a wider discussion on decoding the request parameters.