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

Update SixLabors dependencies #1264

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

Bykiev
Copy link
Collaborator

@Bykiev Bykiev commented Feb 9, 2024

Update SixLabors dependencies to address ImageSharp DDOS security issue

Update SixLabors dependencies to address ImageSharp DDOS security issue
@tonyqus tonyqus modified the milestones: NPOI 2.7.1, NPOI 2.7.0 Feb 9, 2024
@tonyqus
Copy link
Member

tonyqus commented Feb 9, 2024

I notice

  • The latest version of SixLabors.Fonts is 2.0.1
  • The latest version of SixLabors.ImageSharp is 3.1.2.

Do you have any consideration of using SixLabors.ImageSharp 2.1.6 and SixLabors.Font 1.0.1?

@Bykiev
Copy link
Collaborator Author

Bykiev commented Feb 10, 2024

In v2 the license has been changed, but I see it's safe to update in NPOI. Will update the PR if you don't see any issues.

@MagicAndre1981
Copy link

In v2 the license has been changed, but I see it's safe to update in NPOI.

SixLabors.ImageSharp 3.1.2 is .net 6+ only and this would cause end of .net framework and net standard 2.x support. DON't do this. In their code they also changed it to .net8 only.

2.1.6 got the security fix backported and is fine to use.

@Bykiev
Copy link
Collaborator Author

Bykiev commented Feb 15, 2024

I still think SkiaSharp is a better suited for NPOI for these reasons:

  1. Targets almost all framework versions
  2. Performance, Here is a my BenchmarkDotNet result:

BenchmarkDotNet v0.13.12, Windows 10 (10.0.19045.3930/22H2/2022Update)
Intel Core i3-6100U CPU 2.30GHz (Skylake), 1 CPU, 4 logical and 2 physical cores
.NET SDK 8.0.200
[Host] : .NET 6.0.27 (6.0.2724.6912), X64 RyuJIT AVX2
DefaultJob : .NET 6.0.27 (6.0.2724.6912), X64 RyuJIT AVX2

Method Text Mean Error StdDev Ratio RatioSD
SixLaborsFonts a 14,953.2 ns 238.64 ns 265.25 ns 40.01 1.01
SkiaSharp a 374.6 ns 5.32 ns 4.97 ns 1.00 0.00
SixLaborsFonts Hello world 63,096.5 ns 1,084.99 ns 961.82 ns 100.51 4.40
SkiaSharp Hello world 615.4 ns 11.10 ns 19.14 ns 1.00 0.00
SixLaborsFonts Lore(...)tus. [2477] 35,068,575.1 ns 692,512.26 ns 647,776.40 ns 521.89 11.27
SkiaSharp Lore(...)tus. [2477] 67,108.0 ns 464.29 ns 362.49 ns 1.00 0.00
SixLaborsFonts The q(...)y dog [43] 224,897.1 ns 3,872.95 ns 6,680.65 ns 137.07 5.57
SkiaSharp The q(...)y dog [43] 1,656.6 ns 8.78 ns 6.86 ns 1.00 0.00

Here is a benchmark code (from SixLabors.Font project):

// Copyright (c) Six Labors.
// Licensed under the Six Labors Split License.

using System.Globalization;
using BenchmarkDotNet.Attributes;
using SkiaSharp;

namespace SixLabors.Fonts.Benchmarks;

/// <summary>
/// <para>
/// This benchmark is not actually measuring the same operation as SkiSharp is
/// not doing any layout or shaping operations. However it is useful as a marker to measure
/// performance against.
/// </para>
/// <para>We should see if we can include the Skia HarfBuzz extensions to see how we compare.</para>
/// </summary>
public class MeasureTextBenchmark : IDisposable
{
    private readonly TextOptions textOptions;
    private readonly SKTypeface arialTypeface;
    private readonly SKFont font;
    private readonly SKPaint paint;

