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

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

Expand Messages
  • 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 1 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 2 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 3 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 4 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 5 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.