• (cs)

    If this is .NET, you forgot to mention the fact that the Bitmap and Image classes aren't actually supported in ASP.NET, also, for added fun and games

  • Herr Otto Flick (unregistered)

    The original code also verifies that the image is actually an image before spaffing it over the intertubes to clients to whom we say it is an image.

    Admittedly, it could do that without many of the in memory copies - which, although inefficient, are not that inefficient. I've seen a lot worse.

  • Black Bart (unregistered)

    This jumble looks like it started from a misguided attempt to reuse code in ConvertImageToByteArray(). While not needed here, the procedure is necessary in .NET to use an image from a file without the un-intuitive file locking of disk-backed image memory.

  • (cs)

    This could so easily cause the problem which inspired the following ditty (to the tune of Love Story) ...

    Where do I log in The URL has 404'ed on me again I've recompiled it using FORTRAN in CygWin I've even tried to stream the exe as a bin

  • (cs)

    Ahhh.. but arrays are reference objects....so perhaps not quite as many copies as you think....

    (Still a WTF)

  • Craig (unregistered)

    Minor point: It would need to be three lines. The 'replacement' code still doesn't set the content-disposition header.

    There is sort of a decent point made about the code verifying that the file to serve really is an image (perhaps this is only a side-effect). BUT - given that it's hosted on the webserver, you'd hope that any such sanitisation was already performed at the time the file was created.

  • (cs) 
    // close the file stream
