oxidecomputer / oxidecomputer/dropshot

Dropshot does not allow you to rename path variables across versions

Open
#1,566 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
1.2k
Forks
104
Avg merge
1d 19h
Merged PRs (30d)
22

Description

I bumped into this in omicron, and I'm not sure if I'm holding it wrong or if this is a dropshot bug. If I modify the versioning example to take a path argument, but attempt to change the name of the field inside that path between the two versions, like this:

diff --git a/dropshot/examples/versioning.rs b/dropshot/examples/versioning.rs
index fca2bdd..034ea84 100644
--- a/dropshot/examples/versioning.rs
+++ b/dropshot/examples/versioning.rs
@@ -103,11 +103,13 @@ use dropshot::ConfigLogging;
 use dropshot::ConfigLoggingLevel;
 use dropshot::HttpError;
 use dropshot::HttpResponseOk;
+use dropshot::Path;
 use dropshot::RequestContext;
 use dropshot::ServerBuilder;
 use dropshot::VersionPolicy;
 use http::HeaderName;
 use schemars::JsonSchema;
+use serde::Deserialize;
 use serde::Serialize;

 #[tokio::main]
@@ -207,14 +209,20 @@ mod v1 {
         thing1_early: &'static str,
     }

+    #[derive(Deserialize, JsonSchema)]
+    struct Thing1Arg {
+        arg: String,
+    }
+
     /// Fetch `thing1`
     #[endpoint {
         method = GET,
-        path = "/thing1",
+        path = "/thing1/{arg}",
         versions = "1.0.0"..TWO_DOT_OH
     }]
     pub async fn get_thing1(
         _rqctx: RequestContext<()>,
+        _path: Path<Thing1Arg>,
     ) -> Result<HttpResponseOk<Thing1>, HttpError> {
         Ok(HttpResponseOk(Thing1 { thing1_early: "hello from an early v1" }))
     }
@@ -231,14 +239,20 @@ mod v2 {
         thing1_late: &'static str,
     }

+    #[derive(Deserialize, JsonSchema)]
+    struct Thing1Arg {
+        myarg: String,
+    }
+
     /// Fetch `thing1`
     #[endpoint {
         method = GET,
-        path = "/thing1",
+        path = "/thing1/{myarg}",
         versions = "2.0.0"..
     }]
     pub async fn get_thing1(
         _rqctx: RequestContext<()>,
+        _path: Path<Thing1Arg>,
     ) -> Result<HttpResponseOk<Thing1>, HttpError> {
         Ok(HttpResponseOk(Thing1 { thing1_late: "hello from a LATE v1" }))
     }

Attempting to run the example (via either run or openapi) induces this panic:

thread 'main' (762450) panicked at /home/john/github/dropshot/dropshot/src/router.rs:314:33:
URI path "/thing1/{myarg}": attempted to use variable name "myarg", but a different name ("arg") has already been used for this
stack backtrace:
   0: __rustc::rust_begin_unwind
             at /rustc/01f6ddf7588f42ae2d7eb0a2f21d44e8e96674cf/library/std/src/panicking.rs:689:5
   1: core::panicking::panic_fmt
             at /rustc/01f6ddf7588f42ae2d7eb0a2f21d44e8e96674cf/library/core/src/panicking.rs:80:14
   2: dropshot::router::HttpRouter<Context>::insert
             at ./dropshot/src/router.rs:314:33
   3: dropshot::api_description::ApiDescription<Context>::register::_register
             at ./dropshot/src/api_description.rs:463:22
   4: dropshot::api_description::ApiDescription<Context>::register
             at ./dropshot/src/api_description.rs:468:9
   5: versioning::main::{{closure}}
             at ./dropshot/examples/versioning.rs:130:9
   6: tokio::runtime::park::CachedParkThread::block_on::{{closure}}
             at /home/john/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/park.rs:284:71
   7: tokio::task::coop::with_budget
             at /home/john/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/task/coop/mod.rs:167:5
   8: tokio::task::coop::budget
             at /home/john/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/task/coop/mod.rs:133:5
   9: tokio::runtime::park::CachedParkThread::block_on
             at /home/john/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/park.rs:284:31
  10: tokio::runtime::context::blocking::BlockingRegionGuard::block_on
             at /home/john/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/context/blocking.rs:66:14
  11: tokio::runtime::scheduler::multi_thread::MultiThread::block_on::{{closure}}
             at /home/john/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/scheduler/multi_thread/mod.rs:89:22
  12: tokio::runtime::context::runtime::enter_runtime
             at /home/john/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/context/runtime.rs:65:16
  13: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
             at /home/john/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/scheduler/multi_thread/mod.rs:88:9
  14: tokio::runtime::runtime::Runtime::block_on_inner
             at /home/john/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/runtime.rs:370:50
  15: tokio::runtime::runtime::Runtime::block_on
             at /home/john/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.49.0/src/runtime/runtime.rs:342:18
  16: versioning::main
             at ./dropshot/examples/versioning.rs:183:12
  17: core::ops::function::FnOnce::call_once
             at /rustc/01f6ddf7588f42ae2d7eb0a2f21d44e8e96674cf/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with dropshot/src/router.rs around line 314 and reproduce the panic using the versioning example in dropshot/examples/versioning.rs via run or openapi. Trace how routes from different versions are registered and verify that the example can use different path-variable names across versions without panicking. Done means both registration modes complete successfully for the shown example.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.