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

Sentry fails to serialize Locale #2301

Closed
marek-klima opened this issue Oct 17, 2022 · 7 comments · Fixed by #2309
Closed

Sentry fails to serialize Locale #2301

marek-klima opened this issue Oct 17, 2022 · 7 comments · Fixed by #2309
Assignees
Labels
Platform: Java Type: Bug Something isn't working

Comments

@marek-klima
Copy link

marek-klima commented Oct 17, 2022

Integration

sentry

Java Version

17

Version

6.4.3

Steps to Reproduce

Running a code that tries to encode object of type Locale never ends. It probably stays inside JsonReflectionObjectSerializer (here) because object.getClass().getDeclaredFields() returns long list.

There were recent fixes for enums (#2220).

import io.sentry.JsonObjectWriter;
import io.sentry.NoOpLogger;
import java.io.IOException;
import java.io.StringWriter;
import java.util.HashMap;
import java.util.Locale;

public class StartSentry {
    public static void main(String[] args) {
        JsonObjectWriter writer = new JsonObjectWriter(new StringWriter(), 10);
        HashMap<String, Locale> map = new HashMap<>();
        map.put("one", Locale.GERMAN);
        try {
            writer.value(
                    NoOpLogger.getInstance(),
                    map
            );
        } catch (IOException e) {
            System.out.println("IO exception " + e);
        }
    }
}

The same happens on newest version 6.5.0.

Expected Result

Locale is correctly serialized

Actual Result

Potential Out of Memory exception

@philipphofmann
Copy link
Member

philipphofmann commented Oct 17, 2022

@marek-klima, did this work on any of the previous versions? If yes, which version was the last one it still worked?

@marek-klima
Copy link
Author

@marek-klima, did this work on any of the previous versions?

Unfortunately I'm now aware of any older version where this would work correctly.

@romtsn
Copy link
Member

romtsn commented Oct 17, 2022

Hmm, do you really want to serialize the entire Locale object? I guess our serializer is not designed to serialize large deeply-nested object hierarchies.. On the other hand it should in theory function the same way Gson reflection serializer works.

@marek-klima
Copy link
Author

Right, there is no need to serialize the entire Locale object - it should be serialized to something like de_de in the given example. Also, this issue is affecting any large deeply-nested object, not only Locale.

@romtsn romtsn added Type: Bug Something isn't working and removed Status: Untriaged labels Oct 17, 2022
@romtsn
Copy link
Member

romtsn commented Oct 17, 2022

Yeah we definitely need to investigate it, that's for sure

@adinauer
Copy link
Member

I'll take a look

@adinauer adinauer self-assigned this Oct 18, 2022
@adinauer
Copy link
Member

We could skip static fields when going through the fields but in this case it would lead to an empty object being serialized for Locale. We could fall back to toString() in that case as a generic solution. WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: Java Type: Bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants