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

Decoding JPEG puts the process to infinite loop if there is an error on JPEG bytes #214

Closed
xeora opened this Issue May 13, 2017 · 14 comments

Comments

Projects
None yet
6 participants
@xeora
Copy link

xeora commented May 13, 2017

The problem appears when you want to resize a JPEG with crop options.

Problematic code piece is the following;
https://github.com/JimBobSquarePants/ImageSharp/blob/master/src/ImageSharp/Formats/Jpeg/Components/Decoder/InputProcessor.cs#L156

I made a hotfix for myself by changing the code like

while (length > 0 && errorCode == DecoderErrorCode.NoError)

But surely, I didn't test anything else if it breaks something more. Currently working okay in my environment.

It requires deep investigation if it is the correct fix or not...

thanks...

@JimBobSquarePants

This comment has been minimized.

Copy link
Member

JimBobSquarePants commented May 14, 2017

Hi @xeora

Could you please resubmit this with version numbers etc and the offending image? We've nothing to go on since you deleted the template for some reason.

Cheers

James

@tocsoft

This comment has been minimized.

Copy link
Member

tocsoft commented May 14, 2017

Also could you provide a code example when you provide version details as crop/resize functions have no impact the decoder they are entirely separate systems.

@xeora

This comment has been minimized.

Copy link

xeora commented May 14, 2017

Prerequisites

  • I have written a descriptive issue title
  • I have verified that I am running the latest version of ImageSharp
  • I have verified if the problem exist in both DEBUG and RELEASE mode
  • I have searched open and closed issues to ensure it has not already been reported

Description

When you are loading a jpg file with some broken bytes in it, process gets in to infinite loop.

Steps to Reproduce

Try to load the jpg with broken bytes or falsy EOF.
ex: ImageSharp.Image.Load(InputImage)

System Configuration

Windows 10 IIS server but it is happening also linux mono or windows 2012 srd srv with .net 4.5.2 installed. But it is not .net or OS related problem..

  • ImageSharp version: Current Git hub version.
  • Other ImageSharp packages and versions: -
  • Environment (Operating system, version and so on): Windows 10 IIS
  • .NET Framework version: .net 4.5.2
  • Additional information: Sorry for my first entry, I already customize your code a lot to work faster. That's why, I just jump in to the point where problem is exists. However, I didn't modify the related part and when i test it with the current version on github, problem is still there. I said while croping / resizing because my code is joined the load and the resize together. I thought it was an encoding problem but when i check deeper, I realised that it is decoding problem actually.

Thanks...

@JimBobSquarePants

This comment has been minimized.

Copy link
Member

JimBobSquarePants commented May 14, 2017

I already customize your code a lot to work faster

Why would you do this; It's an open source project? If you find issues or have performance related concerns why not create PR's and give back?

We still require a test image to triage against.

@xeora

This comment has been minimized.

Copy link

xeora commented May 14, 2017

It is just aimed purposes modifications. If you apply what I did to your code, you will break the usability. I'm chopping, removing, skipping and it is making fast but it is not okay for everyone this kind of customization. Otherwise, sure I'll share or advice my code like how I report an Issue ;) That's why, keep calm dude, I'm also open source supporter and developer...

I'll provide a test image soon.

@JimBobSquarePants

This comment has been minimized.

Copy link
Member

JimBobSquarePants commented May 14, 2017

@xeora

I assure you I am perfectly calm. There are glaring issue here however.

  • You initially raised the ticket with no version number, stacktrace, code sample, nor image.
  • You raised the issue based on a heavily modified codebase that we have no ability, obligation, nor intent to support.

Now you have, to your credit, provide a potential code fix but we have no way to "deeply investigate" whether that fix is appropriate since you have not provided us with a broken image to test against.

We also do not know what the expected outcome should be. Does System.Drawing successfully open this image? Does MSPaint, Adobe, Libjpeg?

If you can provide us with that image we can double check that fix and move forward. Without that we cannot.

@xeora

This comment has been minimized.

Copy link

