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.

Sign up for GitHub

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

Jump to bottom

math/big: nil *Int appears to not roundtrip with MarshalText and UnmarshalText #60489

Open
mvdan opened this issue May 29, 2023 · 4 comments
Open

math/big: nil *Int appears to not roundtrip with MarshalText and UnmarshalText #60489

mvdan opened this issue May 29, 2023 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@mvdan
Copy link
Member

mvdan commented May 29, 2023 •
edited

https://go.dev/play/p/uV8Vr7trqBv

https://pkg.go.dev/encoding#TextUnmarshaler says:

UnmarshalText must be able to decode the form generated by MarshalText.

MarshalText clearly supports a nil *big.Int value, marshaling it as <nil>, but UnmarshalText rejects it. I think this goes against the TextUnmarshaler docs.

I'm not arguing that marshaling and unmarshaling a nil big int makes any sense, for what it's worth. Just that I always assumed, like the docs say, that roundtripping a value via marshal and then unmarshal was a safe thing to do. If marshaling allows for a nil pointer, unmarshal should support it too - although I'm not sure what it could possibly do, given that it can't set a *big.Int receiver to nil. Perhaps set it to its zero value?

Note that MarshalJSON and UnmarshalJSON don't have this problem. They both support null, and the unmarshal is simply a no-op. Presumably we could do the same in UnmarshalText.
@mknyszek
Copy link
Contributor

CC @griesemer @golang/security via https://dev.golang.org/owners

@mknyszek mknyszek added this to the Backlog milestone May 30, 2023
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 30, 2023
@Jorropo
Copy link
Member

Jorropo commented May 31, 2023

FWIW GobDecode does zero on nil values:

go/src/math/big/intmarsh.go

Lines 33 to 38 in 9552a11

func (z *Int) GobDecode(buf []byte) error {
if len(buf) == 0 {
// Other side sent a nil or default value.
*z = Int{}
return nil
}

@Jorropo
Copy link
Member

Jorropo commented May 31, 2023

But not for json where here it does nothing:

go/src/math/big/intmarsh.go

Lines 77 to 83 in 9552a11

func (z *Int) UnmarshalJSON(text []byte) error {
// Ignore null, like in the main JSON package.
if string(text) == "null" {
return nil
}
return z.UnmarshalText(text)
}

@mvdan
Copy link
Member Author

mvdan commented May 31, 2023

I would have expected them all to zero out the receiver, although I imagine that a non-nil and zero *Int does not encode or stringify the same way as a nil *Int. Still, at least as far as encoding/json goes, a null generally decodes by zeroing rather than doing nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@mknyszek @mvdan @Jorropo