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

Markdig version update #8292

Merged
merged 22 commits into from
Dec 11, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public class FeatureFlagService : IFeatureFlagService
private const string PackageRenamesFeatureName = GalleryPrefix + "PackageRenames";
private const string EmbeddedReadmeFlightName = GalleryPrefix + "EmbeddedReadmes";
private const string LicenseMdRenderingFlightName = GalleryPrefix + "LicenseMdRendering";
private const string MarkdigMdRenderingFlightName = GalleryPrefix + "MarkdigMdRendering";
private const string DeletePackageApiFlightName = GalleryPrefix + "DeletePackageApi";

private const string ODataV1GetAllNonHijackedFeatureName = GalleryPrefix + "ODataV1GetAllNonHijacked";
Expand Down Expand Up @@ -286,6 +287,11 @@ public bool IsODataV2SearchCountNonHijackedEnabled()
return _client.IsEnabled(ODataV2SearchCountNonHijackedFeatureName, defaultValue: true);
}

public bool IsMarkdigMdRenderingEnabled()
{
return _client.IsEnabled(MarkdigMdRenderingFlightName, defaultValue: false);
}

public bool IsDeletePackageApiEnabled(User user)
{
return _client.IsEnabled(DeletePackageApiFlightName, user, defaultValue: false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,10 @@ public interface IFeatureFlagService
bool IsODataV2SearchCountNonHijackedEnabled();

/// <summary>
/// Whether rending Markdown content to HTML using Markdig is enabaled
lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
bool IsMarkdigMdRenderingEnabled();

/// Whether or not the user can delete a package through the API.
/// </summary>
bool IsDeletePackageApiEnabled(User user);
Expand Down
132 changes: 129 additions & 3 deletions src/NuGetGallery/Services/MarkdownService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,58 @@

using System;
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
using System.Timers;
using System.Web;
using CommonMark;
using CommonMark.Syntax;
loic-sharma marked this conversation as resolved.
Show resolved Hide resolved
using Markdig;
using Markdig.Parsers;
using Markdig.Renderers;
using Markdig.Syntax;
using Markdig.Syntax.Inlines;

namespace NuGetGallery
{
public class MarkdownService : IMarkdownService
{
private static readonly TimeSpan RegexTimeout = TimeSpan.FromMinutes(1);
private static readonly Regex EncodedBlockQuotePattern = new Regex("^ {0,3}&gt;", RegexOptions.Multiline, RegexTimeout);
private static readonly Regex CommonMarkLinkPattern = new Regex("<a href=([\"\']).*?\\1", RegexOptions.None, RegexTimeout);
private static readonly Regex LinkPattern = new Regex("<a href=([\"\']).*?\\1", RegexOptions.None, RegexTimeout);

private readonly IFeatureFlagService _features;

public MarkdownService(IFeatureFlagService features)
{
_features = features ?? throw new ArgumentNullException(nameof(features));
}

public RenderedMarkdownResult GetHtmlFromMarkdown(string markdownString)
{
lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved
return GetHtmlFromMarkdown(markdownString, 1);
if (_features.IsMarkdigMdRenderingEnabled())
{
return GetHtmlFromMarkdownMarkdig(markdownString, 1);
}
else
{
return GetHtmlFromMarkdownCommonMark(markdownString, 1);
}
}

public RenderedMarkdownResult GetHtmlFromMarkdown(string markdownString, int incrementHeadersBy)
{
lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved
lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved
if (_features.IsMarkdigMdRenderingEnabled())
{
return GetHtmlFromMarkdownMarkdig(markdownString, incrementHeadersBy);
}
else
{
return GetHtmlFromMarkdownCommonMark(markdownString, incrementHeadersBy);
}
}

private RenderedMarkdownResult GetHtmlFromMarkdownCommonMark(string markdownString, int incrementHeadersBy)
{
var output = new RenderedMarkdownResult()
{
Expand Down Expand Up @@ -106,7 +139,100 @@ public RenderedMarkdownResult GetHtmlFromMarkdown(string markdownString, int inc
{
CommonMarkConverter.ProcessStage3(document, htmlWriter, settings);

output.Content = CommonMarkLinkPattern.Replace(htmlWriter.ToString(), "$0" + " rel=\"nofollow\"").Trim();
output.Content = LinkPattern.Replace(htmlWriter.ToString(), "$0" + " rel=\"nofollow\"").Trim();
return output;
}
}

private RenderedMarkdownResult GetHtmlFromMarkdownMarkdig(string markdownString, int incrementHeadersBy)
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any argument validation needed?

var output = new RenderedMarkdownResult()
{
ImagesRewritten = false,
Content = ""
};

var readmeWithoutBom = markdownString.TrimStart('\ufeff');

// HTML encode markdown, except for block quotes, to block inline html.
var encodedMarkdown = EncodedBlockQuotePattern.Replace(HttpUtility.HtmlEncode(readmeWithoutBom), "> ");
lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved

var pipeline = new MarkdownPipelineBuilder()
.UseGridTables()
.UsePipeTables()
.UseListExtras()
.UseTaskLists()
.UseSoftlineBreakAsHardlineBreak()
.UseEmojiAndSmiley()
.UseReferralLinks("nofollow")
.UseAutoLinks()
.Build();
lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved

using(StringWriter htmlWriter = new StringWriter())
lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved
{
var renderer = new HtmlRenderer(htmlWriter);
pipeline.Setup(renderer);

var document = Markdown.Parse(encodedMarkdown, pipeline);

foreach (var node in document.Descendants())
{
if (node is Markdig.Syntax.Block)
{
// Demote heading tags so they don't overpower expander headings.
if (node is HeadingBlock heading)
{
heading.Level = (byte)Math.Min(heading.Level + incrementHeadersBy, 6);
}
lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved

lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved
// Decode preformatted blocks to prevent double encoding.
// Skip BlockTag.BlockQuote, which are partially decoded upfront.
if (node is FencedCodeBlock || node is CodeBlock)
{
LeafBlock codeBlock = (LeafBlock)node;
var lines =codeBlock.Lines;
lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved
for (int i = 0; i < lines.Count; i++)
{
var content = lines.Lines[i].Slice.ToString();
var unencodedContent = HttpUtility.HtmlDecode(content);
lines.Lines[i].Slice = new Markdig.Helpers.StringSlice(unencodedContent);
}
}
}
else if (node is Markdig.Syntax.Inlines.Inline)
{
if (node is LinkInline linkInline)
{
if (linkInline.IsImage)
{
// Allow only http or https links in markdown. Transform link to https for known domains.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this comment be used for links? I see that we always apply HTTPS for images, even though I am not sure why we design it in this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, It's also used to link. I could make same comments for links if you think it's not clear

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment only applies to links (see "CommonMark" one). For images, we don't "allow http or https", and we don't transform links to https only for known domains. We always force images to use "https", is it ("rewriteAllHttp: true")?

if (!PackageHelper.TryPrepareUrlForRendering(linkInline.Url, out string readyUriString, rewriteAllHttp: true))
{
linkInline.Url = string.Empty;
}
else
{
output.ImagesRewritten = output.ImagesRewritten || (linkInline.Url != readyUriString);
linkInline.Url = readyUriString;
}
}
else
{
if (!PackageHelper.TryPrepareUrlForRendering(linkInline.Url, out string readyUriString))
{
linkInline.Url = string.Empty;
}
else
{
linkInline.Url = readyUriString;
}
}
}
}
}

renderer.Render(document);
output.Content = htmlWriter.ToString().Trim();
return output;
}
}
Expand Down
Loading