ssi icon indicating copy to clipboard operation
ssi copied to clipboard

Refactor error in PrimaryDIDURL::from_str; missing crucial validity check

Open vdods opened this issue 11 months ago • 0 comments

I found an error in a refactor done in commit bd244dfd0474986aedf529cfb3124252f3a99fd5 which causes a crucial check done in PrimaryDIDURL::from_str to not occur except in cfg(test). The diff in question is

 /// Parse a primary DID URL.
 impl FromStr for PrimaryDIDURL {
     type Err = Error;
     fn from_str(didurl: &str) -> Result<Self, Self::Err> {
         // Allow non-DID URL for testing lds-ed25519-2020-issuer0
-        #[cfg(not(any(test, feature = "testing")))]
+        #[cfg(test)]
         if !didurl.starts_with("did:") {
             return Err(Error::DIDURL);
         }
         if didurl.contains('#') {
             return Err(Error::UnexpectedDIDFragment);
         }

From the comment above the changed line, clearly the intent is only to omit the .starts_with("did:") check when testing lds-ed25519-2020-issuer0, which presumably is what the "testing" feature was for. Though the previous condition also omitted the check in cfg(test).

I think the .starts_with("did:") check is crucial in testing and non-testing, since that's one of the most basic validity checks one can do on a DID. Would it be possible to change this to something like #[cfg(not(feature = "testing-lds-ed25519-2020-issuer0"))] so that the .starts_with("did:") check is always done except in that specific case?

vdods avatar Jul 29 '23 00:07 vdods