[Django] #36124: Importing from django.contrib.admindocs.views modifies docutils rst parser

[Django]

#36124: Importing from django.contrib.admindocs.views modifies docutils rst parser
-------------------------------------+-------------------------------------
Reporter: Michal Čihař | Type:
| Uncategorized
Status: new | Component:
| contrib.admindocs
Version: 5.1 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Doing `from django.contrib.admindocs.views import simplify_regex` is
enough to make admindocs customize docutils rst parser what might have
undesired side effects. I've ran into this via django-rest-framework
(https://github.com/encode/django-rest-framework/issues/9626), but there
are apparently more users of this function
(https://github.com/search?q=%22from+django.contrib.admindocs.views+import+simplify_regex%22&type=code).
Not sure if this interface is considered public or not, but apparently it
got some users.

Docutils lack of local registry (https://sourceforge.net/p/docutils
/feature-requests/38/) so there is currently no way to make the
customization local to admindocs.

Would it be possible to separate simplify_regex logic to some utility
module that could be reused by others and would not suffer such side
effects? Any other ideas how to address this?
--
Ticket URL: <https://code.djangoproject.com/ticket/36124>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36124: Importing from django.contrib.admindocs.views modifies docutils rst parser
-------------------------------------+-------------------------------------

Reporter: Michal Čihař | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution: needsinfo
Keywords: docutils | Triage Stage:
simplify_regex roles | Unreviewed
register_canonical_role |

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* keywords: => docutils simplify_regex roles register_canonical_role
* resolution: => needsinfo
* status: new => closed
* type: Uncategorized => Cleanup/optimization
* version: 5.1 => dev

Comment:

Replying to [ticket:36124 Michal Čihař]:

Hello Michal, thank you for taking the time to create this ticket.

> Doing `from django.contrib.admindocs.views import simplify_regex` is
enough to make admindocs customize docutils rst parser what might have
undesired side effects.

I see your point, but I also see that Django registers a specific role,
with a non common name `cmsreference`. Could you provide more details on
how this would produce undesired side effects (other than having a new
role defined)?

> Not sure if this interface is considered public or not, but apparently
it got some users.

This interface is definitely NOT considered public, is not documented nor
advertised in any way.

> Would it be possible to separate simplify_regex logic to some utility
module that could be reused by others and would not suffer such side
effects? Any other ideas how to address this?

For questions like this, the
[https://forum.djangoproject.com/c/internals/5 Django Forum] would be a
great place to seek feedback and suggestions. The forum is read by a
broader audience, including contributors and users who can provide a wider
range of insights and potential solutions. This ticket tracker, on the
other hand, is primarily followed by the Django Fellows, so you might not
receive as much input from the broader community here.

I'll be closing this ticket as `needsinfo` following the
[https://docs.djangoproject.com/en/dev/internals/contributing/triaging-
tickets/#closing-tickets ticket triaging process]. To me, while the
potential for undesirable side effects exists, these appear to be
theoretical at this point. In order to fully understand the scope of the
issue, we would need to investigate specific use cases and determine under
what conditions this could become problematic, so please reopen when you
can provide further details.

Thanks again!
--
Ticket URL: <https://code.djangoproject.com/ticket/36124#comment:1>

[Django]

#36124: Importing from django.contrib.admindocs.views modifies docutils rst parser
-------------------------------------+-------------------------------------
Reporter: Michal Čihař | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution: needsinfo
Keywords: docutils | Triage Stage:
simplify_regex roles | Unreviewed
register_canonical_role |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Michal Čihař):

Replying to [comment:1 Natalia Bidart]:

> I see your point, but I also see that Django registers a specific role,
with a non common name `cmsreference`. Could you provide more details on
how this would produce undesired side effects (other than having a new
role defined)?

It does register other roles:

https://github.com/django/django/blob/e262d5355d82901f81fba6c7015643c2b87125bf/django/contrib/admindocs/utils.py#L94-L100

Using any of them causes crashes while parsing rst via docutils because
the code relies on setting not present in docutils by default:

https://github.com/django/django/blob/e262d5355d82901f81fba6c7015643c2b87125bf/django/contrib/admindocs/utils.py#L132

For now, we work around this by setting this when invoking docutils:

https://github.com/WeblateOrg/weblate/blob/52ab91edd1ff2797da486ddac359688e7cc40744/weblate/checks/markup.py#L495

> To me, while the potential for undesirable side effects exists, these
appear to be theoretical at this point.

I'm not chasing theoretical issues, I'm trying to address issue that hit
me.
--
Ticket URL: <https://code.djangoproject.com/ticket/36124#comment:2>

[Django]

#36124: Importing from django.contrib.admindocs.views modifies docutils rst parser
-------------------------------------+-------------------------------------
Reporter: Michal Čihař | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution: needsinfo
Keywords: docutils | Triage Stage:
simplify_regex roles | Unreviewed
register_canonical_role |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia Bidart):

Replying to [comment:2 Michal Čihař]:

> Replying to [comment:1 Natalia Bidart]:

> > To me, while the potential for undesirable side effects exists, these
appear to be theoretical at this point.
>
> I'm not chasing theoretical issues, I'm trying to address issue that hit
me.

Sorry if my comment came across as dismissive, that wasn’t my intention.
The link to your source code is helpful, but it would be ideal if you
could provide a minimal project that reproduces the issue (or a test case
for the Django test suite). What we're really trying to understand is how
much of the private API is involved in this and whether a solution is
feasible. A simple reproducer would be incredibly helpful in that regard.
--
Ticket URL: <https://code.djangoproject.com/ticket/36124#comment:3>

[Django]

#36124: Importing from django.contrib.admindocs.views modifies docutils rst parser
-------------------------------------+-------------------------------------
Reporter: Michal Čihař | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admindocs | Version: dev
Severity: Normal | Resolution: needsinfo
Keywords: docutils | Triage Stage:
simplify_regex roles | Unreviewed
register_canonical_role |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Michal Čihař):

Using the private API is not required to trigger my original issue. It is
just the way I hit it. The issue can be reproduced by importing
`django.contrib.admindocs.views` or `django.contrib.admindocs.utils`:


{{{
import django.contrib.admindocs.views

import docutils.utils
from docutils.core import Publisher

publisher = Publisher()
publisher.set_components("standalone", "restructuredtext", "null")
settings = publisher.get_settings()
document = docutils.utils.new_document('<rst-doc>', settings=settings)
publisher.reader.parser.parse(":tag:`foo`", document)
}}}

It crashes with

{{{
Traceback (most recent call last):
File "/home/nijel/weblate/weblate/test.py", line 14, in <module>
parser.parse(":tag:`foo`", document)
File "/home/nijel/weblate/weblate/.venv/lib/python3.11/site-
packages/docutils/parsers/rst/__init__.py", line 184, in parse
self.statemachine.run(inputlines, document, inliner=self.inliner)
File "/home/nijel/weblate/weblate/.venv/lib/python3.11/site-
packages/docutils/parsers/rst/states.py", line 169, in run
results = StateMachineWS.run(self, input_lines, input_offset,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/nijel/weblate/weblate/.venv/lib/python3.11/site-
packages/docutils/statemachine.py", line 239, in run
result = state.eof(context)
^^^^^^^^^^^^^^^^^^
File "/home/nijel/weblate/weblate/.venv/lib/python3.11/site-
packages/docutils/parsers/rst/states.py", line 2727, in eof
self.blank(None, context, None)
File "/home/nijel/weblate/weblate/.venv/lib/python3.11/site-
packages/docutils/parsers/rst/states.py", line 2718, in blank
paragraph, literalnext = self.paragraph(
^^^^^^^^^^^^^^^
File "/home/nijel/weblate/weblate/.venv/lib/python3.11/site-
packages/docutils/parsers/rst/states.py", line 416, in paragraph
textnodes, messages = self.inline_text(text, lineno)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/nijel/weblate/weblate/.venv/lib/python3.11/site-
packages/docutils/parsers/rst/states.py", line 425, in inline_text
nodes, messages = self.inliner.parse(text, lineno,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/nijel/weblate/weblate/.venv/lib/python3.11/site-
packages/docutils/parsers/rst/states.py", line 649, in parse
before, inlines, remaining, sysmessages = method(self, match,
^^^^^^^^^^^^^^^^^^^
File "/home/nijel/weblate/weblate/.venv/lib/python3.11/site-
packages/docutils/parsers/rst/states.py", line 792, in
interpreted_or_phrase_ref
nodelist, messages = self.interpreted(rawsource, escaped, role,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/nijel/weblate/weblate/.venv/lib/python3.11/site-
packages/docutils/parsers/rst/states.py", line 889, in interpreted
nodes, messages2 = role_fn(role, rawsource, text, lineno, self)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/nijel/weblate/weblate/.venv/lib/python3.11/site-
packages/django/contrib/admindocs/utils.py", line 116, in _role
inliner.document.settings.link_base,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Values' object has no attribute 'link_base'

}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36124#comment:4>