diff options
| author | DanConwayDev <DanConwayDev@protonmail.com> | 2026-01-27 13:56:45 +0000 |
|---|---|---|
| committer | DanConwayDev <DanConwayDev@protonmail.com> | 2026-01-27 20:38:23 +0000 |
| commit | a1573c6018c2e81795dc87d36011604dfed80936 (patch) | |
| tree | 911eac8b14a0ea094a47fe7a7752b0d776f67ecb /src/sync | |
| parent | c9ab6aef228f0a77b2997cfc6bf83d5761ab7e08 (diff) | |
fix: prevent git protocol errors from triggering naughty list
Change protocol error detection to only match WebSocket-specific errors
(websocket, invalid frame) instead of generic 'protocol' keyword which
was incorrectly catching transient git protocol errors.
Git protocol errors like 'fatal: protocol error: bad line length' are
transient network issues that should use backoff/retry, not permanent
naughty list blocking. Only WebSocket/Nostr protocol violations indicate
persistent infrastructure problems.
Fixes production false positive:
- relay.ngit.dev: git protocol error + remote warning misclassified
Add production test cases for git protocol errors and warning combinations.
Diffstat (limited to 'src/sync')
| -rw-r--r-- | src/sync/naughty_list.rs | 77 |
1 files changed, 71 insertions, 6 deletions
diff --git a/src/sync/naughty_list.rs b/src/sync/naughty_list.rs index 60ab949..0abb986 100644 --- a/src/sync/naughty_list.rs +++ b/src/sync/naughty_list.rs | |||
| @@ -232,12 +232,12 @@ impl NaughtyListTracker { | |||
| 232 | } | 232 | } |
| 233 | } | 233 | } |
| 234 | 234 | ||
| 235 | // Protocol errors | 235 | // Protocol errors - specifically WebSocket/Nostr protocol violations |
| 236 | if error_lower.contains("websocket") | 236 | // Note: We check for "websocket" specifically, NOT generic "protocol" keyword |
| 237 | || error_lower.contains("protocol") | 237 | // because git errors often contain "protocol error" (e.g., "fatal: protocol error: bad line length") |
| 238 | || error_lower.contains("invalid frame") | 238 | // which are transient network issues, not persistent infrastructure problems. |
| 239 | { | 239 | if error_lower.contains("websocket") || error_lower.contains("invalid frame") { |
| 240 | // Exclude connection errors | 240 | // Exclude connection errors (transient) |
| 241 | if !error_lower.contains("connection") | 241 | if !error_lower.contains("connection") |
| 242 | && !error_lower.contains("timeout") | 242 | && !error_lower.contains("timeout") |
| 243 | && !error_lower.contains("refused") | 243 | && !error_lower.contains("refused") |
| @@ -622,3 +622,68 @@ mod tests { | |||
| 622 | assert_eq!(NaughtyCategory::ProtocolError.as_str(), "protocol_error"); | 622 | assert_eq!(NaughtyCategory::ProtocolError.as_str(), "protocol_error"); |
| 623 | } | 623 | } |
| 624 | } | 624 | } |
| 625 | |||
| 626 | #[cfg(test)] | ||
| 627 | mod production_tests { | ||
| 628 | use super::*; | ||
| 629 | |||
| 630 | /// Production case from relay.ngit.dev - remote warning should not be classified | ||
| 631 | #[test] | ||
| 632 | fn test_classify_production_relay_ngit_dev_warning() { | ||
| 633 | let error = | ||
| 634 | "remote: warning: unable to access '/root/.config/git/attributes': Permission denied"; | ||
| 635 | assert_eq!(NaughtyListTracker::classify_error(error), None); | ||
| 636 | } | ||
| 637 | |||
| 638 | /// Git protocol errors are transient, not persistent infrastructure issues | ||
| 639 | #[test] | ||
| 640 | fn test_git_protocol_errors_not_naughty() { | ||
| 641 | // These are common git protocol errors that should NOT be classified as naughty | ||
| 642 | let git_protocol_errors = [ | ||
| 643 | "fatal: protocol error: bad line length character: remo", | ||
| 644 | "fatal: protocol error: expected old/new/ref, got 'shallow", | ||
| 645 | "fatal: git upload-pack: protocol error", | ||
| 646 | "error: protocol error: bad pack header", | ||
| 647 | "fatal: protocol error: bad band #3", | ||
| 648 | ]; | ||
| 649 | |||
| 650 | for error in git_protocol_errors { | ||
| 651 | assert_eq!( | ||
| 652 | NaughtyListTracker::classify_error(error), | ||
| 653 | None, | ||
| 654 | "Git protocol error should not be classified as naughty: {}", | ||
| 655 | error | ||
| 656 | ); | ||
| 657 | } | ||
| 658 | } | ||
| 659 | |||
| 660 | /// Remote warning followed by git protocol error - both should be filtered/ignored | ||
| 661 | #[test] | ||
| 662 | fn test_warning_with_git_protocol_error() { | ||
| 663 | let error = "remote: warning: unable to access '/root/.config/git/attributes': Permission denied\nfatal: protocol error: bad line length character: remo"; | ||
| 664 | assert_eq!( | ||
| 665 | NaughtyListTracker::classify_error(error), | ||
| 666 | None, | ||
| 667 | "Warning + git protocol error should not be classified as naughty" | ||
| 668 | ); | ||
| 669 | } | ||
| 670 | |||
| 671 | /// WebSocket protocol errors ARE naughty (persistent infrastructure issues) | ||
| 672 | #[test] | ||
| 673 | fn test_websocket_errors_still_naughty() { | ||
| 674 | let websocket_errors = [ | ||
| 675 | "websocket protocol error", | ||
| 676 | "websocket handshake failed", | ||
| 677 | "invalid frame received", | ||
| 678 | ]; | ||
| 679 | |||
| 680 | for error in websocket_errors { | ||
| 681 | assert_eq!( | ||
| 682 | NaughtyListTracker::classify_error(error), | ||
| 683 | Some(NaughtyCategory::ProtocolError), | ||
| 684 | "WebSocket error should be classified as protocol_error: {}", | ||
| 685 | error | ||
| 686 | ); | ||
| 687 | } | ||
| 688 | } | ||
| 689 | } | ||