don't
refactor(knot): rename `RepoPath` to `RepoSpec` and improve robustness of deserialization
Signed-off-by: tjh <did:plc:65gha4t3avpfpzmvpbwovss7>
+278
-147
+1
Cargo.lock
+1
Cargo.lock
-3
Cargo.toml
-3
Cargo.toml
···
13
13
14
14
[workspace.dependencies]
15
15
identity = { path = "crates/identity" }
16
-
service_auth = { path = "crates/service_auth" }
17
16
xrpc = { path = "crates/xrpc" }
18
-
knot = { path = "crates/knot" }
19
17
axum = "0.8.4"
20
18
serde = "1.0.226"
21
-
thiserror = "2.0.16"
22
19
tracing = "0.1.41"
23
20
24
21
[profile.release]
+3
crates/knot/Cargo.toml
+3
crates/knot/Cargo.toml
+1
-2
crates/knot/src/lib.rs
+1
-2
crates/knot/src/lib.rs
+3
-2
crates/knot/src/main.rs
+3
-2
crates/knot/src/main.rs
···
9
9
rt::TokioExecutor,
10
10
};
11
11
use identity::Resolver;
12
-
use knot::{RepoPath, model::Knot};
12
+
use knot::{model::Knot, repospec::RepoSpec};
13
13
use reqwest::StatusCode;
14
14
use serde::Deserialize;
15
15
use std::{
···
45
45
46
46
#[derive(Deserialize)]
47
47
struct Repo {
48
-
repo: RepoPath,
48
+
#[serde(with = "knot::repospec::query_param")]
49
+
repo: RepoSpec,
49
50
}
50
51
51
52
fn extract_request_id<B>(request: &Request<B>) -> Option<&str> {
+6
-6
crates/knot/src/model.rs
+6
-6
crates/knot/src/model.rs
···
1
1
use crate::{
2
-
RepoPath,
3
2
convert::time_to_offsetdatetime,
4
3
model::errors::{HeadDetached, PathNotFound, RefNotFound, RepoEmpty, RepoError, RepoNotFound},
5
4
public::xrpc::XrpcError,
5
+
repospec::RepoSpec,
6
6
types::sh::tangled::repo::{
7
7
BlobEncoding, BlobParams, BlobResponse, Branch, BranchesParams, BranchesResponse,
8
8
DefaultBranchResponse, Diff, DiffParams, DiffResponse, GetDefaultBranchParams, JsonBlob,
···
57
57
struct KnotState {
58
58
owner: Did,
59
59
repository_base: PathBuf,
60
-
repository_cache: Mutex<FxHashMap<RepoPath, ThreadSafeRepository>>,
60
+
repository_cache: Mutex<FxHashMap<RepoSpec, ThreadSafeRepository>>,
61
61
}
62
62
63
63
pub trait RepositoryManager {
64
-
fn open(&self, repo: &RepoPath) -> Result<Repository, XrpcError>;
64
+
fn open(&self, repo: &RepoSpec) -> Result<Repository, XrpcError>;
65
65
66
-
fn reopen(&self, repo: &RepoPath) -> Result<Repository, XrpcError>;
66
+
fn reopen(&self, repo: &RepoSpec) -> Result<Repository, XrpcError>;
67
67
}
68
68
69
69
impl RepositoryManager for Knot {
70
-
fn open(&self, repo: &RepoPath) -> Result<Repository, XrpcError> {
70
+
fn open(&self, repo: &RepoSpec) -> Result<Repository, XrpcError> {
71
71
let mut cache = self
72
72
.inner
73
73
.repository_cache
···
104
104
}
105
105
}
106
106
107
-
fn reopen(&self, repo: &RepoPath) -> Result<Repository, XrpcError> {
107
+
fn reopen(&self, repo: &RepoSpec) -> Result<Repository, XrpcError> {
108
108
{
109
109
let mut cache = self
110
110
.inner
+6
-6
crates/knot/src/public/git.rs
+6
-6
crates/knot/src/public/git.rs
···
1
1
use crate::{
2
-
RepoPath,
3
2
model::{Knot, RepositoryManager as _},
4
3
public::git::{helpers::SetOptionEnv as _, protocol::GitProtocol},
4
+
repospec::RepoSpec,
5
5
};
6
6
use axum::{
7
7
Router,
···
123
123
#[tracing::instrument]
124
124
pub async fn info_refs(
125
125
State(knot): State<Knot>,
126
-
Path(repo): Path<RepoPath>,
126
+
Path(repo): Path<RepoSpec>,
127
127
Query(InfoRefsQuery { service }): Query<InfoRefsQuery>,
128
128
protocol: Option<GitProtocol>,
129
129
) -> Result<Response> {
···
137
137
#[tracing::instrument]
138
138
pub async fn advertise_upload_pack(
139
139
knot: Knot,
140
-
repo: RepoPath,
140
+
repo: RepoSpec,
141
141
protocol: Option<GitProtocol>,
142
142
) -> Result<Response> {
143
143
let repo = knot.open(&repo).map_err(|_| Error::not_found())?;
···
182
182
#[tracing::instrument]
183
183
pub async fn advertise_receive_pack(
184
184
knot: Knot,
185
-
repo: RepoPath,
185
+
repo: RepoSpec,
186
186
protocol: Option<GitProtocol>,
187
187
) -> Result<Response> {
188
188
let repo = knot.reopen(&repo).map_err(|_| Error::not_found())?;
···
225
225
/// Transfer phase of a `git clone`, `git fetch`, or `git pull`.
226
226
pub async fn upload_pack(
227
227
State(knot): State<Knot>,
228
-
Path(repo): Path<RepoPath>,
228
+
Path(repo): Path<RepoSpec>,
229
229
protocol: Option<GitProtocol>,
230
230
body: Body,
231
231
) -> Result<Response> {
···
274
274
/// Transfer phase of a `git push`.
275
275
pub async fn receive_pack(
276
276
State(knot): State<Knot>,
277
-
Path(repo): Path<RepoPath>,
277
+
Path(repo): Path<RepoSpec>,
278
278
protocol: Option<GitProtocol>,
279
279
body: Body,
280
280
) -> Result<impl IntoResponse> {
-120
crates/knot/src/repoid.rs
-120
crates/knot/src/repoid.rs
···
1
-
use serde::{
2
-
Deserialize, Deserializer,
3
-
de::{Error, Visitor},
4
-
};
5
-
6
-
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
7
-
pub struct RepoPath {
8
-
/// DID of the repository owner
9
-
pub owner: Box<str>,
10
-
11
-
/// Repository name
12
-
pub name: Box<str>,
13
-
}
14
-
15
-
impl RepoPath {
16
-
pub const fn owner(&self) -> &str {
17
-
&self.owner
18
-
}
19
-
20
-
pub const fn name(&self) -> &str {
21
-
&self.name
22
-
}
23
-
}
24
-
25
-
impl<'de> Deserialize<'de> for RepoPath {
26
-
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
27
-
where
28
-
D: Deserializer<'de>,
29
-
{
30
-
struct RepoIdVisitor;
31
-
32
-
impl<'de> Visitor<'de> for RepoIdVisitor {
33
-
type Value = RepoPath;
34
-
35
-
fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
36
-
formatter.write_str("repository identifier in the form '{owner}/{name}'")
37
-
}
38
-
39
-
fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
40
-
where
41
-
E: Error,
42
-
{
43
-
let Some((owner, name)) = v.split_once('/') else {
44
-
return Err(Error::custom(
45
-
"Expected a repository identifier in the form '{owner}/{name}'",
46
-
));
47
-
};
48
-
49
-
validate(owner).map_err(Error::custom)?;
50
-
validate(name).map_err(Error::custom)?;
51
-
52
-
Ok(Self::Value {
53
-
owner: owner.into(),
54
-
name: name.into(),
55
-
})
56
-
}
57
-
58
-
fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
59
-
where
60
-
A: serde::de::SeqAccess<'de>,
61
-
{
62
-
let owner: Box<str> = seq
63
-
.next_element()?
64
-
.ok_or_else(|| serde::de::Error::invalid_length(0, &self))?;
65
-
let name: Box<str> = seq
66
-
.next_element()?
67
-
.ok_or_else(|| serde::de::Error::invalid_length(0, &self))?;
68
-
69
-
validate(&owner).map_err(Error::custom)?;
70
-
validate(&name).map_err(Error::custom)?;
71
-
72
-
Ok(Self::Value { owner, name })
73
-
}
74
-
}
75
-
76
-
deserializer.deserialize_seq(RepoIdVisitor)
77
-
}
78
-
}
79
-
80
-
fn validate(name: &str) -> Result<(), ValidationError> {
81
-
static FORBIDDEN_SUBSTRINGS: &[&str] = &[".."];
82
-
static FORBIDDEN_PREFIXES: &[&str] = &[".", "-"];
83
-
static ACCEPTED_SYMBOLS: &[char] = &['.', '-', '_', ':'];
84
-
85
-
if name.is_empty() {
86
-
return Err(ValidationError::Empty);
87
-
}
88
-
89
-
for value in name.chars() {
90
-
if !(value.is_ascii_alphanumeric() || ACCEPTED_SYMBOLS.contains(&value)) {
91
-
return Err(ValidationError::Character { value });
92
-
}
93
-
}
94
-
95
-
for substring in FORBIDDEN_SUBSTRINGS {
96
-
if name.contains(substring) {
97
-
return Err(ValidationError::Substring { substring });
98
-
}
99
-
}
100
-
101
-
for prefix in FORBIDDEN_PREFIXES {
102
-
if name.starts_with(prefix) {
103
-
return Err(ValidationError::Prefix { prefix });
104
-
}
105
-
}
106
-
107
-
Ok(())
108
-
}
109
-
110
-
#[derive(Debug, thiserror::Error)]
111
-
enum ValidationError {
112
-
#[error("repository name cannot be empty")]
113
-
Empty,
114
-
#[error("invalid character in repository name: '{value}'")]
115
-
Character { value: char },
116
-
#[error("invalid sub-string in repository name: '{substring}'")]
117
-
Substring { substring: &'static str },
118
-
#[error("invalid prefix for respository name: '{prefix}'")]
119
-
Prefix { prefix: &'static str },
120
-
}
+243
crates/knot/src/repospec.rs
+243
crates/knot/src/repospec.rs
···
1
+
use serde::{Deserialize, Serialize};
2
+
use std::str::FromStr;
3
+
4
+
/// Repository path specifier.
5
+
#[derive(Clone, Debug, PartialEq, Eq, Hash, Deserialize, Serialize)]
6
+
#[serde(try_from = "UnvalidatedRepoSpec")]
7
+
pub struct RepoSpec {
8
+
/// DID of the repository owner
9
+
pub owner: Box<str>,
10
+
11
+
/// Repository name
12
+
pub name: Box<str>,
13
+
}
14
+
15
+
#[derive(Deserialize)]
16
+
struct UnvalidatedRepoSpec {
17
+
pub owner: Box<str>,
18
+
pub name: Box<str>,
19
+
}
20
+
21
+
impl TryFrom<UnvalidatedRepoSpec> for RepoSpec {
22
+
type Error = RepoSpecError;
23
+
24
+
#[inline]
25
+
fn try_from(
26
+
UnvalidatedRepoSpec { owner, name }: UnvalidatedRepoSpec,
27
+
) -> Result<Self, Self::Error> {
28
+
Self::from_parts(owner, name)
29
+
}
30
+
}
31
+
32
+
impl RepoSpec {
33
+
pub fn from_parts(
34
+
owner: impl Into<Box<str>>,
35
+
name: impl Into<Box<str>>,
36
+
) -> Result<Self, RepoSpecError> {
37
+
let owner = owner.into();
38
+
let name = name.into();
39
+
40
+
validate(owner.as_ref())?;
41
+
validate(name.as_ref())?;
42
+
43
+
Ok(Self { owner, name })
44
+
}
45
+
46
+
pub const fn owner(&self) -> &str {
47
+
&self.owner
48
+
}
49
+
50
+
pub const fn name(&self) -> &str {
51
+
&self.name
52
+
}
53
+
}
54
+
55
+
impl FromStr for RepoSpec {
56
+
type Err = RepoSpecError;
57
+
58
+
fn from_str(s: &str) -> Result<Self, Self::Err> {
59
+
let Some((owner, name)) = s.split_once('/') else {
60
+
return Err(RepoSpecError::Format);
61
+
};
62
+
63
+
validate(owner)?;
64
+
validate(name)?;
65
+
66
+
Ok(Self {
67
+
owner: owner.into(),
68
+
name: name.into(),
69
+
})
70
+
}
71
+
}
72
+
73
+
pub mod query_param {
74
+
//! Serialize/Deserialize a `RepoSpec` from a string.
75
+
//!
76
+
//! # Example
77
+
//!
78
+
//! ```rust,no_run
79
+
//! # use knot::repospec::RepoSpec;
80
+
//! #[derive(serde::Deserialize)]
81
+
//! struct Params {
82
+
//! #[serde(with = "knot::repospec::query_param")]
83
+
//! repo: RepoSpec,
84
+
//! }
85
+
//! ```
86
+
//!
87
+
use super::RepoSpec;
88
+
use serde::{Deserialize, Deserializer, Serializer};
89
+
use std::str::FromStr;
90
+
91
+
pub fn deserialize<'de, D>(deserializer: D) -> Result<RepoSpec, D::Error>
92
+
where
93
+
D: Deserializer<'de>,
94
+
{
95
+
let s = <&str>::deserialize(deserializer)?;
96
+
let repo = RepoSpec::from_str(s).map_err(serde::de::Error::custom)?;
97
+
Ok(repo)
98
+
}
99
+
100
+
#[allow(unused)]
101
+
pub fn serialize<S>(v: &RepoSpec, serializer: S) -> Result<S::Ok, S::Error>
102
+
where
103
+
S: Serializer,
104
+
{
105
+
serializer.serialize_str(&format!("{}/{}", v.owner(), v.name()))
106
+
}
107
+
}
108
+
109
+
fn validate(name: &str) -> Result<(), RepoSpecError> {
110
+
static FORBIDDEN_SUBSTRINGS: &[&str] = &[".."];
111
+
static FORBIDDEN_PREFIXES: &[&str] = &[".", "-"];
112
+
static ACCEPTED_SYMBOLS: &[char] = &['.', '-', '_', ':'];
113
+
114
+
if name.is_empty() {
115
+
return Err(RepoSpecError::Empty);
116
+
}
117
+
118
+
for value in name.chars() {
119
+
if !(value.is_ascii_alphanumeric() || ACCEPTED_SYMBOLS.contains(&value)) {
120
+
return Err(RepoSpecError::Character { value });
121
+
}
122
+
}
123
+
124
+
for substring in FORBIDDEN_SUBSTRINGS {
125
+
if name.contains(substring) {
126
+
return Err(RepoSpecError::Substring { substring });
127
+
}
128
+
}
129
+
130
+
for prefix in FORBIDDEN_PREFIXES {
131
+
if name.starts_with(prefix) {
132
+
return Err(RepoSpecError::Prefix { prefix });
133
+
}
134
+
}
135
+
136
+
Ok(())
137
+
}
138
+
139
+
#[derive(Debug, thiserror::Error)]
140
+
pub enum RepoSpecError {
141
+
#[error("expected a repository specifier in form '{{owner}}/{{name}}'")]
142
+
Format,
143
+
#[error("repository name cannot be empty")]
144
+
Empty,
145
+
#[error("invalid character in repository name: '{value}'")]
146
+
Character { value: char },
147
+
#[error("invalid sub-string in repository name: '{substring}'")]
148
+
Substring { substring: &'static str },
149
+
#[error("invalid prefix for respository name: '{prefix}'")]
150
+
Prefix { prefix: &'static str },
151
+
}
152
+
153
+
#[cfg(test)]
154
+
mod tests {
155
+
use axum::{
156
+
Json, Router,
157
+
body::Body,
158
+
extract::{Path, Query},
159
+
http::{Request, Response},
160
+
routing,
161
+
};
162
+
use http_body_util::BodyExt as _;
163
+
use serde::Deserialize;
164
+
use tower::ServiceExt as _;
165
+
166
+
use super::RepoSpec;
167
+
168
+
const SAMPLE_OWNER: &str = "did:plc:65gha4t3avpfpzmvpbwovss7";
169
+
const SAMPLE_NAME: &str = "gordian";
170
+
171
+
async fn check_response(res: Response<Body>) {
172
+
let body = res.into_body().collect().await.unwrap();
173
+
let repo: RepoSpec = serde_json::from_slice(&body.to_bytes()).unwrap();
174
+
175
+
assert_eq!(repo.owner.as_ref(), SAMPLE_OWNER);
176
+
assert_eq!(repo.name.as_ref(), SAMPLE_NAME);
177
+
}
178
+
179
+
#[tokio::test]
180
+
async fn extract_from_query() {
181
+
#[derive(Deserialize)]
182
+
struct Params {
183
+
#[serde(with = "super::query_param")]
184
+
repo: RepoSpec,
185
+
}
186
+
187
+
let res = Router::new()
188
+
.route(
189
+
"/something",
190
+
routing::get(async |Query(Params { repo }): Query<Params>| Json(repo)),
191
+
)
192
+
.oneshot(
193
+
Request::builder()
194
+
.uri(format!("/something?repo={SAMPLE_OWNER}/{SAMPLE_NAME}"))
195
+
.body(Body::empty())
196
+
.unwrap(),
197
+
)
198
+
.await
199
+
.unwrap();
200
+
201
+
check_response(res).await;
202
+
}
203
+
204
+
#[tokio::test]
205
+
async fn extract_from_path() {
206
+
let res = Router::new()
207
+
.route(
208
+
"/{owner}/{name}",
209
+
routing::get(async |Path(repo): Path<RepoSpec>| Json(repo)),
210
+
)
211
+
.oneshot(
212
+
Request::builder()
213
+
.uri(format!("/{SAMPLE_OWNER}/{SAMPLE_NAME}"))
214
+
.body(Body::empty())
215
+
.unwrap(),
216
+
)
217
+
.await
218
+
.unwrap();
219
+
220
+
check_response(res).await;
221
+
}
222
+
223
+
#[tokio::test]
224
+
async fn extract_from_path_backwards() {
225
+
// Ensure we are extracting placeholders by name not position.
226
+
227
+
let res = Router::new()
228
+
.route(
229
+
"/{name}/{owner}",
230
+
routing::get(async |Path(repo): Path<RepoSpec>| Json(repo)),
231
+
)
232
+
.oneshot(
233
+
Request::builder()
234
+
.uri(format!("/{SAMPLE_NAME}/{SAMPLE_OWNER}"))
235
+
.body(Body::empty())
236
+
.unwrap(),
237
+
)
238
+
.await
239
+
.unwrap();
240
+
241
+
check_response(res).await;
242
+
}
243
+
}
+15
-8
crates/knot/src/types.rs
+15
-8
crates/knot/src/types.rs
···
31
31
}
32
32
33
33
pub mod repo {
34
-
use crate::{ObjectId, RepoPath, objectid::Array};
34
+
use crate::{ObjectId, objectid::Array, repospec::RepoSpec};
35
35
use serde::{Deserialize, Serialize};
36
36
use std::{borrow::Cow, collections::HashMap, path::PathBuf};
37
37
use time::OffsetDateTime;
38
38
39
39
#[derive(Debug, Deserialize)]
40
40
pub struct GetDefaultBranchParams {
41
-
pub repo: RepoPath,
41
+
#[serde(with = "crate::repospec::query_param")]
42
+
pub repo: RepoSpec,
42
43
}
43
44
44
45
/// Response type for the `sh.tangled.repo.getDefaultBranch` query.
···
113
112
114
113
#[derive(Debug, Deserialize)]
115
114
pub struct BranchesParams {
116
-
pub repo: RepoPath,
115
+
#[serde(with = "crate::repospec::query_param")]
116
+
pub repo: RepoSpec,
117
117
#[serde(default = "branches_limit_default")]
118
118
pub limit: u16,
119
119
#[serde(default)]
···
143
141
144
142
#[derive(Debug, Deserialize)]
145
143
pub struct LogParams {
146
-
pub repo: RepoPath,
144
+
#[serde(with = "crate::repospec::query_param")]
145
+
pub repo: RepoSpec,
147
146
#[serde(rename = "ref")]
148
147
pub rev: Option<String>,
149
148
// @NOTE The lexicon states `cursor` is a string containing a pagination
···
162
159
163
160
#[derive(Debug, Deserialize)]
164
161
pub struct TagParams {
165
-
pub repo: RepoPath,
162
+
#[serde(with = "crate::repospec::query_param")]
163
+
pub repo: RepoSpec,
166
164
#[serde(default)]
167
165
pub cursor: usize,
168
166
#[serde(default = "tag_limit_default")]
···
211
207
212
208
#[derive(Debug, Deserialize)]
213
209
pub struct TreeParams {
214
-
pub repo: RepoPath,
210
+
#[serde(with = "crate::repospec::query_param")]
211
+
pub repo: RepoSpec,
215
212
#[serde(rename = "ref")]
216
213
pub rev: Option<String>,
217
214
pub path: Option<PathBuf>,
···
240
235
241
236
#[derive(Debug, Deserialize)]
242
237
pub struct BlobParams {
243
-
pub repo: RepoPath,
238
+
#[serde(with = "crate::repospec::query_param")]
239
+
pub repo: RepoSpec,
244
240
#[serde(rename = "ref")]
245
241
pub rev: Option<String>,
246
242
pub path: PathBuf,
···
333
327
334
328
#[derive(Debug, Deserialize)]
335
329
pub struct DiffParams {
336
-
pub repo: RepoPath,
330
+
#[serde(with = "crate::repospec::query_param")]
331
+
pub repo: RepoSpec,
337
332
#[serde(rename = "ref")]
338
333
pub rev: String,
339
334
}