Let's append Redundantly?



  • Just found this is in some old vb.net code I'm migrating from 1.1 to 3.5. and to get it out of the way I hate vb as much as the next guy but I'll be damned if I am gonna rewrite all 1172 code files of this website in c#.  I would also like to add that this file was untouched since it's creation.

    <FONT color=#0000ff size=2><FONT color=#0000ff size=2>Dim</FONT></FONT><FONT size=2> stbContent </FONT><FONT color=#0000ff size=2><FONT color=#0000ff size=2>As</FONT></FONT><FONT size=2> </FONT><FONT color=#0000ff size=2><FONT color=#0000ff size=2>New</FONT></FONT><FONT size=2> StringBuilder(</FONT><FONT color=#a31515 size=2><FONT color=#a31515 size=2>""</FONT></FONT><FONT size=2>)</FONT><FONT size=2>

    stbContent.Append(</FONT><FONT color=#a31515 size=2><FONT color=#a31515 size=2>"<script language='VBScript'>"</FONT></FONT><FONT size=2> & vbCrLf)
    stbContent.Append(</FONT><FONT color=#a31515 size=2><FONT color=#a31515 size=2>"Function GetBigPicture(idGallery)"</FONT></FONT><FONT size=2> & vbCrLf)
    stbContent.Append(</FONT><FONT color=#a31515 size=2><FONT color=#a31515 size=2>"window.open ""photogallery.aspx?imgGallery="" & idGallery , ""_new"", ""toolbar=no, resizable=no, scrollbar=no, height=500, width=625, top=150, left=150"""</FONT></FONT><FONT size=2> & vbCrLf)
    stbContent.Append(</FONT><FONT color=#a31515 size=2><FONT color=#a31515 size=2>"End Function"</FONT></FONT><FONT size=2> & vbCrLf)
    stbContent.Append(</FONT><FONT color=#a31515 size=2><FONT color=#a31515 size=2>"</scr"</FONT></FONT><FONT size=2> & </FONT><FONT color=#a31515 size=2><FONT color=#a31515 size=2>"ipt>"</FONT></FONT><FONT size=2> & vbCrLf)

    </FONT>


  • WTFs:

    1. Using VBScript on the web.
    2. Using a StringBuilder, but concatenating strings anyway.
    3. "</scr" & "ipt>" I wonder if that's supposed to be some retarded attempt at "security".

  • :belt_onion:

    @galgorah said:

    <font size="2">stbContent.Append(</font><font size="2" color="#a31515"><font size="2" color="#a31515">"</scr"</font></font><font size="2"> & </font><font size="2" color="#a31515"><font size="2" color="#a31515">"ipt>"</font></font><font size="2"> & vbCrLf)</font>
     

    quoted for extra vileness


  • Considered Harmful

    I think this might stem from some confusion of client- and server-side code. In Javascript inlined inside HTML, you can't have a closing tag (even inside a string literal) because that will get picked up by the parser, which will assume the script block is ended. So it's fairly common in client-side Javascript to break apart closing tags with a string concatenation (usually at the slash) to avoid that problem.

    Of course, this is run on the server side in VB.NET, where no such issue exists.

    Using VBScript on the client side really is inexcusable though; and I still get a light chuckle from seeing string concatenation within Append calls (though the pattern has shown itself here repeatedly.)

    It's almost always better to write function definitions in the static portion of the HTML/Javascript file and just emit function calls from the server side portion as necessary, as well.

    See also: AppendLine

    (I just realized that that's all just static text. Why go through all the trouble of constructing it as a string to begin with?)



  • @mallard said:

    WTFs:

    1. Using VB(Script) on the web.

     

    FTFY



  •  @mallard said:

    WTFs:

    1. Using VBScript on the web.
    2. Using a StringBuilder, but concatenating strings anyway.
    3. "</scr" & "ipt>" I wonder if that's supposed to be some retarded attempt at "security".

    If VBScript is anything as stupid as JavaScript, #3 there isn't this guy's fault:

    <script>
        document.write("here's a </script> tag");
    </script>

    at least used to cause problems where some browsers would see the first </script> in the string and cutoff the script there.

    Conversation with some co-workers recently mentioned that there is also some misguided security in IE that says you can't document.write a script tag, but you CAN document.write a 'src'+'ipt' tag.  I don't know how true that is though.  Haven't gotten around to looking deeper.  At any event, splitting up </script> tags that are part of other strings, is standard practice for Javascript developers

    As for the appending thing, I know later Java compilers optimize those out so there's no difference.  Does .NET do the same?



  • @vt_mruhlin said:

     @mallard said:

    WTFs:

    1. Using VBScript on the web.
    2. Using a StringBuilder, but concatenating strings anyway.
    3. "</scr" & "ipt>" I wonder if that's supposed to be some retarded attempt at "security".

    If VBScript is anything as stupid as JavaScript, #3 there isn't this guy's fault:

    <script>
        document.write("here's a </script> tag");
    </script>

    at least used to cause problems where some browsers would see the first </script> in the string and cutoff the script there.

    If you want to hide stuff from being parsed by the browser, isn't the non-WTFy way to that these days by using CDATA?  Or is that too enterprisey?

    @vt_mruhlin said:

    At any event, splitting up </script> tags that are part of other strings, is standard practice for Javascript developers

    That doesn't necessarily mean it's not retarded...! 



  • @vt_mruhlin said:

    at least used to cause problems where some browsers would see the first </script> in the string and cutoff the script there.
     

    I didn't believe you at first, but I tested it and it still is at least in IE7 and FF3. Test file:
    <html>
    <body>
    <script type="text/javascipt">
    alert("Testing for </script>");
    alert("This should also alert, not show as text, but it still does");
    </script>
    </body>
    </html>


  • BINNED

    Does the "</script>" behaviour come from an age old bug that has become a compatibility burden or was it actually designed that way??

    Because it looks like one has to write an incredibly stupid parser to come up with this. Seriously, interpreting string literals?!



  • @topspin said:

    Does the "</script>" behaviour come from an age old bug that has become a compatibility burden or was it actually designed that way??

    Because it looks like one has to write an incredibly stupid parser to come up with this. Seriously, interpreting string literals?!

     

    I beleive it's due to older HTML pasers that treated the script tag like any other.

    Of course, the proper thing to do in XHTML nowdays is to use CDATA tags around the script.



  • @topspin said:

    Does the "</script>" behaviour come from an age old bug that has become a compatibility burden or was it actually designed that way??

    Because it looks like one has to write an incredibly stupid parser to come up with this. Seriously, interpreting string literals?!

     

     Actually, the problem is that it doesn't interpret the strings at all.  It's completely content-agnostic.  For all it cares, the whole thing could be

         <foo>This is something "in quotes</foo>"

     To make it recgonize that the content of a <script> tag is script and know to treat quotes specially would complicate the parser.

    I believe the approved way to do this is...

     <script>
         // <!--
         alert("</script>")
         alert("this should show up in an alert, not as text")
         // -->
    </script>

    The JavaScript will ignore the two lines starting with // and the HTML parser will not try to parse anything between <!-- and --> as HTML.

     



  • @dtech said:

    @vt_mruhlin said:

    at least used to cause problems where some browsers would see the first </script> in the string and cutoff the script there.
     

    I didn't believe you at first, but I tested it and it still is at least in IE7 and FF3. Test file:
    <html>
    <body>
    <script type="text/javascipt">
    alert("Testing for </script>");
    alert("This should also alert, not show as text, but it still does");
    </script>
    </body>
    </html>

    This whole thread makes me so very angry.  How can so many people not know how to properly write HTML?  The HTML parser does not try to figure out if an ending tag is inside of a block of Javascript, that is not its job.  The proper way to do this is to properly encode the HTML entities so the parser does not recognize them.  CDATA sections are technically supposed to work, but unfortunately browsers don't tend to properly implement the XML spec.  Using unencoded < and & characters in inline JS is improper unless enclosed in CDATA sections.  However, XHTML support is such an abysmal mess right now that you're better off sticking with HTML 4.01 and just using entity encoding.



  • @dwilliss said:

    @topspin said:

    Does the "</script>" behaviour come from an age old bug that has become a compatibility burden or was it actually designed that way??

    Because it looks like one has to write an incredibly stupid parser to come up with this. Seriously, interpreting string literals?!

     

     Actually, the problem is that it doesn't interpret the strings at all.  It's completely content-agnostic.  For all it cares, the whole thing could be

         <foo>This is something "in quotes</foo>"

     To make it recgonize that the content of a <script> tag is script and know to treat quotes specially would complicate the parser.

    I believe the approved way to do this is...

     <script>
         // <!--
         alert("</script>")
         alert("this should show up in an alert, not as text")
         // -->
    </script>

    The JavaScript will ignore the two lines starting with // and the HTML parser will not try to parse anything between <!-- and --> as HTML.

    Using comments to embed JS is sloppy.  Technically, the parser doesn't even have to pay attention to anything within the comment section, which means if browsers properly implemented the specs the JS would be "invisible".  Also, most browsers have buggy parsing of comment sections causing them to just randomly drop out of parsing as a comment for no reason.  This may have been fixed but used to be an issue with FF and IE.  Regardless, it's still a bad idea. 



  • @mallard said:

    WTFs:

    1. Using VBScript on the web.
    2. Using a StringBuilder, but concatenating strings anyway.
    3. "</scr" & "ipt>" I wonder if that's supposed to be some retarded attempt at "security".

     

    This is VB.NET, judging by the declaration syntax and the fact that VBScript has no built-in StringBuilder class.

    VB.NET on the web is hardly a wtf. It is adorable, though.



  • @sootzoo said:

    This is VB.NET, judging by the declaration syntax and the fact that VBScript has no built-in StringBuilder class.

    The VB.NET code emits VBScript.



  • @sootzoo said:

    @mallard said:

    WTFs:

    1. Using VBScript on the web.
    2. Using a StringBuilder, but concatenating strings anyway.
    3. "</scr" & "ipt>" I wonder if that's supposed to be some retarded attempt at "security".

     

    This is VB.NET, judging by the declaration syntax and the fact that VBScript has no built-in StringBuilder class.

    VB.NET on the web is hardly a wtf. It is adorable, though.

     

    Doesn't ASP.NET give you some kind of <script runat="server"> tag that's really no different than a <? ?> or <% %> or whatever their delimeters for that sort of thing are?



  • @MiffTheFox said:

    Of course, the proper thing to do in XHTML nowdays is to use CDATA tags around the script.

    No, the proper thing to do is include it as a separate file, which allows for it to be cached since it changes less than your presumably dynamic content.

    Nowadays the proper way to handle it would be to use the DOM.



  • @Lingerance said:

    @MiffTheFox said:
    Of course, the proper thing to do in XHTML nowdays is to use CDATA tags around the script.
    No, the proper thing to do is include it as a separate file, which allows for it to be cached since it changes less than your presumably dynamic content.
    Nowadays the proper way to handle it would be to use the DOM.

    How do you set dynamic variables, then?  For any reasonably dynamic site, some inline JS is going to be necessary.  I'm not sure what your DOM comment is about.  DOM itself is tedious and bloated and I tend to avoid it for any actual node generation as it is much simpler to use AJAX to return server-side generated HTML directly to the browser and slap that into the proper place in the DOM using innerHTML.  It renders far faster than DOM and is a lot faster to write, lets you re-use the same server-side templating code and has fewer cross-browser quirks than DOM.



  •  Can't inject a script tag by setting innerHTML (well, you can but it wont run, which klinda defeats the point)



  • @morbiuswilters said:

    How do you set dynamic variables, then? For any reasonably dynamic site, some inline JS is going to be necessary.

    I agree with you, but the situation itself doesn't need something that complicated. Inline JS is ok if you have something really simple (like setting variables), but anything complicated should really be it's own file, especially if it never changes (In other words, if it isn't dynamic).
    @morbiuswilters said:
    I'm not sure what your DOM comment is about.

    Use the DOM to put a script tag out, it is the proper way of handling it, although it isn't the easiest as you stated...
    @morbiuswilters said:
    DOM itself is tedious and bloated and I tend to avoid it for any actual node generation as it is much simpler to use AJAX to return server-side generated HTML directly to the browser and slap that into the proper place in the DOM using innerHTML.

    Yes, for pushing HTML chucks it'd be wiser to use innerHTML, parsing HTML in JS would just be ridiculous.
    @morbiuswilters said:
    It renders far faster than DOM and is a lot faster to write, lets you re-use the same server-side templating code and has fewer cross-browser quirks than DOM.

    Right, but your use case is different from the one presented. The one presented they need to push out a script tag, which should normally be eval'd but in this case it is client side VB. Also as vt_mruhlin pointed out while I was witting this, innerHTML won't work in this situation anyways.



  • @vt_mruhlin said:

    <script>
        document.write("here's a </script> tag");
    </script>

    at least used to cause problems where some browsers would see the first </script> in the string and cutoff the script there.

    Conversation with some co-workers recently mentioned that there is also some misguided security in IE that says you can't document.write a script tag, but you CAN document.write a 'src'+'ipt' tag.  I don't know how true that is though.  Haven't gotten around to looking deeper.  At any event, splitting up </script> tags that are part of other strings, is standard practice for Javascript developers

    A HTML parser is the first thing to see the markup. It sees an opening script tag, then keeps searching for a closing script tag. As soon as it finds one, it passes whatever was in between to the JS engine, which executes the JavaScript. The HTML parser has no understanding of JS syntax at all (of course), so it does not know the closing tag is inside a string. This is not IE's weird behaviour (and certainly not a security restriction), or the weird behaviour of any other browser at all. In fact, browsers are too lenient. HTML 4 is supposed to be SGML, and per SGML, "</" should be enough to close the script element. Browsers ignore that, and wait until they see "</script"

    Oh, and web page authors who use '</scri'+'pt>' are clearly showing their lack of understanding for the language they are using. JavaScript has a way of dealing with it that works perfectly well in real browsers, and complies with SGML as well: '<\/script>'

    From memory (I may be wrong since I never wrote in the language), ASP can also use <script> tags, so presumably you'd have to take the same approach there to avoid the closing script tag breaking the ASP. The author of this code was probably an ASP dev.



  • @MiffTheFox said:

    Of course, the proper thing to do in XHTML nowdays is to use CDATA tags around the script.
     

    That didn't seem to work in Firefox 3 that I just tested. But then it wasn't a valid XHTML document so that could have something to do with it. (CBF putting in the DOCTYPE)

    JSON escapes the / character so it becomes "<\/script>" and that isn't picked up by the HTML parser, although it is annoying when using JSON with a URL: http:\/\/www.example.com\/path\/file\/etc.

     And Google Analytics uses escape() and unescape() to insert the script tags...



  • You never write a script tag in client-side JS.



  • @Lingerance said:

    I agree with you, but the situation itself doesn't need something that complicated. Inline JS is ok if you have something really simple (like setting variables), but anything complicated should really be it's own file, especially if it never changes (In other words, if it isn't dynamic).

    Right, but I was referencing setting variables inline which is still vulnerable to problems if you don't properly encode, so external files don't solve this.  Putting functions inline is just stupid.

     

    @Lingerance said:

    es, for pushing HTML chucks it'd be wiser to use innerHTML, parsing HTML in JS would just be ridiculous.

    Who said anything about parsing?  I'm talking about using DOM to dynamically add nodes.  Using AJAX to request XML/JSON and then building nodes in JS is generally inferior to just using AJAX to get the fully-generated HTML from the server and updating the page with innerHTML.

     

    @Lingerance said:

    Right, but your use case is different from the one presented. The one presented they need to push out a script tag, which should normally be eval'd but in this case it is client side VB. Also as vt_mruhlin pointed out while I was witting this, innerHTML won't work in this situation anyways.

    I apologize for the confusion, I was talking about general best practice, not actually referencing the OP's code.  I spent awhile doing the XML/JSON crap before I realized innerHTML is a far better solution for most AJAX apps.



  • @TarquinWJ said:

    Oh, and web page authors who use '</scri'+'pt>' are clearly showing their lack of understanding for the language they are using. JavaScript has a way of dealing with it that works perfectly well in real browsers, and complies with SGML as well: '<\/script>'

    No, dammit, no!  Read what I fucking wrote: HTML entities should be encoded inside of script tags.  That is the proper way of handling this.  CDATA will also work, in theory, but the browser support is too wonky so it's not worth messing with. 



  • Entities; You can't write &lt; instead of < inside a document.write (which seems to be what you're suggesting, excuse me if I misunderstood), because it will be written as an entity, not decoded before being passed to a script, so instead of creating a script tag, it will write the markup as plain text instead. Not the desired effect.

    You also can't use <!-- --> to handle it (as from a later comment), because inside a script block, it's CDATA *by default* in HTML, so the HTML parser will not treat it as a comment (unless it is an old browser that does not understand script at all). Therefore it ignores the HTML comments completely. It's the JS engine that then ignores the commented line, not the HTML parser. In true XHTML, it's a different matter.

    As for your comment about using a CDATA block inside XML, that works for all browsers that understand XHTML - that means all major browsers except IE. However, when I say XHTML, I mean XHTML served with the correct mime type, not text/html with an XHTML doctype (which browsers will just treat as broken HTML, though the W3C validator will treat is as XHTML).

    The correct way to write it in HTML is <\/script>. The correct way to write it in true XHTML (with the proper mime type), is by wrapping it in CDATA blocks, and then the backslash is optional - of course, you could also use \xxxx escapes or fromCharCode(n) at your leisure, but they're overkill compared with the proper way of just escaping the forward slash with a backslash.

    Hint; I work in the browser industry as a JavaScript QA - prepare your arguments carefully ;)



  • I recall using "<\scr"+"ipt>" before. I can't remember what it was for, or why the hell I would have written a script tag from script though. The wtf that stood out to me was the client side vbscript.



  • @TarquinWJ said:

    Entities; You can't write &lt; instead of < inside a document.write (which seems to be what you're suggesting, excuse me if I misunderstood), because it will be written as an entity, not decoded before being passed to a script, so instead of creating a script tag, it will write the markup as plain text instead. Not the desired effect.

    Yeah, I'm not sure if I just phrased this badly or was so drunk I thought I made sense.  Entities work in XHTML (due to script elements not being CDATA), but not HTML (where script elements are CDATA).  So in XHTML you can declare the section CDATA or entify.

     

    @TarquinWJ said:

    As for your comment about using a CDATA block inside XML, that works for all browsers that understand XHTML - that means all major browsers except IE. However, when I say XHTML, I mean XHTML served with the correct mime type, not text/html with an XHTML doctype (which browsers will just treat as broken HTML, though the W3C validator will treat is as XHTML).

    In my experience, most non-IE browsers may "understand" XHTML but they still have far too many quirks to be used reliably.  Regardless, excluding IE is never an option for me (nor is it for most people), so XHTML is off of the table.

     

    @TarquinWJ said:

    The correct way to write it in HTML is <\/script>. The correct way to write it in true XHTML (with the proper mime type), is by wrapping it in CDATA blocks, and then the backslash is optional - of course, you could also use \xxxx escapes or fromCharCode(n) at your leisure, but they're overkill compared with the proper way of just escaping the forward slash with a backslash.

    That's the correct way for Javascript, not VBScript.  I realize the post you were replying to was JS, just want to clarify that within a script element where the language is defined as VBScript, the concatenation method is the way to go.  However, one should still generally use the backslash escape thing for any "</" sequence that appears in a JS script element, and the concat method for VBScript ones; I know that most HTML UAs will only stop on "</script", but it's just not worth risking with when you can simply do it right to begin with.

     

    Anyway, sorry for jumping on you.  I've never had to write any HTML tags in my inline JS, so I just sort of glossed over what you were saying.  I was so enraged over how many people didn't know how the HTML parser worked that I assumed yours was yet another hack, instead of the proper way for HTML + JS.  I'm going to go ahead and say ever needing any HTML tags in your inline JS is TRWTF.


Log in to reply