Outputting images in ASP.NET: The hard way



  • A while back I encountered this piece of code:

    byte[] objImgData = ConvertImageToByteArray(new Bitmap(fileName), ImageFormat.Jpeg);
    MemoryStream objMemoryStream = new MemoryStream();
    objMemoryStream.Write(objImgData, 0, objImgData.Length);
    Image objImage = Image.FromStream(objMemoryStream);
    HttpContext.Current.Response.AddHeader("Content-Disposition", "filename=" + fileName);
    HttpContext.Current.Response.ContentType = "image/Jpeg";
    objImage.Save(HttpContext.Current.Response.OutputStream, ImageFormat.Jpeg);
    ...
    
    private byte[] ConvertImageToByteArray(Image objImageToConvert, ImageFormat objFormatOfImage)
    {
    	byte[] objByteImage;
    
    	using (MemoryStream objMemoryStream = new MemoryStream())
    	{
    		objImageToConvert.Save(objMemoryStream, objFormatOfImage);
    		objByteImage = objMemoryStream.ToArray();
    	}
    
    	return objByteImage;
    }
    

    So essentially what it does is:

    1. Loads an image from a file, via Bitmap class
    2. Converts image to byte array:
      1. Writes image to a memory ​stream​
      2. Gets bytes from the memory ​stream​
    3. Writes byte array into an in-memory ​stream​ again (!!)
    4. Creates an image from the ​stream​ (note that Bitmap is a subclass of Image, so this is identical to the first step except it's loading from a stream instead of from the file)
    5. Writes bytes from image into the output ​stream​

    It's using two different instances of the same image and three different streams in memory. Surely there's a better way?

    It's already known beforehand that the image is a valid image file. I replaced it with a single line of code:

    Response.WriteFile(filename);

    Bonus bug: The Content-Disposition header in the old code had the full server-side path for the file, not just the file name)



  • Well code above will also convert image to jpeg.



  • Another bug

    HttpContext.Current.Response.ContentType = "image/Jpeg";

    Disregarding the type (Jpeg != jpeg)



  • Also, OutputStream isn't seekable, and Image.Save sometime requires a seekable stream, depending on the format.  The posted code would fail if the format was changed to PNG. Doing this with an image that is read from disk is stupid, but I often do something similar when I need to manipulate the image (like watermarking or resizing).  I usually save the image to a MemoryStream, then write the stream to response.OutputStream.



  • @ubersoldat said:

    Another bug

    HttpContext.Current.Response.ContentType = "image/Jpeg";

    Disregarding the type (Jpeg != jpeg)

    No it's not. MIME types are case-insensitive (altough you'd better stick to lowercase since not all implementations adhere)



  • This sounds like an attempt to avoid the dreaded "A generic error occurred in GDI+" exception. This error is as helpful as it sounds, and I personally have sunk days into trying to figure out how to fix it, and ended up giving up and saying "we can't do that".

    In a nut shell, when you load an Image, it depends on the stream it was created from. If you close that stream, error. If that stream is not seekable (in certain cases), error. Also, there are other stupid things, like if the image is read-only, error. Or, if some arbitrary temp folder is read only, error. All these errors throw the "generic error" mentioned above.



  • Any idea what the original intention was? Possibly trying to remove or add a thumbnail or some other metadata? I can understand not knowing about WriteFile and re-implementing that badly, but the conversion from JPEG to JPEG is surely not explained by not knowing that you can read a file to a byte array.



  •  @pkmnfrk said:

    This sounds like an attempt to avoid the dreaded "A generic error occurred in GDI+" exception. This error is as helpful as it sounds, and I personally have sunk days into trying to figure out how to fix it, and ended up giving up and saying "we can't do that".

    In a nut shell, when you load an Image, it depends on the stream it was created from. If you close that stream, error. If that stream is not seekable (in certain cases), error. Also, there are other stupid things, like if the image is read-only, error. Or, if some arbitrary temp folder is read only, error. All these errors throw the "generic error" mentioned above.

    I once got a cannot call virtual function error, but only on one specific OEM Model PC, traced it to bad chipset drivers, intel had no update and didn't produce that chip anymore. Installing a $20 pci-e workaround fixed the problem.

    TRWTF: the owner hoarded OEM XP machines to sell to customers who refused to upgrade.

     



  • @Jaime said:

    The posted code would fail if the format was changed to PNG.

    As far as I can tell, the images on disk are always JPEG files.

    Any idea what the original intention was? Possibly trying to remove or add a thumbnail or some other metadata? I can understand not knowing about WriteFile and re-implementing that badly, but the conversion from JPEG to JPEG is surely not explained by not knowing that you can read a file to a byte array.
    The code is in a page called "GetImage.ashx" and as far as I can tell it's only designed to retrieve a JPEG image file off a network share (I think it's coming from a NetApp NAS box). The images are just small photos and I think all the processing is done when the images are uploaded. No clue what the original developer was thinking or if I'm missing anything :P


  • @Daniel15 said:

    The code is in a page called "GetImage.ashx" and as far as I can tell it's only designed to retrieve a JPEG image file off a network share

    Well that makes a bit of sense then.
    If the site uses windows auth and/or the share is locked down to certian users then the current user may not be able to access the images, Doing it this way will use the Application pool identity to access the file and pass it back to the client - still more code than is needed, but it does make a bit of sense.



  • @wonkoTheSane said:

    If the site uses windows auth and/or the share is locked down to certian users then the current user may not be able to access the images, Doing it this way will use the Application pool identity to access the file and pass it back to the client - still more code than is needed, but it does make a bit of sense.

    The site doesn't use Windows auth, but the share is not accessible to users. So it does make sense to have a page to proxy the images, but it could just use Response.WriteFile or Response.TransmitFile instead.


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.