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

Error deserializing django SafeString #478

Closed
maxkylu opened this issue Mar 5, 2024 · 6 comments
Closed

Error deserializing django SafeString #478

maxkylu opened this issue Mar 5, 2024 · 6 comments

Comments

@maxkylu
Copy link

maxkylu commented Mar 5, 2024

Serializing and then deserializing a django SafeString runs into error:
AttributeError: 'SafeString' object attribute 'capitalize' is read-only

jsonpickle version: 3.0.3
django version: 4.2.11

@maxkylu
Copy link
Author

maxkylu commented Mar 5, 2024

This script reproduces the error independent from django

import jsonpickle

########################################
### Verbatim copy from django 4.2.11 ###
########################################
class SafeData:
    __slots__ = ()

    def __html__(self):
        """
        Return the html representation of a string for interoperability.

        This allows other template engines to understand Django's SafeData.
        """
        return self


class SafeString(str, SafeData):
    """
    A str subclass that has been specifically marked as "safe" for HTML output
    purposes.
    """

    __slots__ = ()

    def __add__(self, rhs):
        """
        Concatenating a safe string with another safe bytestring or
        safe string is safe. Otherwise, the result is no longer safe.
        """
        t = super().__add__(rhs)
        if isinstance(rhs, SafeData):
            return SafeString(t)
        return t

    def __str__(self):
        return self

    def __str__(self):
        return self

### End copy from django 4.2.11 ###

s = SafeString("hi")

pickled = jsonpickle.encode(s)
print(pickled)
jsonpickle.decode(pickled)

@maxkylu
Copy link
Author

maxkylu commented Mar 5, 2024

This is an minimal reproducing example:

import jsonpickle

class SafeData:
    __slots__ = ()

class SafeString(str, SafeData):
    __slots__ = ()


s = SafeString("hi")

pickled = jsonpickle.encode(s)
print(pickled)
jsonpickle.decode(pickled)


@Theelx
Copy link
Contributor

Theelx commented Mar 9, 2024

Hey, thanks for reporting this! Sorry for the late response, I've been busy with schoolwork. I'll take a look at this shortly.

@Theelx
Copy link
Contributor

Theelx commented Mar 10, 2024

This looks to be very similar to #422 (comment). In that case, the issue is because the attribute is being inherited from a class implemented in C, which can make read-only attributes. The fix I implemented in that case was to skip it for subclasses of int, but since this is happening for subclasses of str too, I'll add str to the list of parent classes that can be skipped. Eventually, I'll make a more flexible check that doesn't require us to whitelist types, but that's going to have to be on the TODO list for now.

Theelx added a commit to Theelx/jsonpickle that referenced this issue Mar 10, 2024
@Theelx Theelx closed this as completed in 423faa8 Mar 11, 2024
@maxkylu
Copy link
Author

maxkylu commented Mar 12, 2024

Thank you for this speedy fix.

Question: Wouldn't it be better to fix the pickler to not pickle the read only attributes? The pickled string methods actually only spam the serialized data.

@Theelx
Copy link
Contributor

Theelx commented Mar 12, 2024

I thought about that, but I didn't bother trying because I didn't think it would be serializable if it didn't include that data. However, you make a good point that read-only attributes won't change and so they spam the serialized data. I'll look into working around that that, hopefully I can get a change landed soon to reduce the spammed read-only attributes while still preserving unpickleability.

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

No branches or pull requests

2 participants