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

Cycle detection doesn't catch cycles through collections #652

Closed
cushon opened this issue Nov 27, 2021 · 4 comments
Closed

Cycle detection doesn't catch cycles through collections #652

cushon opened this issue Nov 27, 2021 · 4 comments

Comments

@cushon
Copy link
Contributor

cushon commented Nov 27, 2021

The cycle detection introduced in #632 / #645 doesn't handle cycles that involved collections. The following example still results in a StackOverflowError:

import java.util.Collections;
import java.util.List;
import org.json.JSONObject;

class Json {

  public static void main(String[] args) {
    new JSONObject(new Foo());
  }

  public static class Foo {

    public List<Foo> getFoos() {
      return Collections.singletonList(this);
    }
  }
}
Exception in thread "main" java.lang.StackOverflowError
	at org.json.JSONObject.getKeyNameFromMethod(JSONObject.java:1591)
	at org.json.JSONObject.populateMap(JSONObject.java:1548)
	at org.json.JSONObject.populateMap(JSONObject.java:1529)
	at org.json.JSONObject.<init>(JSONObject.java:366)
	at org.json.JSONObject.wrap(JSONObject.java:2499)
	at org.json.JSONObject.wrap(JSONObject.java:2457)
	at org.json.JSONArray.addAll(JSONArray.java:1609)
	at org.json.JSONArray.<init>(JSONArray.java:176)
	at org.json.JSONObject.wrap(JSONObject.java:2478)
	at org.json.JSONObject.populateMap(JSONObject.java:1563)
	at org.json.JSONObject.populateMap(JSONObject.java:1529)
	at org.json.JSONObject.<init>(JSONObject.java:366)
@stleary
Copy link
Owner

stleary commented Dec 1, 2021

To be fixed

@stleary stleary closed this as completed Dec 1, 2021
@johnjaylward
Copy link
Contributor

@stleary this is a different issue than the one fixed in #651

@hendrixjoseph
Copy link

I have a fix for this (plus a unit test), as well as a couple other locations this might still be happening. I'll try to make a PR tomorrow.

Can you assign me please? Thank you.

@stleary
Copy link
Owner

stleary commented Sep 4, 2023

I don't think it will be possible to check for cycles in collections without impacting performance. If you disagree, post here to reopen this ticket.

@stleary stleary closed this as completed Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants