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

New release of mistune is a major version release: please take it up #1685

Open
ViktorHaag opened this issue Dec 6, 2021 · 21 comments
Open

Comments

@ViktorHaag
Copy link

ViktorHaag commented Dec 6, 2021

Mistune's latest release is a major change and a re-design. Now that its version 2.0 is out of a preview release, please consider taking it up here to facilitate supporting downstream CI installs.

It'd be great if nbconvert ensured it worked with the new version of mistune and unpinned its dependency (or repinned to a later version window).

Downstream consumers of nbconvert can pin, but may run into situations where other projects depend on the newer version of mistune.

@michaelaye
Copy link

Yeah:

----> 1 from nbconvert.preprocessors import Preprocessor

/luna4/maye/miniconda3/envs/py38/lib/python3.8/site-packages/nbconvert/__init__.py in <module>
      2 
      3 from ._version import version_info, __version__
----> 4 from .exporters import *
      5 from . import filters
      6 from . import preprocessors

/luna4/maye/miniconda3/envs/py38/lib/python3.8/site-packages/nbconvert/exporters/__init__.py in <module>
      1 from .base import (export, get_exporter, 
      2                    ExporterNameError, get_export_names)
----> 3 from .html import HTMLExporter
      4 from .slides import SlidesExporter
      5 from .templateexporter import TemplateExporter

/luna4/maye/miniconda3/envs/py38/lib/python3.8/site-packages/nbconvert/exporters/html.py in <module>
     13 
     14 from nbconvert.filters.highlight import Highlight2HTML
---> 15 from nbconvert.filters.markdown_mistune import IPythonRenderer, MarkdownWithMath
     16 
     17 from .templateexporter import TemplateExporter

/luna4/maye/miniconda3/envs/py38/lib/python3.8/site-packages/nbconvert/filters/__init__.py in <module>
      4 from .highlight import *
      5 from .latex import *
----> 6 from .markdown import *
      7 from .strings import *
      8 from .metadata import *

/luna4/maye/miniconda3/envs/py38/lib/python3.8/site-packages/nbconvert/filters/markdown.py in <module>
     11 
     12 try:
---> 13     from .markdown_mistune import markdown2html_mistune
     14 except ImportError as e:
     15     # store in variable for Python 3

/luna4/maye/miniconda3/envs/py38/lib/python3.8/site-packages/nbconvert/filters/markdown_mistune.py in <module>
     29 
     30 
---> 31 class MathBlockGrammar(mistune.BlockGrammar):
     32     """This defines a single regex comprised of the different patterns that 
     33     identify math content spanning multiple lines. These are used by the

AttributeError: module 'mistune' has no attribute 'BlockGrammar'

@tobihan
Copy link

tobihan commented Dec 13, 2021

Hi! mistune 2.0 is now in Debian and I am currently working on an updated sagemath package for Debian. Unfortunately sagemath Jupyter notebooks do not work at all because of this. It would be great if you could make nbcovert work with mistune 2.0.

@juliangilbey
Copy link

And likewise me with Spyder 5.x.

@juliangilbey
Copy link

And likewise me with Spyder 5.x.

Though the Spyder team are currently addressing this issue.

@juliangilbey
Copy link

And likewise me with Spyder 5.x.

Though the Spyder team are currently addressing this issue.

Oh sorry, I got confused; the Spyder team are addressing the XIO fatal IO error, which is nothing to do with this issue. The mistune/nbconvert issue is still a blocker for getting the new version of Spyder into Debian.

@blink1073
Copy link
Contributor

blink1073 commented Dec 28, 2021

Note: I took a look at this, and we're using several classes that are no longer available in mistune 2.0, and there is not a migration guide for the new mistune API. Either way, since classes like MarkdownWithMath are part of our public API, we'd have to have a major version bump to accommodate the mistune changes.

@juliangilbey
Copy link

@blink1073 - indeed, and thanks for looking into this. Having a migration guide would be super helpful. Version 0.8.4 of mistune had an API with a publicised description, but version 2.0.0 only has a much smaller public API, documented at https://mistune.readthedocs.io/en/v2.0.0/api.html. There is already an issue against mistune requesting a migration guide: lepture/mistune#290

(There is also a separate question about how much the mistune author expects the API to remain stable.)

Since mistune 2.0 is essentially incompatible with mistune 0.8.4, but cannot be installed simultaneously, it will be necessary for all interacting software to either use mistune 0.8.4 or mistune 2.0.0. So having a mistune 2.0.0 compatible nbconvert will sadly be needed.

@bollwyvl
Copy link
Contributor

I haven't seen a lot of arguments for why nbconvert needs to upgrade other than NEWAR GOOD, especially with no PRs forthcoming. While sensitive to downstream/packaging issues, this is kind of a hard ship to turn for not a lot of benefit for users of nbconvert. "What comes out of mistune" is pretty much the definition of "Jupyter Markdown," many downstreams are very sensitive to what comes out of nbconvert as a CLI tool, so we at least owe a best-effort attempt to keep the output the same for those users.

If the new modularity/extensibility means things might change without a user expecting them, then that's going to be a pretty hard sell.

The Python API will pretty much be broken, of course, and there's not much we can do about that if (only) the 2.x line were to be supported.

I see a few different paths, each of which has some trade-offs:

  • do a 6.x.0 release that supports >=0.8,<3.0
    • maintain what of the public python API we can, e.g. basic API markdown2html_mistune utility function
    • this is nice-ish because we can keep both code paths "hot" and test them against each other (at least within one CI run)
    • this would not feel good in 6.x, but it would unblock some of these things
  • do a 7.0.0 release that moves the mistune implementation
    • move the dependency to an (initially required) nbconvert-mistune, which launches with two versions to support the two APIs
      • in the future, make this not required, leaving space in the future for alternate markdown implementations e.g. markdown-it-py
  • do a 7.0.0 release that only supports 2.0
    • but this might happen again with 3.x, forcing another version?

I'd be very tempted to try the first approach, at least initially, as it's the most testable, and wouldn't actually be that much more work to try (though keeping it healthy in the long term might be hard). But again, as I don't really see the end user value yet, it's hard to prioritize this over other work given limited maintainer time.

@juliangilbey
Copy link

I've been thinking about this too. There's a not-particularly-great solution which might solve this issue in the short term and requires minimal maintainer time. Since mistune 0.8.6 is a single file, you could include a copy of that version of mistune.py in nbconvert, say as mistune1.py (to avoid confusion), then instead of import mistune, use import .mistune1 as mistune. That way, any longer-term changes can be left for now.

@bollwyvl
Copy link
Contributor

bollwyvl commented Dec 29, 2021 via email

@michaelaye
Copy link

I wonder how nbdev will cope with all this. They already only allow nbconvert=5 and jupyter_client=6 and they don't have the resources to keep things up to date either. I'm quite scared about the fragility of the whole system.

@juliangilbey
Copy link

Yeah vendoring is nobody's favorite, but in this case the single file and compatible license might be reasonable... But, much like with the compatibility layer, any downstream that was using the mistune-related API, their stuff would probably not work anymore, and would be just as broken (but probably more mysteriously). And if there were user benefits to the new mistune, it would be harder to realize them as packages would continue to use the vendor crutch.

I do see the problem, yes. However, I'm not sure that this would be the case: if the nbconvert object is still subclassed from what is effectively mistune v1 (0.8.6), it should still behave identically.

@mcepl
Copy link
Contributor

mcepl commented Dec 30, 2021

Just adding as the openSUSE Python maintainer my €0.02 here … we are in the same bad situation: we have upgraded mistune so we broke number of apps (some of them have at least unofficial patches), the alternative being not upgrading mistune and breaking the other number of apps. I am afraid the way forward is the only way (or temporary vendoring, as much as I hate it).

@username30
Copy link

username30 commented Feb 21, 2022

Hello,
Thank you for your great work!

I'd like to ask to take up with the latest mistune version as well, since with it's being properly updated to v2.0.2, Jupyter yields the following issue (i.e. missing ability to export Notebooks) and the only workaround for it being this, that is reinstalling nbconvert with

pip uninstall nbconvert
pip install nbconvert

which brings mistune of 0.8.4 back into system. It's very inconvenient, since it's only working until next global update of pip packages, and forces to remember all the packages that should be left w/o updating them.

If anything, in my case nbconvert stumbled at the following place in the new mistune:

AttributeError: module 'mistune' has no attribute 'BlockGrammar'

@pietrodn
Copy link

pietrodn commented Apr 6, 2022

mistune < 2.0.1 (including the latest release supported by nbconvert, which is 0.8.4) has a XSS vulnerability. Therefore, it would be wise to update.

@mr-c
Copy link

mr-c commented Apr 11, 2022

mistune < 2.0.1 (including the latest release supported by nbconvert, which is 0.8.4) has a XSS vulnerability. Therefore, it would be wise to update.

I don't think that vulnerability is in mistune 0.8.x ; it appears to be unique to the 2.x re-write. I've summited a change request to Snyk to update the vulnerable versions accordingly.

From 0.8.4, the link is escaped:

https://github.com/lepture/mistune/blob/6470c42b2087a4649ad0437227cf7b5c570c3a91/mistune.py#L899

https://github.com/lepture/mistune/blob/6470c42b2087a4649ad0437227cf7b5c570c3a91/mistune.py#L660

as is version 0.8.3
https://github.com/lepture/mistune/blob/92b7f32664bad1a4b3740ee81eda47e5246e780f/mistune.py#L875

and version 0.8.2
https://github.com/lepture/mistune/blob/b8da37ec3d0e6f63273e37a7c47a0f00bd477db0/mistune.py#L875

@mr-c
Copy link

mr-c commented Apr 11, 2022

FYI, for Debian, the Debian Python team created a new package for mistune 0.8.x to give us more time to deal with the abrupt API change.

https://tracker.debian.org/pkg/mistune0

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1003567#70

@marmitar marmitar mentioned this issue Apr 23, 2022
5 tasks
@marmitar
Copy link
Contributor

marmitar commented Apr 23, 2022

I've proposed an update to mistune 2.0.2 (#1764), but I'm not too confident it should be merged, specially if the XSS vulnerability doesn't affect 0.8.4. The new lib is far less tested and documented, and even uses the completely undocumented re.Scanner (it exists at least since 2003 and may have some unexpected problems).

@SimonHeybrock
Copy link

I just got a Dependabot alert about a vulnerability in mistune<2.0.3. nbconvert seems to prevent an update to a safe version.

@blink1073
Copy link
Contributor

Hi @SimonHeybrock, are you referring to https://www.cvedetails.com/cve/CVE-2022-34749/? If so it is related to regex backtracking and is not a serious security risk. If it is something more serious please report security vulnerabilities to [email protected] per our security guide. We are in the midst of an RC release cycle that will update to the latest mistune. I've opened #1818 to bump the minimum version. In the mean time, you can run pip install --upgrade --pre mistune nbconvert to get the latest versions.

@jarrodmillman
Copy link

https://pypi.org/project/mistune/3.0.0/ is out now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests