Entire Process Terminates when Time Limit Exceeded

[tim31415]

Prerequisites

• [x] I have written a descriptive issue title
• [X] I have verified that I am using the latest version of Magick.NET
• [X] I have searched open and closed issues to ensure it has not already been reported

Description

When configuring a time limit resource policy, Magick.NET appears to honor that policy as an aggregate of all operations, not for individual operations. After that time limit is exceeded (in aggregate of all operations), the entire process terminates unexpectedly.

My expectation is that if a time limit is exceeded a managed exception would be thrown, rather than a hard process termination-- similar to how Magick.NET handles exceeding disk limit policy.

Further, my expectation is that in order to be useful as an in-process library, the "time" policy limit would be enforced per operation, not aggregated over the lifetime of the entire process-- though this might be considered more of a feature request.

I am able to work around this defect simply by not configuring a time policy in my application. However, such a policy as this is very useful for mitigating potential denial-of-service attacks by malformed images; and I would love to see it available in Magick.NET.

Steps to Reproduce

using this policy:

<policymap>    
  <policy domain="resource" name="memory" value="1GiB"/>
  <policy domain="resource" name="map" value="2GiB"/>
  <policy domain="resource" name="disk" value="5GiB"/>
  <policy domain="resource" name="width" value="8KP"/>
  <policy domain="resource" name="height" value="8KP"/>
  <policy domain="resource" name="area" value="16MP"/>  
  <policy domain="resource" name="file" value="768"/>
  <policy domain="resource" name="thread" value="2"/>
  <policy domain="resource" name="throttle" value="0"/>
  <policy domain="resource" name="time" value="10"/>
  <policy domain="resource" name="list-length" value="64"/>
  <policy domain="system" name="precision" value="6"/>
  <policy domain="coder" rights="none" pattern="*" />  
  <policy domain="coder" rights="read|write" pattern="{JPEG,PNG,GIF,WEBP}" />
  <policy domain="filter" rights="none" pattern="*" />
  <policy domain="delegate" rights="none" pattern="*" />
  <policy domain="path" rights="none" pattern="@*"/>  
</policymap>

And this code:

class Program {
        static async Task Main(string[] args) {
            try {
                MagickNET.Initialize(Path.Combine(Environment.CurrentDirectory, "Config"));
                byte[] sampleData;
                using (var httpClient = new HttpClient()) {
                    sampleData = await httpClient.GetByteArrayAsync(@"https://imagemagick.org/image/wizard.jpg");
                }
                var stopWatch = new System.Diagnostics.Stopwatch();
                stopWatch.Start();
                for (int i = 0; i < 10000; ++i) {
                    Console.WriteLine($"Attempt #{i} @{stopWatch.ElapsedMilliseconds}ms");
                    using (var test = new MagickImage(sampleData)) {
                        using (var fakeOutput = new MemoryStream()) {
                            test.Write(fakeOutput, MagickFormat.Jpeg);
                        }
                    }
                }

                stopWatch.Stop();
            } catch (Exception e) {
                Console.Write(e.Message);
                Console.Write(e.StackTrace);
            }
        }
    }

After running for ~10 seconds the process will crash. The exception handler will not catch any exceptions. The following message is displayed on stderr:

time limit exceeded `Unknown error' @ fatal/cache.c/GetImagePixelCache/1696.

System Configuration

Magick.NET-QA-AnyCPU v7.13.1 Windows 10, x64 Targeting .NET 4.7.1, Any Cpu (prefer 32-bit is off), using C# 7.1

[dlemstra]

Normally ImageMagick is run on the command line as an executable and the time limit can be used to make the program exit when it runs to long. I will need to take a look at how this is done but I wonder if this feature could be changed into something that would work for a library. And adding a call to reset the time limit in each method sounds like a lot of overhead. I am now wondering if this option should be removed from this library.

[urban-warrior]

We moved the timeout out of MagickCore as you recommend. However, the time is still a global accumulation rather than per operation.

[frederikbosch]

As I reported in the commit by @urban-warrior, this commit causes problems in my PHP Imagick scripts. Reading a simple image of just a few bytes causes my scripts to throw exceptions after the container has run for quite a while. I guess this starts happening after the number of seconds specified in the policy.xml, in my case 7200 seconds.

[dlemstra]

The only change is that the type of the exception was changed, this did not change the behavior so you might need to search somewhere else.