Details

      Description

      The code in NetServlet has a few quality issues, some of which are easily addressed. I plan to address the easy ones here, the main one will be removing unused variables.

      1. derby-5639-4a-xss.diff
        9 kB
        Kristian Waagan
      2. derby-5639-3a-html.diff
        2 kB
        Kristian Waagan
      3. derby-5639-2a-misc.stat
        1.0 kB
        Kristian Waagan
      4. derby-5639-2a-misc.diff
        15 kB
        Kristian Waagan
      5. derby-5639-1a-remove_request_param.diff
        13 kB
        Kristian Waagan

        Activity

        Hide
        Kristian Waagan added a comment -

        Attaching patch 1a, which removes the commonly unused request paramter from several methods.

        Committed to trunk with revision 1297016.

        Show
        Kristian Waagan added a comment - Attaching patch 1a, which removes the commonly unused request paramter from several methods. Committed to trunk with revision 1297016.
        Hide
        Kristian Waagan added a comment -

        Attaching patch 2a, containing minor code cleanups.
        Most notably:
        o made 'Runnable service' local (was instance variable)
        o removed three unused messages (also from the resource bundle files, i.e. servlet_LANG.properties)
        o made getHtmlLabelledMessageInstance private
        o removed a bunch of unused local variable initialization values

        Patch ready for review.

        Show
        Kristian Waagan added a comment - Attaching patch 2a, containing minor code cleanups. Most notably: o made 'Runnable service' local (was instance variable) o removed three unused messages (also from the resource bundle files, i.e. servlet_LANG.properties) o made getHtmlLabelledMessageInstance private o removed a bunch of unused local variable initialization values Patch ready for review.
        Hide
        Kristian Waagan added a comment -

        Committed patch 2a to trunk with revision 1299600.

        Show
        Kristian Waagan added a comment - Committed patch 2a to trunk with revision 1299600.
        Hide
        Kristian Waagan added a comment -

        Attached patch 3a, which fixes some HTML errors.
        Added simple doctype, fixed ordering, added head tag.

        Committed patch 3a to trunk with revision 1299982.

        Show
        Kristian Waagan added a comment - Attached patch 3a, which fixes some HTML errors. Added simple doctype, fixed ordering, added head tag. Committed patch 3a to trunk with revision 1299982.
        Hide
        Kristian Waagan added a comment -

        Attaching patch 4a, which should protect the servlet against basic XSS attacks. It also addresses a few non-XSS issues.

        Brief descriptions:
        o use a safer value for the form action attribute
        o write Integer instead of raw String in message
        (this was safe in the current implementation, but not good practice)
        o escaped strings passed to langUtil.getTextMessage
        o made error reporting less verbose when the form parameter is unknown
        o added missing ';' in escapeSingleQuotes
        o added esacpeHTML

        There are no tests for NetServlet, so I have tested it manually.
        Patch ready for review.

        Show
        Kristian Waagan added a comment - Attaching patch 4a, which should protect the servlet against basic XSS attacks. It also addresses a few non-XSS issues. Brief descriptions: o use a safer value for the form action attribute o write Integer instead of raw String in message (this was safe in the current implementation, but not good practice) o escaped strings passed to langUtil.getTextMessage o made error reporting less verbose when the form parameter is unknown o added missing ';' in escapeSingleQuotes o added esacpeHTML There are no tests for NetServlet, so I have tested it manually. Patch ready for review.
        Hide
        Kristian Waagan added a comment -

        Committed patch 4a to trunk with revision 1308297.
        I don't plan any more work on this issue.

        Show
        Kristian Waagan added a comment - Committed patch 4a to trunk with revision 1308297. I don't plan any more work on this issue.
        Hide
        Kristian Waagan added a comment -

        Closing issue.

        Show
        Kristian Waagan added a comment - Closing issue.

          People

          • Assignee:
            Kristian Waagan
            Reporter:
            Kristian Waagan
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development