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

Remove quotes from Specifier.String() #116

Merged
merged 1 commit into from May 5, 2023

Conversation

n8maninger
Copy link
Member

@n8maninger n8maninger commented May 5, 2023

// MarshalText implements encoding.TextMarshaler.
func (uk UnlockKey) MarshalText() ([]byte, error) {
	return marshalHex(uk.Algorithm.String(), uk.Key[:])
}

Causes UnlockKey to JSON encode as "\"ed25519\":48a468d62e48e27ef24707bd1f467476e9605954e201834c659e614b9704d041"

This will likely break a bunch of things in hostd and renterd. Is adding compatibility code for this in core worth it?

@lukechampine
Copy link
Member

dang.

Specifiers are annoying because they aren't always simple strings. For example, siad has a handful of specifiers that end in \n, as well as a few that end in types.RuneToString(2). That's why I chose to quote them in the String method. However, I don't see any "annoying" specifiers in core, renterd, or hostd. So I agree that we should remove the quotes, and maybe add a check to NewSpecifier that rejects non-alphanumeric characters.

As for compatibility... yeah, we should make UnmarshalText handle both cases for a while. We can toss it once we're out of beta.

@n8maninger n8maninger force-pushed the nate/unquote-specifier-string branch from 40928d6 to e0be7ff Compare May 5, 2023 17:49
@lukechampine lukechampine merged commit 2550df5 into master May 5, 2023
6 checks passed
@lukechampine lukechampine deleted the nate/unquote-specifier-string branch May 5, 2023 17:59
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

Successfully merging this pull request may close these issues.

None yet

2 participants