xeora commented May 14, 2017

I'll provide you a test image very soon.

  • System.Drawing is not working. It fails.
  • MSPaint fails.
  • Adobe Photoshop Opens but wrong.
  • Libjpeg, I have not tried.
  • ImageSharp gets into an infinite loop.

heavily modified codebase

It is behaving the same on an untouched code.

Just wait for the jpg from me. 👍

@xeora

This comment has been minimized.

Copy link

xeora commented May 15, 2017

a532bbff-f588-4a04-904b-93f09ff4f6af

I attached the file which creates the problem... @JimBobSquarePants @tocsoft

Also attaching the fixed one to see the byte differences between files.

a532bbff-f588-4a04-904b-93f09ff4f6af 2

In addition to this, this problem has occurred 8 or 9 times in around 2 million thumbnail images. It may also help you to decide if it is a major problem or not...

One last thing; I'm opening a file stream of the image file and try to load it from that stream.

@KLuuKer

This comment has been minimized.

Copy link

KLuuKer commented May 15, 2017

Some programs that fail to open it

  • MSPaint
  • paint.net 4.0.16
  • windows 10 photos app
  • irfanview 4.42 (jpeg datastream contains no image)
  • adobe photoshop CC 2017 (with latest updates)
@antonfirsov

This comment has been minimized.

Copy link
Member

antonfirsov commented May 15, 2017

Also none of my web browsers can decode it (Chrome, Edge).

I'd say, being able to decode this is a nice to have feature, but from our point of view it has a low priority.

@xeora if you could help us by trying your code changes on our existing tests, and make a PR if it doesn't break them, that would be nice :)

@xeora

This comment has been minimized.

Copy link

xeora commented May 15, 2017

Honestly, I'm not expecting to open the image or process it. If the application can throw an exception other than getting into an infinite loop, will be the correct behavior because It is clearly a bug. The explanation of the change that I did in the code is;

when the result return on this line
errorCode = this.Bytes.FillUnsafe(this.InputStream);

errorCode becomes UnexpectedEndOfStream. While there is an UnexpectedEndOfStream error on decoding, I don't see any logic about trying to continue to decode process. But somehow, it continues and that creates an infinite loop.

Do not consider that, I want to open it or trying to process something on this image. The image is already trash for me, I just need to continue to my process with the following ones but that infinite loop is locking my operation...

@antonfirsov

This comment has been minimized.

Copy link
Member

antonfirsov commented May 15, 2017

Cool, throwing an exception shouldn't be that hard. Running into an infinite loop is definitely a bug.

@dlemstra

This comment has been minimized.

Copy link
Member

dlemstra commented May 15, 2017

I do think we need to check if this.Bytes.J - this.Bytes.I is smaller than the length in the else block: https://github.com/JimBobSquarePants/ImageSharp/blob/master/src/ImageSharp/Formats/Jpeg/Components/Decoder/InputProcessor.cs#L166 and return DecoderErrorCode.UnexpectedEndOfStream if it is.

@xeora

This comment has been minimized.

Copy link

xeora commented May 15, 2017

Guys, I can not know your codes better than you. I just did a hotfix for myself and saved the day. I am just obligated to report the issue. You can discuss between you and find the best way to fix it. I already like what you are doing and appreciate that you make it open source. Thanks for your effort.

Actually, there is another problem exists about handling big size images of which is memory exception but I saw those tickets already created. That's why I'm not mentioning that... When I have time to focus ImageSharp, I'll also try to help to take care of it but I'm currently running behind 7 big projects. No time sadly...

@JimBobSquarePants JimBobSquarePants changed the title Resizing JPEG puts the process to infinite loop if there is an error on JPEG bytes Decoding JPEG puts the process to infinite loop if there is an error on JPEG bytes May 16, 2017

@JimBobSquarePants JimBobSquarePants referenced this issue Jul 10, 2017

Merged

WIP Replace Jpeg Decoder #274

4 of 4 tasks complete

@tocsoft tocsoft closed this in f11fc03 Sep 14, 2017

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