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

Jump to bottom

encoding/json: cyclic maps and slices are not detected #40745

Closed
lujjjh opened this issue Aug 13, 2020 · 6 comments
Closed

encoding/json: cyclic maps and slices are not detected #40745

lujjjh opened this issue Aug 13, 2020 · 6 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.

Comments

@lujjjh
Copy link
Contributor

lujjjh commented Aug 13, 2020

The documentation says:

JSON cannot represent cyclic data structures and Marshal does not handle them. Passing cyclic structures to Marshal will result in an error.

However, there are still cases result in an infinite recursion in Go 1.15.

It seems that 64c9ee9 does not handle cyclic maps or slices:

package main

import "encoding/json"

func main() {
	x := map[string]interface{}{}
	x["x"] = x
	json.Marshal(x)
}

and

package main

import "encoding/json"

func main() {
	x := []interface{}{nil}
	x[0] = x
	json.Marshal(x)
}

Originally posted by @lujjjh in #10769 (comment)
@lujjjh lujjjh mentioned this issue Aug 13, 2020
Closed
@mvdan
Copy link
Member

mvdan commented Aug 13, 2020

I agree that this is probably a case we missed and should be fixed. Do you want to send a CL?

@mvdan mvdan added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Aug 13, 2020
@lujjjh lujjjh mentioned this issue Aug 13, 2020
Closed
@gopherbot
Copy link

Change https://golang.org/cl/248358 mentions this issue: encoding/json: detect cyclic maps and slices

@eaglexiang
Copy link

great, new test case TestSliceNoCycle was created, but why there is no TestSliceCycle now and even no TestPointerCycle in the past.

@lujjjh
Copy link
Contributor Author

lujjjh commented Aug 14, 2020

great, new test case TestSliceNoCycle was created, but why there is no TestSliceCycle now and even no TestPointerCycle in the past.

@eaglexiang Thank you for reviewing the code. Since marshalling cyclic structures results in a UnsupportedValueError, these cases are tested in TestUnsupportedValues.

@lujjjh
Copy link
Contributor Author

lujjjh commented Aug 21, 2020

I agree that this is probably a case we missed and should be fixed. Do you want to send a CL?

@mvdan I sent a CL last week, but received no feedback.

This is my first time to conribute to Go. What should I do next? Or just wait?

Thank you.

@mvdan
Copy link
Member

mvdan commented Aug 21, 2020 •
edited

@lujjjh yes, I was pinged by Gerrit a week ago, and I've seen your ping yesterday on Gerrit and your ping here today. I'm taking some time off this week, or at least trying to. A week is not a long time to wait for a review, especially given that it's August.

I'll likely give a review sometime next week, unless someone beats me to it. But in general, please be a bit patient.

@bep bep mentioned this issue Apr 20, 2021
Closed
@golang golang locked and limited conversation to collaborators Sep 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

encoding/json: detect cyclic maps and slices
4 participants
@lujjjh @mvdan @gopherbot @eaglexiang