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

Adding support for <link> css inclusion #721

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
107 changes: 107 additions & 0 deletions Source/Document Structure/SvgLink.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Net;
using System.Text;

namespace Svg
{
[NonSvgElementAttribute("link")]
public class SvgLink : NonSvgElement
{
public SvgLink()
:base("link")
{
FabioFerrettiMoodys marked this conversation as resolved.
Show resolved Hide resolved

FabioFerrettiMoodys marked this conversation as resolved.
Show resolved Hide resolved
}
public enum RelativeValue
{
Unknown,
Alternate,
Author,
Help,
Icon,
License,
Next,
Pingback,
Preconnect,
Prefetch,
Preload,
Prerender,
Prev,
Search,
Stylesheet
}

public override SvgElement DeepCopy()
{
return DeepCopy<SvgTitle>();
FabioFerrettiMoodys marked this conversation as resolved.
Show resolved Hide resolved
}

FabioFerrettiMoodys marked this conversation as resolved.
Show resolved Hide resolved

[SvgAttribute("href")]
public string Href
{
get { return GetAttribute<string>("href", false,string.Empty); }
FabioFerrettiMoodys marked this conversation as resolved.
Show resolved Hide resolved
set { Attributes["href"] = value; }
}

[SvgAttribute("rel")]
public RelativeValue Rel
{
get { return GetAttribute<RelativeValue>("rel", false, RelativeValue.Unknown); }
set { Attributes["rel"] = value; }
}

FabioFerrettiMoodys marked this conversation as resolved.
Show resolved Hide resolved


public string GetLinkContentAsText(RelativeValue rel)
{
var stream = GetLinkContentAsStream(rel);
FabioFerrettiMoodys marked this conversation as resolved.
Show resolved Hide resolved
if (stream == null)
return null;

string content = null;
using (StreamReader sr = new StreamReader(stream))
FabioFerrettiMoodys marked this conversation as resolved.
Show resolved Hide resolved
{
content = sr.ReadToEnd();
}
stream.Dispose();
return content;
}


public Stream GetLinkContentAsStream(RelativeValue rel= RelativeValue.Unknown)
{
if (Rel != rel && rel != RelativeValue.Unknown)
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're doing a comparison with Rel property, why not use the value of Rel property value ??

Choose a reason for hiding this comment

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

it is to avoid as much as possible the use of the content with a incorrect rel type.

if you call GetLinkContentAsStream(stylesheet) on a link with rel="icon" for exemple, you will not get the content and cannot use it as a stylesheet by mistake (and make the code crash).

assuming you are absolutely sure of what you do and what you have, you can just call GetLinkContentAsStream(), you will get the content independently of the rel

these technique avoid to check everywhere if the Rel is correct every where you use the link content, here it is done in only one place.

I can remove if if you prefer.

return null;
// Uri MaxLength is 65519 (https://msdn.microsoft.com/en-us/library/z6c2z492.aspx)
// if using data URI scheme, very long URI may happen.
var safeUriString = Href.Length > 65519 ? Href.Substring(0, 65519) : Href;

try
{
var uri = new Uri(safeUriString, UriKind.RelativeOrAbsolute);

if (!uri.IsAbsoluteUri)
uri = new Uri(OwnerDocument.BaseUri, uri);

// should work with http: and file: protocol urls
var httpRequest = WebRequest.Create(uri);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should check that SvgDocument.ResolveExternalResources is true before creating a web request to avoid XSS vulnerabilities. See #873.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kimsey0 Can you fix it by editing the PR source?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's been too long since I worked on this project, so I can't really remember the details, nor how tests etc. are structured. I can see that #873 split SvgDocument.ResolveExternalResources into three properties: ResolveExternalXmlEntites, ResolveExternalImages, and ResolveExternalElements, probably to give more fine-grained control over which external resources are allowed to be resolved. You may want to introduce a new ExternalType ResolveExternalStylesheets property, somehow pass it in or make it available to this GetLinkContentAsStream method, and then trace a warning and return null or an empty string if !ResolveExternalStylesheets.AllowsResolving(uri).

Copy link
Contributor

@paulushub paulushub Jan 19, 2024

Choose a reason for hiding this comment

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

@kimsey0 Thanks for the update. Just do not know the best way to handle these staled PR.

  • Take over (clone PR reposition and fix issues), the original owner looses his/her contribution
  • Close the PR and see how to reimplement the issue, a waste but may be worth it

Copy link
Member

Choose a reason for hiding this comment

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

  • adapt/finish the original PR, so it will just have 2 authors

Copy link
Contributor

Choose a reason for hiding this comment

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

  • adapt/finish the original PR, so it will just have 2 authors

Never done, can you outline the procedure and give me some hints?

Copy link
Member

@mrbean-bremen mrbean-bremen Jan 19, 2024

Choose a reason for hiding this comment

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

If you are asking about the git workflow, something like:

git remote add FabioFerrettiMoodys https://github.com/FabioFerrettiMoodys/SVG
git checkout FabioFerrettiMoodys/master (probably under some other name)
git rebase origin/main -> resolve conflicts as usual
git push --force-with-lease
(or the equivalents if you use a git GUI client(

  • make your changes, commit, push...
  • when done and reviewed, squash the commit, edit the commit message, but leave the authors in the message

Is this what you are asking?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this what you are asking?

I think so, thanks - will look into it this weekend.

Copy link
Contributor

Choose a reason for hiding this comment

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

Depending on the policies of this project (the contribution guide doesn't say), you can also just avoid squashing the commits, leaving the original author for each one and making it clear who's done what.

Copy link
Member

Choose a reason for hiding this comment

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

the contribution guide doesn't say

Something to be added...

The current (undocumented) convention is that commits are usually squashed and only rebased if the individual commits are independent changes to be preserved, and have sensible commit comments (which is not the case here). Though it is possible to rebase the PR respectively before the merge, if a rebase is wanted.


var webResponse = httpRequest.GetResponse();
var stream = webResponse.GetResponseStream();
FabioFerrettiMoodys marked this conversation as resolved.
Show resolved Hide resolved
if (stream.CanSeek)
stream.Position = 0;
return stream;
}
catch (Exception ex)
{
Trace.TraceError("Error loading Link content: '{0}', error: {1} ", Href, ex.Message);
return null;
}

}
}
}
27 changes: 27 additions & 0 deletions Source/NonSvgElementAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using System;
using System.Collections.Generic;
using System.Text;

namespace Svg
{
/// <summary>
/// Specifies the SVG name of an <see cref="SvgElement"/>.
/// </summary>
[AttributeUsage(AttributeTargets.Class)]
public sealed class NonSvgElementAttribute : Attribute
{
/// <summary>
/// Gets the name of the SVG element.
/// </summary>
public string ElementName { get; private set; }

/// <summary>
/// Initializes a new instance of the <see cref="NonSvgElementAttribute"/> class with the specified element name;
/// </summary>
/// <param name="elementName">The name of the non SVG element.</param>
public NonSvgElementAttribute(string elementName)
{
this.ElementName = elementName;
}
}
}
49 changes: 35 additions & 14 deletions Source/SvgDocument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -260,32 +260,35 @@ private static bool ExceptionCaughtIsGdiPlusRelated(Exception e)
/// Opens the document at the specified path and loads the SVG contents.
/// </summary>
/// <param name="path">A <see cref="string"/> containing the path of the file to open.</param>
/// <param name="baseUri"> base uri for linked file</param>
/// <returns>An <see cref="SvgDocument"/> with the contents loaded.</returns>
/// <exception cref="FileNotFoundException">The document at the specified <paramref name="path"/> cannot be found.</exception>
public static SvgDocument Open(string path)
public static SvgDocument Open(string path, Uri baseUri = null)
FabioFerrettiMoodys marked this conversation as resolved.
Show resolved Hide resolved
{
return Open<SvgDocument>(path, null);
return Open<SvgDocument>(path, null, baseUri == null ? new Uri(System.IO.Path.GetFullPath(path)) : baseUri);
}

/// <summary>
/// Opens the document at the specified path and loads the SVG contents.
/// </summary>
/// <param name="path">A <see cref="string"/> containing the path of the file to open.</param>
/// <param name="baseUri"> base uri for linked file</param>
/// <returns>An <see cref="SvgDocument"/> with the contents loaded.</returns>
/// <exception cref="FileNotFoundException">The document at the specified <paramref name="path"/> cannot be found.</exception>
public static T Open<T>(string path) where T : SvgDocument, new()
public static T Open<T>(string path, Uri baseUri = null) where T : SvgDocument, new()
{
return Open<T>(path, null);
return Open<T>(path, null, baseUri);
}

/// <summary>
/// Opens the document at the specified path and loads the SVG contents.
/// </summary>
/// <param name="path">A <see cref="string"/> containing the path of the file to open.</param>
/// <param name="baseUri"> base uri for linked file</param>
/// <param name="entities">A dictionary of custom entity definitions to be used when resolving XML entities within the document.</param>
/// <returns>An <see cref="SvgDocument"/> with the contents loaded.</returns>
/// <exception cref="FileNotFoundException">The document at the specified <paramref name="path"/> cannot be found.</exception>
public static T Open<T>(string path, Dictionary<string, string> entities) where T : SvgDocument, new()
public static T Open<T>(string path, Dictionary<string, string> entities, Uri baseUri = null) where T : SvgDocument, new()
{
if (string.IsNullOrEmpty(path))
{
Expand All @@ -299,8 +302,11 @@ public static SvgDocument Open(string path)

using (var stream = File.OpenRead(path))
{
var doc = Open<T>(stream, entities);
doc.BaseUri = new Uri(System.IO.Path.GetFullPath(path));
var doc = Open<T>(stream, entities, baseUri == null ? new Uri(System.IO.Path.GetFullPath(path)) : baseUri);
if (baseUri == null)
doc.BaseUri = new Uri(System.IO.Path.GetFullPath(path));
else
doc.BaseUri = baseUri;
return doc;
}
}
Expand All @@ -309,9 +315,10 @@ public static SvgDocument Open(string path)
/// Attempts to open an SVG document from the specified <see cref="Stream"/>.
/// </summary>
/// <param name="stream">The <see cref="Stream"/> containing the SVG document to open.</param>
public static T Open<T>(Stream stream) where T : SvgDocument, new()
/// <param name="baseUri"> base uri for linked file</param>
public static T Open<T>(Stream stream, Uri baseUri = null) where T : SvgDocument, new()
{
return Open<T>(stream, null);
return Open<T>(stream, null, baseUri );
}


Expand Down Expand Up @@ -342,8 +349,9 @@ public static SvgDocument Open(string path)
/// </summary>
/// <param name="stream">The <see cref="Stream"/> containing the SVG document to open.</param>
/// <param name="entities">Custom entity definitions.</param>
/// <param name="baseUri"> base uri for linked file</param>
/// <exception cref="ArgumentNullException">The <paramref name="stream"/> parameter cannot be <c>null</c>.</exception>
public static T Open<T>(Stream stream, Dictionary<string, string> entities) where T : SvgDocument, new()
public static T Open<T>(Stream stream, Dictionary<string, string> entities, Uri baseUri = null) where T : SvgDocument, new()
{
if (stream == null)
{
Expand All @@ -356,10 +364,10 @@ public static SvgDocument Open(string path)
XmlResolver = new SvgDtdResolver(),
WhitespaceHandling = WhitespaceHandling.None
};
return Open<T>(reader);
return Open<T>(reader, baseUri);
}

private static T Open<T>(XmlReader reader) where T : SvgDocument, new()
private static T Open<T>(XmlReader reader, Uri baseUri = null) where T : SvgDocument, new()
{
if (!SkipGdiPlusCapabilityCheck)
{
Expand Down Expand Up @@ -392,6 +400,8 @@ public static SvgDocument Open(string path)
else
{
svgDocument = elementFactory.CreateDocument<T>(reader);
if(baseUri!=null)
FabioFerrettiMoodys marked this conversation as resolved.
Show resolved Hide resolved
svgDocument.BaseUri = baseUri;
element = svgDocument;
}

Expand Down Expand Up @@ -435,6 +445,16 @@ public static SvgDocument Open(string path)
{
styles.Add(unknown);
}
else if (element is SvgLink)
{
var stylecontent = ((SvgLink)element).GetLinkContentAsText(SvgLink.RelativeValue.Stylesheet);
if(!string.IsNullOrEmpty(stylecontent))
{
var elem = new SvgUnknownElement("style");
elem.Content = stylecontent;
styles.Add(elem);
}
}
break;
case XmlNodeType.CDATA:
case XmlNodeType.Text:
Expand Down Expand Up @@ -491,16 +511,17 @@ public static SvgDocument Open(string path)
/// Opens an SVG document from the specified <see cref="XmlDocument"/>.
/// </summary>
/// <param name="document">The <see cref="XmlDocument"/> containing the SVG document XML.</param>
/// <param name="baseUri"> base uri for linked file</param>
/// <exception cref="ArgumentNullException">The <paramref name="document"/> parameter cannot be <c>null</c>.</exception>
public static SvgDocument Open(XmlDocument document)
public static SvgDocument Open(XmlDocument document, Uri baseUri = null)
{
if (document == null)
{
throw new ArgumentNullException("document");
}

var reader = new SvgNodeReader(document.DocumentElement, null);
return Open<SvgDocument>(reader);
return Open<SvgDocument>(reader, baseUri);
}

public static Bitmap OpenAsBitmap(string path)
Expand Down
49 changes: 44 additions & 5 deletions Source/SvgElementFactory.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Xml;
Expand All @@ -14,8 +14,9 @@ namespace Svg
/// </summary>
internal class SvgElementFactory
{
private List<ElementInfo> availableElements;
private Parser cssParser = new Parser();
private List<ElementInfo> availableElements;
FabioFerrettiMoodys marked this conversation as resolved.
Show resolved Hide resolved
private List<ElementInfo> availableNonSvgElements;
private Parser cssParser = new Parser();

/// <summary>
/// Gets a list of available types that can be used when creating an <see cref="SvgElement"/>.
Expand All @@ -38,6 +39,31 @@ where t.GetCustomAttributes(typeof(SvgElementAttribute), true).Length > 0
}
}



/// <summary>
/// Gets a list of available types that can be used when creating an <see cref="NonSvgElement"/>.
/// </summary>
public List<ElementInfo> AvailableNonSvgElements
{
get
{
if (availableNonSvgElements == null)
{
var nonSvgTypes = from t in typeof(SvgDocument).Assembly.GetExportedTypes()
where t.GetCustomAttributes(typeof(NonSvgElementAttribute), true).Length > 0
&& t.IsSubclassOf(typeof(NonSvgElement))
select new ElementInfo { ElementName = ((NonSvgElementAttribute)t.GetCustomAttributes(typeof(NonSvgElementAttribute), true)[0]).ElementName, ElementType = t };

availableNonSvgElements = nonSvgTypes.ToList();
}

return availableNonSvgElements;
}
}



/// <summary>
/// Creates an <see cref="SvgDocument"/> from the current node in the specified <see cref="XmlTextReader"/>.
/// </summary>
Expand Down Expand Up @@ -111,8 +137,21 @@ public SvgElement CreateElement(XmlReader reader, SvgDocument document)
else
{
// All non svg element (html, ...)
createdElement = new NonSvgElement(elementName);
SetAttributes(createdElement, reader, document);
ElementInfo validType;
if (AvailableNonSvgElements.ToDictionary(e => e.ElementName, e => e).TryGetValue(elementName, out validType))
{
createdElement = (NonSvgElement)Activator.CreateInstance(validType.ElementType);
}
else
{
createdElement = new NonSvgElement(elementName);
}

if (createdElement != null)
{
SetAttributes(createdElement, reader, document);
}

}

//Trace.TraceInformation("End CreateElement");
Expand Down