    private const string LoremIpsum = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. "
   + "Sed non risus. Suspendisse lectus tortor, dignissim sit amet, adipiscing nec, ultricies sed, dolor. "
   + "Cras elementum ultrices diam. Maecenas ligula massa, varius a, semper congue, euismod non, mi. "
   + "Proin porttitor, orci nec nonummy molestie, enim est eleifend mi, non fermentum diam nisl sit amet erat. "
   + "Duis semper. Duis arcu massa, scelerisque vitae, consequat in, pretium a, enim. Pellentesque congue. "
   + "Ut in risus volutpat libero pharetra tempor. Cras vestibulum bibendum augue. Praesent egestas leo in pede. "
   + "Praesent blandit odio eu enim. Pellentesque sed dui ut augue blandit sodales. "
   + "Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae; "
   + "Aliquam nibh. Mauris ac mauris sed pede pellentesque fermentum. Maecenas adipiscing ante non diam sodales hendrerit. "
   + "\n\n"
   + "Ut velit mauris, egestas sed, gravida nec, ornare ut, mi. Aenean ut orci vel massa suscipit pulvinar. "
   + "Nulla sollicitudin. Fusce varius, ligula non tempus aliquam, nunc turpis ullamcorper nibh, in tempus sapien eros vitae ligula. "
   + "Pellentesque rhoncus nunc et augue. Integer id felis. Curabitur aliquet pellentesque diam. "
   + "Integer quis metus vitae elit lobortis egestas. Lorem ipsum dolor sit amet, consectetuer adipiscing elit. "
   + "Morbi vel erat non mauris convallis vehicula. Nulla et sapien. Integer tortor tellus, aliquam faucibus, "
   + "convallis id, congue eu, quam. Mauris ullamcorper felis vitae erat. Proin feugiat, augue non elementum posuere, "
   + "metus purus iaculis lectus, et tristique ligula justo vitae magna.\n\n"
   + "Aliquam convallis sollicitudin purus. Praesent aliquam, enim at fermentum mollis, ligula massa adipiscing nisl, "
   + "ac euismod nibh nisl eu lectus. Fusce vulputate eleifend sapien. Vestibulum purus quam, scelerisque ut, mollis sed, "
   + "nonummy id, metus. Nullam accumsan lorem in dui. Cras ultricies mi eu turpis hendrerit fringilla. "
   + "Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae; In ac dui quis mi consectetuer lacinia. "
   + "Nam pretium turpis et arcu. Duis arcu tortor, suscipit eget, imperdiet nec, imperdiet iaculis, ipsum. "
   + "Sed aliquam ultrices mauris. Integer ante arcu, accumsan a, consectetuer eget, posuere ut, mauris. "
   + "Praesent adipiscing. Phasellus ullamcorper ipsum rutrum nunc. Nunc nonummy metus. "
   + "Vestibulum volutpat pretium libero. Cras id dui. Aenean ut eros et nisl sagittis vestibulum. "
   + "Nullam nulla eros, ultricies sit amet, nonummy id, imperdiet feugiat, pede. Sed lectus.";

    public MeasureTextBenchmark()
    {
        const string fontFamilyName = "Arial";
        const int fontSize = 16;

        FontFamily fontFamily = SystemFonts.Get(fontFamilyName, CultureInfo.InvariantCulture);
        Font font = fontFamily.CreateFont(fontSize, FontStyle.Regular);

        this.textOptions = new TextOptions(font)
        {
            WrappingLength = 400
        };

        this.arialTypeface = SKTypeface.FromFamilyName(fontFamilyName, SKFontStyle.Normal);
        this.font = new SKFont(this.arialTypeface, fontSize);
        this.paint = new SKPaint(this.font);
    }

    public void Dispose()
    {
        this.arialTypeface.Dispose();
        this.font.Dispose();
        this.paint.Dispose();
    }

    [Params("a", "Hello world", "The quick brown fox jumps over the lazy dog", LoremIpsum)]
    public string Text { get; set; } = string.Empty;

    [Benchmark]
    public void SixLaborsFonts() => TextMeasurer.MeasureSize(this.Text, this.textOptions);

    [Benchmark(Baseline = true)]
    public void SkiaSharp() => this.paint.MeasureText(this.Text);
}

@MagicAndre1981
Copy link

I still think SkiaSharp is a better suited for NPOI for these reasons:

  1. Targets almost all framework versions
  2. Performance, Here is a my BenchmarkDotNet result:

create a PR for the NPOI 2.7.1. Version 2.7.0 is scheduled for End February, so having such a breaking change needs more testing time.

@Bykiev
Copy link
Collaborator Author

Bykiev commented Feb 15, 2024

I still think SkiaSharp is a better suited for NPOI for these reasons:

  1. Targets almost all framework versions
  2. Performance, Here is a my BenchmarkDotNet result:

create a PR for the NPOI 2.7.1. Version 2.7.0 is scheduled for End February, so having such a breaking change needs more testing time.

Let's see what @tonyqus think about it. The cons for SixLabors is it 100% managed

@tonyqus
Copy link
Member

tonyqus commented Feb 15, 2024

I agree with @MagicAndre1981. It's safer to use SkiaSharp in the release 2.7.1. It may cause some breaking changes and even Autoresize may stop working.

@tonyqus tonyqus modified the milestones: NPOI 2.7.0, NPOI 2.7.1 Feb 15, 2024
@Bykiev
Copy link
Collaborator Author

Bykiev commented Feb 15, 2024

Thanks, anyway this PR should be merged to 2.7.0

@tonyqus tonyqus merged commit ccc46f0 into nissl-lab:master Feb 15, 2024
3 checks passed
@tonyqus
Copy link
Member

tonyqus commented Mar 6, 2024

New Security Update from SixLabors
GHSA-65x7-c272-7g7r

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

Successfully merging this pull request may close these issues.

3 participants