diff options
| author | DanConwayDev <DanConwayDev@protonmail.com> | 2026-04-10 16:42:35 +0000 |
|---|---|---|
| committer | DanConwayDev <DanConwayDev@protonmail.com> | 2026-04-10 16:42:35 +0000 |
| commit | dfd20a39a7ddaea07103cac45d4d79bc7e6ce0d7 (patch) | |
| tree | f4d3c38c09c7b27a25f6b6933c9de0e42149c82f /src/nostr | |
| parent | 2d74b9ca69b3a1e0b9a2359c12cc2d1979fc6130 (diff) | |
fix: accept any d-tag identifier; percent-encode in URLs
NIP-01 places no restriction on d tag characters and NIP-34 only
recommends kebab-case without mandating it. Rejecting identifiers with
whitespace or other URL-unsafe characters was therefore overly strict.
The correct approach (per NIP-34 PR #2312 and GRASP-01) is to store
identifiers verbatim on disk and percent-encode them when constructing
URLs. The previous commit already handled the incoming direction
(percent-decoding URL paths before filesystem lookup); this commit
handles the outgoing direction and removes the validation restriction.
Changes:
- validate_identifier: drop whitespace rejection; only reject chars
that are unsafe as filesystem directory names (/, \, null, . / ..)
- git/mod.rs: add percent_encode() alongside percent_decode()
- landing.rs: percent-encode identifier in nostr:// clone URL and
gitworkshop link (also fixes a pre-existing bug where the clone URL
displayed literal '{npub}' / '{identifier}' instead of the values)
Diffstat (limited to 'src/nostr')
| -rw-r--r-- | src/nostr/events.rs | 59 |
1 files changed, 24 insertions, 35 deletions
diff --git a/src/nostr/events.rs b/src/nostr/events.rs index 88ed6ae..77a9d9f 100644 --- a/src/nostr/events.rs +++ b/src/nostr/events.rs | |||
| @@ -361,19 +361,20 @@ impl RepositoryState { | |||
| 361 | } | 361 | } |
| 362 | } | 362 | } |
| 363 | 363 | ||
| 364 | /// Validate that a repository identifier is safe for use as a filesystem path component | 364 | /// Validate that a repository identifier is safe for use as a filesystem path component. |
| 365 | /// and as a URL path segment without percent-encoding. | ||
| 366 | /// | 365 | /// |
| 367 | /// Rejects identifiers that: | 366 | /// NIP-34 places no restriction on `d` tag characters beyond NIP-01 (any string). |
| 368 | /// - Are empty | 367 | /// Identifiers are stored on disk verbatim and percent-encoded when used in URLs |
| 369 | /// - Contain path separators (`/`, `\`) | 368 | /// (per NIP-34 `nostr://` spec and GRASP-01 HTTP path spec). This function only |
| 370 | /// - Contain null bytes | 369 | /// rejects identifiers that cannot safely be stored as a filesystem directory name: |
| 371 | /// - Contain whitespace (spaces, tabs, newlines, etc.) — these require percent-encoding | ||
| 372 | /// in URLs and cause a mismatch between the stored path and the URL-decoded request | ||
| 373 | /// - Are `.` or `..` (directory traversal) | ||
| 374 | /// | 370 | /// |
| 375 | /// NIP-34 recommends kebab-case identifiers; this function enforces the minimum | 371 | /// - Empty string |
| 376 | /// safety constraints needed for correct filesystem and HTTP serving behaviour. | 372 | /// - Path separators (`/`, `\`) — would escape the repository directory |
| 373 | /// - Null bytes — rejected by most filesystems | ||
| 374 | /// - `.` or `..` — reserved path components (directory traversal) | ||
| 375 | /// | ||
| 376 | /// Whitespace and other characters that require percent-encoding in URLs are | ||
| 377 | /// explicitly allowed here; callers are responsible for encoding them in URLs. | ||
| 377 | pub fn validate_identifier(identifier: &str) -> Result<(), String> { | 378 | pub fn validate_identifier(identifier: &str) -> Result<(), String> { |
| 378 | if identifier.is_empty() { | 379 | if identifier.is_empty() { |
| 379 | return Err("identifier must not be empty".to_string()); | 380 | return Err("identifier must not be empty".to_string()); |
| @@ -397,12 +398,6 @@ pub fn validate_identifier(identifier: &str) -> Result<(), String> { | |||
| 397 | identifier | 398 | identifier |
| 398 | )); | 399 | )); |
| 399 | } | 400 | } |
| 400 | if ch.is_whitespace() { | ||
| 401 | return Err(format!( | ||
| 402 | "identifier '{}' contains whitespace — use hyphens instead (e.g. 'my-repo')", | ||
| 403 | identifier | ||
| 404 | )); | ||
| 405 | } | ||
| 406 | } | 401 | } |
| 407 | Ok(()) | 402 | Ok(()) |
| 408 | } | 403 | } |
| @@ -1573,6 +1568,11 @@ mod tests { | |||
| 1573 | assert!(validate_identifier("my_repo").is_ok()); | 1568 | assert!(validate_identifier("my_repo").is_ok()); |
| 1574 | assert!(validate_identifier("repo123").is_ok()); | 1569 | assert!(validate_identifier("repo123").is_ok()); |
| 1575 | assert!(validate_identifier("kuboslopp").is_ok()); | 1570 | assert!(validate_identifier("kuboslopp").is_ok()); |
| 1571 | // Whitespace is valid — identifiers are stored verbatim on disk and | ||
| 1572 | // percent-encoded when used in URLs (NIP-34 / GRASP-01) | ||
| 1573 | assert!(validate_identifier("kuboslopp by Shakespeare").is_ok()); | ||
| 1574 | assert!(validate_identifier("my\trepo").is_ok()); | ||
| 1575 | assert!(validate_identifier("my repo").is_ok()); | ||
| 1576 | } | 1576 | } |
| 1577 | 1577 | ||
| 1578 | #[test] | 1578 | #[test] |
| @@ -1593,19 +1593,12 @@ mod tests { | |||
| 1593 | } | 1593 | } |
| 1594 | 1594 | ||
| 1595 | #[test] | 1595 | #[test] |
| 1596 | fn test_validate_identifier_rejects_whitespace() { | 1596 | fn test_validate_announcement_accepts_identifier_with_spaces() { |
| 1597 | assert!(validate_identifier("kuboslopp by Shakespeare").is_err()); | ||
| 1598 | assert!(validate_identifier("my\trepo").is_err()); | ||
| 1599 | assert!(validate_identifier("my\nrepo").is_err()); | ||
| 1600 | } | ||
| 1601 | |||
| 1602 | #[test] | ||
| 1603 | fn test_validate_announcement_rejects_identifier_with_spaces() { | ||
| 1604 | use crate::config::Config; | 1597 | use crate::config::Config; |
| 1605 | use crate::nostr::policy::AnnouncementResult; | 1598 | use crate::nostr::policy::AnnouncementResult; |
| 1606 | 1599 | ||
| 1607 | let keys = create_test_keys(); | 1600 | let keys = create_test_keys(); |
| 1608 | // Identifier contains spaces — should be rejected regardless of clone/relay tags | 1601 | // Identifier contains spaces — valid per NIP-34; must be percent-encoded in URLs |
| 1609 | let event = create_announcement_event( | 1602 | let event = create_announcement_event( |
| 1610 | &keys, | 1603 | &keys, |
| 1611 | "kuboslopp by Shakespeare", | 1604 | "kuboslopp by Shakespeare", |
| @@ -1618,14 +1611,10 @@ mod tests { | |||
| 1618 | ..Config::for_testing() | 1611 | ..Config::for_testing() |
| 1619 | }; | 1612 | }; |
| 1620 | let result = validate_announcement(&event, &config); | 1613 | let result = validate_announcement(&event, &config); |
| 1621 | if let AnnouncementResult::Reject(reason) = result { | 1614 | assert!( |
| 1622 | assert!( | 1615 | matches!(result, AnnouncementResult::Accept), |
| 1623 | reason.contains("whitespace") || reason.contains("identifier"), | 1616 | "Expected Accept for identifier with spaces, got {:?}", |
| 1624 | "unexpected rejection reason: {}", | 1617 | result |
| 1625 | reason | 1618 | ); |
| 1626 | ); | ||
| 1627 | } else { | ||
| 1628 | panic!("Expected Reject for identifier with spaces, got {:?}", result); | ||
| 1629 | } | ||
| 1630 | } | 1619 | } |
| 1631 | } | 1620 | } |