objMemoryStream.Close();

    Yep.

  • Pista (unregistered)

    How comes nobody realized that this code is not a WTF, it's enterprisey!!!

  • Bleeding Heart (unregistered) in reply to Craig
    Craig:
    ...BUT - given that it's hosted on the webserver, you'd hope that any such sanitisation was already performed at the time the file was created.

    If you find yourself saying this, you've already introduced a flaw that can be corrupted later.

    captcha: dolor - of course it's a color, nobody could have changed it after I sanitized the captcha

  • foo (unregistered)

    Hey... I saw this exact same WTF before in production code, except in Perl.

  • (cs)

    The explanation is incorrect. Some of the copying isn't actually copying: it's image conversion. The data that is sent has been through two lossy conversions with whatever the default quality parameters are.

  • nobulate (unregistered)

    The code is analogous to a junkie repeatedly stabbing their arm to find a vein, but they have all collapsed due to memory - er - drug abuse.

  • nobulate (unregistered) in reply to pjt33
    pjt33:
    The explanation is incorrect. Some of the copying isn't actually copying: it's image conversion. The data that is sent has been through two lossy conversions with whatever the default quality parameters are.

    Well then the code must either be running on Bing Image search, or a porn site.

  • faoileag (unregistered) in reply to Pista
    Pista:
    How comes nobody realized that this code is not a WTF, it's enterprisey!!!
    Because it's somewhat lacking in the "let's use some XML as well" department.

    public void ResponseImageToBrowserFromXML(string xmlWithFileLocation) { ... } would be a quick fix, provided it uses:

    ResponseImageFilePathExtractor pe = new ResponseImageFilePathExtractor(xmlWithFileLocation); string fileName = pw.getFilePath();

    And of course the XML should come from a database.

  • faoileag (unregistered)
    the article said:
    Infogram bitmaps containing the insightful witticisms of Justin Bieber (admittedly, a small bitmap...

    Would make a nice insult: "The sum total of your knowledge could be steganographed into a favicon!!!".

  • Pista (unregistered) in reply to faoileag
    faoileag:
    Pista:
    How comes nobody realized that this code is not a WTF, it's enterprisey!!!
    Because it's somewhat lacking in the "let's use some XML as well" department.

    public void ResponseImageToBrowserFromXML(string xmlWithFileLocation) { ... } would be a quick fix, provided it uses:

    ResponseImageFilePathExtractor pe = new ResponseImageFilePathExtractor(xmlWithFileLocation); string fileName = pw.getFilePath();

    And of course the XML should come from a database.

    Kudos to you for pointing out where this little gem can be improved :D

  • Stereotyper (unregistered) in reply to Herr Otto Flick

    I've Seen Worse Guy

  • Scourge of programmers. (unregistered) in reply to foo
    foo:
    Hey... I saw this exact same WTF before in production code, except in Perl.

    And what part of PERL was dealing in image processing?

  • faoileag (unregistered) in reply to Scourge of programmers.
    Scourge of programmers.:
    foo:
    Hey... I saw this exact same WTF before in production code, except in Perl.

    And what part of PERL was dealing in image processing?

    Never heard of Image::Magick?

  • Tux "Tuxedo" Penguin (unregistered) in reply to DamienK
    DamienK:
    If this is .NET, you forgot to mention the fact that the Bitmap and Image classes aren't actually supported in ASP.NET, also, for added fun and games

    No this is part of IE. I am pretty sure about that. Just didn't know that Jack will send it to this site like he told he'll do.

    Captcha: Appellatio: You know, Jack - you can always appellatio to an upper management when you'll get fired for breaking NDA. That will do nothing though.

    The IE is crap and it will remain a crap until I live and work in this place. And even then my successor will keep it as crappy as possible. It's bad enough that it does follow standards now. It's harder and harder to make site that doesn't work in IE while working in sane browser. This isn't good. This isn't good.

  • sol (unregistered)

    The code is most likely old also it was probably copied from code that was using a PNG. I'm not sure if it still happens but if you attempted to write a PNG to the response directly in older version the code would crash, but if you wrote to a memory stream first the PNG would properly write out.

    The real WTF is the .net image handling.

    http://stackoverflow.com/questions/5629251/c-sharp-outputting-image-to-response-output-stream-giving-gdi-error

  • Scourge of programmers. (unregistered) in reply to faoileag
    faoileag:
    Scourge of programmers.:
    foo:
    Hey... I saw this exact same WTF before in production code, except in Perl.

    And what part of PERL was dealing in image processing?

    Never heard of Image::Magick?

    I hadn't. But despite my complete ignorance of the topic at hand, I felt the need to to insert some sort of smug comment to assuage my crushing lack of self-esteem. I'm an idiot, and I'll probably do it again in future.

  • (cs) in reply to Scourge of programmers.
    Scourge of programmers.:
    And what part of PERL was dealing in image processing?

    And there's the real WTF… spelling it as PERL like it's an acronym. Hint: wrong.

  • faoileag (unregistered) in reply to Scourge of programmers.
    Scourge of programmers.:
    I felt the need to to insert some sort of smug comment to assuage my crushing lack of self-esteem ... I'll probably do it again in future
    Looking at the level of self-esteem usually found in programming forums like this, if you want to catch up you'll need a very good data plan if you're posting via your smartphone...
  • PC Amok (unregistered)
    Bruce Johnson:
    HttpContext.Current.Response.ContentType = "image/jpeg"; HttpContext.Current.Response.WriteFile(filename);

    Sorry Bruce, but while a given browser might forgive you for serving it a bitmap and calling it a jpeg, I won't on your code review. A bitmap is not a jpeg. The nice thing about the Image class (and its descendants such as Bitmap) in .NET is that it will allow you to save any supported image type to any other supported image type.

    And whoever said that ASP.NET does not support the Bitmap and Image classes is full of pungent fertilizer.

  • mainframe web developer (unregistered) in reply to faoileag
    faoileag:
    Pista:
    How comes nobody realized that this code is not a WTF, it's enterprisey!!!
    Because it's somewhat lacking in the "let's use some XML as well" department.

    public void ResponseImageToBrowserFromXML(string xmlWithFileLocation) { ... } would be a quick fix, provided it uses:

    ResponseImageFilePathExtractor pe = new ResponseImageFilePathExtractor(xmlWithFileLocation); string fileName = pw.getFilePath();

    And of course the XML should come from a database.

    Newb. The binary bitmap should be 64-bit encoded and then GZIPPED before storage into a BLOB column.

  • anonymous (unregistered) in reply to Craig
    Craig:
    Minor point: It would need to be three lines. The 'replacement' code still doesn't set the content-disposition header.

    There is sort of a decent point made about the code verifying that the file to serve really is an image (perhaps this is only a side-effect). BUT - given that it's hosted on the webserver, you'd hope that any such sanitisation was already performed at the time the file was created.

    Not if the filename could be in any way, shape, or form supplied by the user. E.g. http://example.org/cgi-bin/image.pl?name=/images/logo.jpg might send me the contents of logo.jpg, but http://example.org/cgi-bin/image.pl?name=../../.htpasswd might send me something that you probably didn't want me to have.

  • (cs) in reply to faoileag
    faoileag:
    the article said:
    Infogram bitmaps containing the insightful witticisms of Justin Bieber (admittedly, a small bitmap...

    Would make a nice insult: "The sum total of your knowledge could be steganographed into a favicon!!!".

    Posted to Facebook; soon to be someone's (:-) ) Slashdot sig

  • (cs) in reply to DamienK
    DamienK:
    If this is .NET, you forgot to mention the fact that the Bitmap and Image classes aren't actually supported in ASP.NET, also, for added fun and games

    Yes they are, if you add a reference to System.Drawing

  • nmclean (unregistered) in reply to PC Amok
    PC Amok:
    A bitmap is not a jpeg.
    And a Bitmap is not a bitmap. The article never says we're dealing with image/bmp files.

    The code is only problematic if we can't guarantee filename will always point to a jpeg. More information needed.

  • anonymous (unregistered) in reply to nmclean
    nmclean:
    PC Amok:
    A bitmap is not a jpeg.
    And a Bitmap is not a bitmap. The article never says we're dealing with image/bmp files.

    The code is only problematic if we can't guarantee filename will always point to a jpeg. More information needed.

    As long as the filename points to an image, the Bitmap constructor will create a bitmap from it, which will then be encoded as a JPEG. It is doing image conversion, but not very efficiently.

  • garaden (unregistered) in reply to TheCPUWizard
    TheCPUWizard:
    Ahhh.. but arrays are reference objects....so perhaps not quite as many copies as you think...

    My first thought was "nope, arrays of primitive types actually store their contents". But that's in Java, and this is .NET, which I don't know...

    Someone please tell me .NET doesn't store this as references to some kind of "byte" object!

  • nmclean (unregistered) in reply to anonymous
    anonymous:
    nmclean:
    PC Amok:
    A bitmap is not a jpeg.
    And a Bitmap is not a bitmap. The article never says we're dealing with image/bmp files.

    The code is only problematic if we can't guarantee filename will always point to a jpeg. More information needed.

    As long as the filename points to an image, the Bitmap constructor will create a bitmap from it, which will then be encoded as a JPEG. It is doing image conversion, but not very efficiently.
    We were talking about the "Response.WriteFile(filename)" method, not the original.

    Also, the original code is not doing image conversion. The Bitmap constructor does not create any new data. If you save to the same format as the input, you will get an exact duplicate of the file, right down to the EXIF data. It doesn't re-encode anything unless the format actually changes.

  • Frank (unregistered)

    Gosh, I've been away too long. When did [image] stop working?

  • anonymous (unregistered) in reply to nmclean
    nmclean:
    the original code is not doing image conversion. The Bitmap constructor does not create any new data. If you save to the same format as the input, you will get an exact duplicate of the file, right down to the EXIF data. It doesn't re-encode anything unless the format actually changes.
    It does do image conversion, though, if the source image isn't a JPEG.
  • aliquam (unregistered) in reply to anonymous
    anonymous:
    As long as the filename points to an image, the Bitmap constructor will create a bitmap from it, which will then be encoded as a JPEG. It is doing image conversion, but not very efficiently.
    That has nothing to do with the post this is allegedly a reply to.
  • modo (unregistered) in reply to anonymous
    anonymous:
    Not if the filename could be in any way, shape, or form supplied by the user.
    If that's a possibility then it applies to the original code as well - you presumably don't want to let the user download any image from anywhere on the server's hard drive.
  • Frank (unregistered) in reply to anonymous
    anonymous:
    Craig:
    Minor point: It would need to be three lines. The 'replacement' code still doesn't set the content-disposition header.

    There is sort of a decent point made about the code verifying that the file to serve really is an image (perhaps this is only a side-effect). BUT - given that it's hosted on the webserver, you'd hope that any such sanitisation was already performed at the time the file was created.

    Not if the filename could be in any way, shape, or form supplied by the user. E.g. http://example.org/cgi-bin/image.pl?name=/images/logo.jpg might send me the contents of logo.jpg, but http://example.org/cgi-bin/image.pl?name=../../.htpasswd might send me something that you probably didn't want me to have.
    Well, yes, but even verifying that the file is an image doesn't handle security concerns correctly. If security is an issue (and it usually is) you need a white list of valid inputs you will accept from the user. Just assuming "the user can have any image they want" is not enough.

    Real world example (I spotted this while working for a Fortune 50 online brokerage):

    image.pl?name=/charts/MYUSERID/asset_allocation.png

    shows my current investment portfolio, while

    image.pl?name=/charts/YOURUSERID/asset_allocation.png

    reveals yours to anyone who asks nicely.

  • (cs) in reply to Frank
    Frank:
    Gosh, I've been away too long. When did [image] stop working?

    Uppercase tag/attribute, not closed, no alt attribute. Gross.

    Also, you're assuming that the image itself is in a directory that is served.

  • Frank (unregistered) in reply to chubertdev
    chubertdev:
    Frank:
    Gosh, I've been away too long. When did [image] stop working?

    Uppercase tag/attribute, not closed, no alt attribute. Gross.

    Also, you're assuming that the image itself is in a directory that is served.

    OK, I was trying to be terse in order to focus on the essential point, but for your benefit alone:

    One pixel for spacing because we're obsessive but ignorant control freaks.

    Better?

    So back to my original question. Did that stop working? Or do you overengineering weenies just feel the need to write new object oriented enterprisey code for things that already worked fine long ago?

  • (cs) in reply to Frank
    Frank:
    chubertdev:
    Frank:
    Gosh, I've been away too long. When did [image] stop working?

    Uppercase tag/attribute, not closed, no alt attribute. Gross.

    Also, you're assuming that the image itself is in a directory that is served.

    OK, I was trying to be terse in order to focus on the essential point, but for your benefit alone:

    One pixel for spacing because we're obsessive but ignorant control freaks.

    Better?

    So back to my original question. Did that stop working? Or do you overengineering weenies just feel the need to write new object oriented enterprisey code for things that already worked fine long ago?

    To answer your question, yes, that stopped working. http://thedailywtf.com/Comments/Seeing-Sextuple.aspx#432153 http://thedailywtf.com/Comments/Seeing-Sextuple.aspx#432152

    You're way off on just so much.

  • a real sysadmin (unregistered) in reply to Frank

    Not really... depending on (x)html version, case sensivity comes into play. Also you haven't (self) closed it.

  • suscipit (unregistered) in reply to a real sysadmin
    a real sysadmin:
    Not really... depending on (x)html version, case sensivity comes into play. Also you haven't (self) closed it.
    If it's valid in some HTML variants and not others, and it's not specified which variant it's supposed to be, then you should assume the variant in which it is valid - just as you assume this post to be English and not French, because it's (hopefully) valid English but not valid French. (And the variants in which upper-case "IMG" is valid are also the variants in which there's no need for it to be closed, self or otherwise.)
  • noland (unregistered) in reply to chubertdev
    chubertdev:
    Frank:
    Gosh, I've been away too long. When did [image] stop working?

    Uppercase tag/attribute, not closed, no alt attribute. Gross.

    Also, you're assuming that the image itself is in a directory that is served.

    In fact uppercase tags were encouraged and considered to be good style some time ago. Closing slashes were only introduced with XHTML and HTML5 (which isn't a final standard yet) — so the majority of DOCTYPE-declarations is against you. ;-)

  • (cs)

    HTML: forgiveness by default, but it's still sloppy.

  • commoveo (unregistered) in reply to chubertdev
    chubertdev:
    HTML: forgiveness by default, but it's still sloppy.
    Not closing an [image] element in HTML 4 isn't "sloppy", and accepting it isn't "forgiveness", it's simply the correct syntax. "[image]" in HTML 4 means the same as "[image]>", as the W3C validator will tell you, although few browsers ever implemented it, and even those that once did changed to the XHTML interpretation to cater to people who incorrectly tried to use it in HTML documents. And as HTML 4 is case-insensitive, [image] and [image] are equally valid, except for subjective stylistic preferences; one isn't the "sloppy" version of the other.
  • amet (unregistered) in reply to commoveo
    commoveo:
    ...
    I should however note, just for the record, that the point about the missing ALT attribute is entirely legitimate.
  • (cs) in reply to commoveo
    commoveo:
    chubertdev:
    HTML: forgiveness by default, but it's still sloppy.
    Not closing an [image] element in HTML 4 isn't "sloppy", and accepting it isn't "forgiveness", it's simply the correct syntax. "[image]" in HTML 4 means the same as "[image]>", as the W3C validator will tell you, although few browsers ever implemented it, and even those that once did changed to the XHTML interpretation to cater to people who incorrectly tried to use it in HTML documents. And as HTML 4 is case-insensitive, [image] and [image] are equally valid, except for subjective stylistic preferences; one isn't the "sloppy" version of the other.

    it's LOOSE

  • (cs)

    obligatory XKCD for HTML standards: https://xkcd.com/927/

  • (cs) in reply to slippyr4
    slippyr4:
    DamienK:
    If this is .NET, you forgot to mention the fact that the Bitmap and Image classes aren't actually supported in ASP.NET, also, for added fun and games

    Yes they are, if you add a reference to System.Drawing

    Well, I guess someone ought to tell Microsoft to update their documentation and remove the scare quote on the System.Drawing namespace:

    Classes within the System.Drawing namespace are not supported for use within a Windows or ASP.NET service. Attempting to use these classes from within one of these application types may produce unexpected problems, such as diminished service performance and run-time exceptions.

Leave a comment on “Seeing Sextuple”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article