RST files are not properly rendered if they contain an image (with docutils=0.13.1)

changed title from Since 8.16.5, RST files are not properly rendered if they contain an image to Since 8.16.5, RST files are not properly rendered if they contain an SVG image

changed title from Since 8.16.5, RST files are not properly rendered if they contain an SVG image to RST files are not properly rendered if they contain an image

I don’t know how and why this broke, but I successfully “fixed” this on my gitlab instance by changing the file vendor/bundle/ruby/2.3.0/gems/gitlab-markup-1.5.1/lib/github/commands like this:

--- rest2html.old       2017-02-16 20:43:29.004451772 +0100
+++ rest2html   2017-02-16 20:41:22.214000870 +0100
@@ -144,7 +144,8 @@
             self.body.pop()
         # add on `img` with attributes
             self.body.append(self.starttag(node, 'img', **atts))
-        self.body.append(self.context.pop())
+        if self.context:
+            self.body.append(self.context.pop())

Maybe there’s something deeper, but I didn’t try anything else.

Because, without this patch, you get:

% ./rest2html.old 
.. image:: http://coucou
Traceback (most recent call last):
  File "./rest2html.old", line 196, in <module>
    sys.stdout.write("%s%s" % (main(), "\n"))
  File "./rest2html.old", line 182, in main
    parts = publish_parts(text, writer=writer, settings_overrides=SETTINGS)
  File "/usr/lib/python2.7/site-packages/docutils/core.py", line 448, in publish_parts
    enable_exit_status=enable_exit_status)
  File "/usr/lib/python2.7/site-packages/docutils/core.py", line 662, in publish_programmatically
    output = pub.publish(enable_exit_status=enable_exit_status)
  File "/usr/lib/python2.7/site-packages/docutils/core.py", line 219, in publish
    output = self.writer.write(self.document, self.destination)
  File "/usr/lib/python2.7/site-packages/docutils/writers/__init__.py", line 80, in write
    self.translate()
  File "/usr/lib/python2.7/site-packages/docutils/writers/_html_base.py", line 71, in translate
    self.document.walkabout(visitor)
  File "/usr/lib/python2.7/site-packages/docutils/nodes.py", line 174, in walkabout
    if child.walkabout(visitor):
  File "/usr/lib/python2.7/site-packages/docutils/nodes.py", line 187, in walkabout
    visitor.dispatch_departure(self)
  File "/usr/lib/python2.7/site-packages/docutils/nodes.py", line 1895, in dispatch_departure
    return method(node)
  File "./rest2html.old", line 147, in depart_image
    self.body.append(self.context.pop())
IndexError: pop from empty list

@louiz Please could you create an example project here on GitLab.com with the failing .RST file? Thanks!

added ~65372 label

So, this is weird, I can’t reproduce on gitlab.com (See https://gitlab.com/louiz/rst-with-image)

But, I found more: by downgrading from docutils 0.13.1 (was release in december 2016, so not long ago) to 0.12 (2014), my previous ./rest2html example works.

I guess on gitlab.com, docutils=0.12 is installed, but I have 0.13.1 installed (@Eijebong probably has version 0.13.1 too, as he’s using Archlinux on the server, I believe).

Proof:

gitlab@fenouil % cat test.rst
.. image:: http://coucou

gitlab@fenouil % python -c "import docutils; print(docutils.__version__)" 
0.13.1

gitlab@fenouil % ./rest2html.old test.rst
Traceback (most recent call last):
  File "./rest2html.old", line 196, in <module>
    sys.stdout.write("%s%s" % (main(), "\n"))
  File "./rest2html.old", line 182, in main
    parts = publish_parts(text, writer=writer, settings_overrides=SETTINGS)
  File "/usr/lib/python2.7/site-packages/docutils/core.py", line 448, in publish_parts
    enable_exit_status=enable_exit_status)
  File "/usr/lib/python2.7/site-packages/docutils/core.py", line 662, in publish_programmatically
    output = pub.publish(enable_exit_status=enable_exit_status)
  File "/usr/lib/python2.7/site-packages/docutils/core.py", line 219, in publish
    output = self.writer.write(self.document, self.destination)
  File "/usr/lib/python2.7/site-packages/docutils/writers/__init__.py", line 80, in write
    self.translate()
  File "/usr/lib/python2.7/site-packages/docutils/writers/_html_base.py", line 71, in translate
    self.document.walkabout(visitor)
  File "/usr/lib/python2.7/site-packages/docutils/nodes.py", line 174, in walkabout
    if child.walkabout(visitor):
  File "/usr/lib/python2.7/site-packages/docutils/nodes.py", line 187, in walkabout
    visitor.dispatch_departure(self)
  File "/usr/lib/python2.7/site-packages/docutils/nodes.py", line 1895, in dispatch_departure
    return method(node)
  File "./rest2html.old", line 147, in depart_image
    self.body.append(self.context.pop())
