Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vdods opened this issue Jul 29, 2023 · 0 comments
Open

Comments

@vdods
Copy link
Contributor

vdods commented Jul 29, 2023

I found an error in a refactor done in commit bd244df 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant