Loading ...
Sorry, an error occurred while loading the content.

Re: [jslint] Method operations on string literals in concatenation chains

Expand Messages
  • Erik Eckhardt
    I also meant to mention that you are (possibly) mixing safe and unsafe strings (in the sense of HTML-escaped or not). I hope that does not bite you. if the
    Message 1 of 8 , Jul 6, 2011
    • 0 Attachment
      I also meant to mention that you are (possibly) mixing safe and unsafe
      strings (in the sense of HTML-escaped or not). I hope that does not bite
      you. if the content can ever come from the database, and the data can ever
      be inserted by a user, then you've opened yourself up to javascript
      injection aka "cross-site scripting". Check out the XSS Cheat
      Sheet<http://ha.ckers.org/xss.html>to get an idea of how hard it is to
      blacklist these attacks out of
      existence.

      On Wed, Jul 6, 2011 at 5:14 PM, emmett.thesane <emmett.thesane@...>wrote:

      > **
      >
      >
      > A coding mistake I find myself making often is exemplified by this
      > contrived mustache-style markup in a JS string:
      >
      > var title = 'blix',
      > ____html =
      > ______'<div>' +
      > ________'<span>{{title}}</span>' +
      > ______'<\/div>'
      > ______.replace(/\{\{title\}\}/g, title);
      >
      > The replace will only operate on the very last string literal, but it's not
      > immediately obvious. Sometimes this will be the desired behaviour, but in
      > our case, we want to do this:
      >
      > var title = 'blix',
      > ____html =
      > ______('<div>' +
      > ________'<span>{{title}}</span>' +
      > ______'<\/div>')
      > ______.replace(/\{\{title\}\}/g, title);
      >
      > (Or an array and join(''), but I've seen arguments both ways on using that)
      >
      > My suggestion is that any method operations on string literals that are
      > part of a chain of concatenations *must* operate on a string expression that
      > is wrapped in brackets
      >
      > So, valid examples according to this rule:
      >
      > var foo = "<span>" + ('{{title}}').replace(/\{\{title\}\}/g, title) +
      > "<\/span>";
      >
      > var foo = "<span>" + ('{{title}}'.replace(/\{\{title\}\}/g, title)) +
      > "<\/span>";
      > (because the string '{{title}}' isn't part of a concatenation)
      >
      > var foo = ("<span>" + '{{title}}' + "<\/span>").replace(/\{\{title\}\}/g,
      > title);
      >
      > Thoughts?
      >
      >
      >


      [Non-text portions of this message have been removed]
    • Emmett Pickerel
      That s a great point, Erik. I do try to scrub anything from an untrusted source (which in my case does, in fact, include the database, even though it is
      Message 2 of 8 , Jul 6, 2011
      • 0 Attachment
        That's a great point, Erik. I do try to scrub anything from an untrusted source (which in my case does, in fact, include the database, even though it is restricted access).

        In re to creating a small library for these tasks: mustache.js. I use the replace method only for very small, self contained pieces of functionality that cannot rely on a bigger library. They usually only do the replace once.

        Cheers,
        Emmett

        On Jul 6, 2011, at 6:21 PM, Erik Eckhardt <erik@...> wrote:

        > I also meant to mention that you are (possibly) mixing safe and unsafe
        > strings (in the sense of HTML-escaped or not). I hope that does not bite
        > you. if the content can ever come from the database, and the data can ever
        > be inserted by a user, then you've opened yourself up to javascript
        > injection aka "cross-site scripting". Check out the XSS Cheat
        > Sheet<http://ha.ckers.org/xss.html>to get an idea of how hard it is to
        > blacklist these attacks out of
        > existence.
        >
        > On Wed, Jul 6, 2011 at 5:14 PM, emmett.thesane <emmett.thesane@...>wrote:
        >
        >> **
        >>
        >>
        >> A coding mistake I find myself making often is exemplified by this
        >> contrived mustache-style markup in a JS string:
        >>
        >> var title = 'blix',
        >> ____html =
        >> ______'<div>' +
        >> ________'<span>{{title}}</span>' +
        >> ______'<\/div>'
        >> ______.replace(/\{\{title\}\}/g, title);
        >>
        >> The replace will only operate on the very last string literal, but it's not
        >> immediately obvious. Sometimes this will be the desired behaviour, but in
        >> our case, we want to do this:
        >>
        >> var title = 'blix',
        >> ____html =
        >> ______('<div>' +
        >> ________'<span>{{title}}</span>' +
        >> ______'<\/div>')
        >> ______.replace(/\{\{title\}\}/g, title);
        >>
        >> (Or an array and join(''), but I've seen arguments both ways on using that)
        >>
        >> My suggestion is that any method operations on string literals that are
        >> part of a chain of concatenations *must* operate on a string expression that
        >> is wrapped in brackets
        >>
        >> So, valid examples according to this rule:
        >>
        >> var foo = "<span>" + ('{{title}}').replace(/\{\{title\}\}/g, title) +
        >> "<\/span>";
        >>
        >> var foo = "<span>" + ('{{title}}'.replace(/\{\{title\}\}/g, title)) +
        >> "<\/span>";
        >> (because the string '{{title}}' isn't part of a concatenation)
        >>
        >> var foo = ("<span>" + '{{title}}' + "<\/span>").replace(/\{\{title\}\}/g,
        >> title);
        >>
        >> Thoughts?
        >>
        >>
        >>
        >
        >
        > [Non-text portions of this message have been removed]
        >
        >
        >
        > ------------------------------------
        >
        > Yahoo! Groups Links
        >
        >
        >
      • Rob Richardson
        I like where this is going. I d also point out that concatenating strings isn t very performant. I would refactor to this: var title = blix , ____html = [
        Message 3 of 8 , Jul 6, 2011
        • 0 Attachment
          I like where this is going. I'd also point out that concatenating strings
          isn't very performant. I would refactor to this:

          var title = 'blix',
          ____html = [
          ______'<div>',
          ________'<span>{{title}}</span>',
          ______'<\/div>'
          ____].join('').replace(/\{\{title\}\}/g, title);

          At this point, .replace() always happens on the results of .join(), negating
          the concern.

          Rob


          -----Original Message-----
          From: jslint_com@yahoogroups.com [mailto:jslint_com@yahoogroups.com] On
          Behalf Of Emmett Pickerel
          Sent: Wednesday, July 06, 2011 7:27 PM
          To: jslint_com@yahoogroups.com
          Subject: Re: [jslint] Method operations on string literals in concatenation
          chains

          That's a great point, Erik. I do try to scrub anything from an untrusted
          source (which in my case does, in fact, include the database, even though it
          is restricted access).

          In re to creating a small library for these tasks: mustache.js. I use the
          replace method only for very small, self contained pieces of functionality
          that cannot rely on a bigger library. They usually only do the replace
          once.

          Cheers,
          Emmett

          On Jul 6, 2011, at 6:21 PM, Erik Eckhardt <erik@...> wrote:

          > I also meant to mention that you are (possibly) mixing safe and unsafe
          > strings (in the sense of HTML-escaped or not). I hope that does not bite
          > you. if the content can ever come from the database, and the data can ever
          > be inserted by a user, then you've opened yourself up to javascript
          > injection aka "cross-site scripting". Check out the XSS Cheat
          > Sheet<http://ha.ckers.org/xss.html>to get an idea of how hard it is to
          > blacklist these attacks out of
          > existence.
          >
          > On Wed, Jul 6, 2011 at 5:14 PM, emmett.thesane
          <emmett.thesane@...>wrote:
          >
          >> **
          >>
          >>
          >> A coding mistake I find myself making often is exemplified by this
          >> contrived mustache-style markup in a JS string:
          >>
          >> var title = 'blix',
          >> ____html =
          >> ______'<div>' +
          >> ________'<span>{{title}}</span>' +
          >> ______'<\/div>'
          >> ______.replace(/\{\{title\}\}/g, title);
          >>
          >> The replace will only operate on the very last string literal, but it's
          not
          >> immediately obvious. Sometimes this will be the desired behaviour, but in
          >> our case, we want to do this:
          >>
          >> var title = 'blix',
          >> ____html =
          >> ______('<div>' +
          >> ________'<span>{{title}}</span>' +
          >> ______'<\/div>')
          >> ______.replace(/\{\{title\}\}/g, title);
          >>
          >> (Or an array and join(''), but I've seen arguments both ways on using
          that)
          >>
          >> My suggestion is that any method operations on string literals that are
          >> part of a chain of concatenations *must* operate on a string expression
          that
          >> is wrapped in brackets
          >>
          >> So, valid examples according to this rule:
          >>
          >> var foo = "<span>" + ('{{title}}').replace(/\{\{title\}\}/g, title) +
          >> "<\/span>";
          >>
          >> var foo = "<span>" + ('{{title}}'.replace(/\{\{title\}\}/g, title)) +
          >> "<\/span>";
          >> (because the string '{{title}}' isn't part of a concatenation)
          >>
          >> var foo = ("<span>" + '{{title}}' + "<\/span>").replace(/\{\{title\}\}/g,
          >> title);
          >>
          >> Thoughts?
          >>
          >>
          >>
          >
          >
          > [Non-text portions of this message have been removed]
          >
          >
          >
          > ------------------------------------
          >
          > Yahoo! Groups Links
          >
          >
          >
        • Luke Page
          You have to be concatenating a massive number of strings for that to make real world difference. Otherwise it is just needlessly complicating things. Ideally
          Message 4 of 8 , Jul 7, 2011
          • 0 Attachment
            You have to be concatenating a massive number of strings for that to make
            real world difference. Otherwise it is just needlessly complicating things.

            Ideally for best clarity and separation your html template will be defined
            separate from your template mechanism and separate from logic generating
            your replacement values.

            Partly because of this I can't think of any times I've seen a bug along
            these lines. The most common intention in code I've seen would be the case
            you don't want! I would rather be forced to write
            A = "b"+"c";
            A = A.x();

            For your case and I certainly don't think it looks nice having to write

            A = "b" + ("c").x();

            You might as well require every expression have brackets to ensure the
            programmer doesn't have a bad knowledge of operator precedence (which I
            already do in non obvious cases). I'd consider this a reasonably obvious
            case.

            What do people think about operator precedence / bracket usage in general -
            are there any rules everyone agrees would be useful?
            On 7 Jul 2011 07:49, "Rob Richardson" <erobrich@...> wrote:


            [Non-text portions of this message have been removed]
          • Erik Eckhardt
            Why concatenate string literals together, anyway? A = a + b + c .replace(); seems like it simply ought to be: A = abc .replace(); Perhaps it is
            Message 5 of 8 , Jul 7, 2011
            • 0 Attachment
              Why concatenate string literals together, anyway?

              A = "a" + "b" + "c".replace();

              seems like it simply ought to be:

              A = "abc".replace();

              Perhaps it is multi-line strings that introduce bugs more than improper
              parentheses for the replace function.

              On Thu, Jul 7, 2011 at 12:53 AM, Luke Page <luke.a.page@...> wrote:

              > **
              >
              >
              > You have to be concatenating a massive number of strings for that to make
              > real world difference. Otherwise it is just needlessly complicating things.
              >
              > Ideally for best clarity and separation your html template will be defined
              > separate from your template mechanism and separate from logic generating
              > your replacement values.
              >
              > Partly because of this I can't think of any times I've seen a bug along
              > these lines. The most common intention in code I've seen would be the case
              > you don't want! I would rather be forced to write
              > A = "b"+"c";
              > A = A.x();
              >
              > For your case and I certainly don't think it looks nice having to write
              >
              > A = "b" + ("c").x();
              >
              > You might as well require every expression have brackets to ensure the
              > programmer doesn't have a bad knowledge of operator precedence (which I
              > already do in non obvious cases). I'd consider this a reasonably obvious
              > case.
              >
              > What do people think about operator precedence / bracket usage in general -
              > are there any rules everyone agrees would be useful?
              >
              > On 7 Jul 2011 07:49, "Rob Richardson" <erobrich@...> wrote:
              >
              > [Non-text portions of this message have been removed]
              >
              >
              >


              [Non-text portions of this message have been removed]
            • Emmett Pickerel
              Most recently, I ve been doing a lot of embed codes (object + params + embed). I break them into multiple lines to aid readability, as they tend to be quite
              Message 6 of 8 , Jul 7, 2011
              • 0 Attachment
                Most recently, I've been doing a lot of embed codes (object + params + embed). I break them into multiple lines to aid readability, as they tend to be quite long and would wrap in odd places.


                ----- Original Message -----
                From: Erik Eckhardt <erik@...>
                To: jslint_com@yahoogroups.com
                Cc:
                Sent: Thursday, 7 July 2011, 8:37
                Subject: Re: [jslint] Method operations on string literals in concatenation chains

                Why concatenate string literals together, anyway?

                A = "a" + "b" + "c".replace();

                seems like it simply ought to be:

                A = "abc".replace();

                Perhaps it is multi-line strings that introduce bugs more than improper
                parentheses for the replace function.

                On Thu, Jul 7, 2011 at 12:53 AM, Luke Page <luke.a.page@...> wrote:

                > **
                >
                >
                > You have to be concatenating a massive number of strings for that to make
                > real world difference. Otherwise it is just needlessly complicating things.
                >
                > Ideally for best clarity and separation your html template will be defined
                > separate from your template mechanism and separate from logic generating
                > your replacement values.
                >
                > Partly because of this I can't think of any times I've seen a bug along
                > these lines. The most common intention in code I've seen would be the case
                > you don't want! I would rather be forced to write
                > A = "b"+"c";
                > A = A.x();
                >
                > For your case and I certainly don't think it looks nice having to write
                >
                > A = "b" + ("c").x();
                >
                > You might as well require every expression have brackets to ensure the
                > programmer doesn't have a bad knowledge of operator precedence (which I
                > already do in non obvious cases). I'd consider this a reasonably obvious
                > case.
                >
                > What do people think about operator precedence / bracket usage in general -
                > are there any rules everyone agrees would be useful?
                >
                > On 7 Jul 2011 07:49, "Rob Richardson" <erobrich@...> wrote:
                >
                > [Non-text portions of this message have been removed]
                >

                >


                [Non-text portions of this message have been removed]



                ------------------------------------

                Yahoo! Groups Links
              Your message has been successfully submitted and would be delivered to recipients shortly.