IndexError: pop from empty list

9:26 gitlab@fenouil % pip install docutils==0.12 --user
Collecting docutils==0.12
  Using cached docutils-0.12.tar.gz
Installing collected packages: docutils
  Running setup.py install for docutils ... done
Successfully installed docutils-0.13.1

gitlab@fenouil % python -c "import docutils; print(docutils.__version__)" 
0.12 
gitlab@fenouil % ./rest2html.old test.rst                                 
<img alt="http://coucou" src="http://coucou" />

>>> import docutils
>>> docutils.__version__
'0.13.1'

@louiz Yeah, docutils 0.13.1 here, same errror

/cc @brodock

rest2html was patched in a previous release to prevent raw HTML from being used in cross-site scripting attacks:

https://about.gitlab.com/2017/01/10/gitlab-8-dot-15-dot-4-released/

https://gitlab.com/gitlab-org/gitlab-ce/issues/26411

closed

I think the issue is not related to that patch, @briann, but with a patch to support python 2 and python 3, so I'm reopenning this.

reopened

Looks like I haven't merged the MR: https://gitlab.com/gitlab-org/gitlab-markup/merge_requests/5. I can take a look on this next week only, if someone can try the solution there, we can speed this up.

gitlab-markup!5 (merged) seems to fix it for docutils 0.13, but breaks if we still have 0.12 installed (I’ve left a comment there with more details).

@brodock Can we close off this issue?

mentioned in issue gitlab-foss#29793 (closed)

@markglenfletcher It's not fixed yet. I will take a look into that now.

changed milestone to %9.1 in gitlab-foss

assigned to @brodock

I can't reproduce the issue anymore with current gitlab version. Can you confirm you are still experiencing the issue @louiz ?

@louiz Please can you provide an update for this issue? Thanks!

Using gitlab v9.1.0-rc3 (although I’m not 100% sure this updates github-markup as well, how do I make sure of this? I install gitlab by cloning the git repository, calling bundle install and all that stuff), this is broken with python-docutils=0.13.1 but it works with python-docutils=0.12.

The installed version of python3-docutils doesn’t change anything.

I haven't released a new "gitlab-markup" gem yet. Will do later today, but I can't promise it will be available in 9.2 as this is working well in omnibus install and I have some higher priority issues to tackle before the feature freeze for 9.2. If you want to try in a source install it should be easily to update the Gemfile and point to 1.6.0 version.

https://rubygems.org/gems/gitlab-markup/versions/1.6.0

added enhancement in GitLab FOSS and removed ~65372 labels

changed title from RST files are not properly rendered if they contain an image to RST files are not properly rendered if they contain an image (with docutils=0.13.1)

changed milestone to %9.2 in gitlab-foss

changed milestone to %9.3 in gitlab-foss

mentioned in issue omnibus-gitlab#2361 (closed)

There are discussions going on here: https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2361 about moving away from docutils to Sphinx. I've asked there if we want to close this in favor of the other, or if we should upgrade the omnibus so we release something for %9.3 in gitlab-foss