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

escape() fails with certain str subclasses #472

Closed
sgfost opened this issue Oct 10, 2024 · 7 comments · Fixed by #477
Closed

escape() fails with certain str subclasses #472

sgfost opened this issue Oct 10, 2024 · 7 comments · Fixed by #477
Milestone

Comments

@sgfost
Copy link

sgfost commented Oct 10, 2024

I have a very similar issue to #467, however it is one that persists after the 3.0.1 patch.

Essentially what is happening is that str subclasses which override __str__() are no longer (since 2.1.5) handled in the same way

I encountered this with django's SafeString, however for the purpose of repro I have a simpler example below:

from markupsafe import _escape_inner

class NewString(str):
  def __str__(self):
    return self

s = NewString("abc")
_escape_inner(s)

output:

---------------------------------------------------------------------------
SystemError                               Traceback (most recent call last)
Cell In[4], line 1
----> 1 _escape_inner(s)

SystemError: <built-in function _escape_inner> returned NULL without setting an exception

Environment:

  • Python version: 3.10.12
  • MarkupSafe version: 3.0.1
@davidism
Copy link
Member

A similar thing to the previous issue is happening. The object isn't a direct str, so it goes to the str(o) path, which results in the same object, still not a str. So the C code still gets unexpected data.

@karolyi

This comment was marked as off-topic.

@karolyi

This comment was marked as outdated.

@davidism
Copy link
Member

davidism commented Oct 16, 2024

I'll still fix this. But I'm pretty sure __str__ returning anything other than an actual str type is not correct, even if it works. https://docs.python.org/3/reference/datamodel.html#object.__str__

The return value must be a string object.

Regardless of the fix here, this should probably be reported as a bug in any code returning a str subclass instead of a str.

That's probably why MarkupSafe provides the soft_str function rather than overriding Markup.__str__.

@karolyi
Copy link

karolyi commented Oct 16, 2024

@davidism it is a bug, no one claimed otherwise. I just posted a workaround since I needed it too.

@davidism
Copy link
Member

davidism commented Oct 16, 2024

I'm saying the class producing this error is also a bug. Outside of this library.

@davidism
Copy link
Member

MarkupSafe 3.0.2 is available on PyPI.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants