Details

    • Patch Available

    Description

      Currently Derby requires RDBNAM on ACCSEC. The DDM spec lists it as optional. For DERBY-728, in order to negotiate the encoding, EXCSAT and ACCSEC will need to remain EBCDIC, so we want the RDBNAM to be optional. We can use the RDBNAM sent in SECCHK instead.

      Once this is done. Client can be changed to only send RDBNAM on ACCSEC if an EBCDIC conversion is possible.

      Attachments

        1. derby-4004-fix_new_protcoltest.diff
          0.8 kB
          Kristian Waagan
        2. derby-4004_diff.txt
          10 kB
          Katherine Marsden
        3. derby-4004_10_4_diff.txt
          8 kB
          Katherine Marsden

        Activity

          Here is a preliminary patch for this issue. I am running tests now. The change makes RDBNAM optional on ACCSEC and initialize the datbase name on SECCHK instead. I added two protocol tests. One for successful connection if there is no RDBNAM on ACCSEC but we send it on SECCHK. Another to test that we throw a SYNTAXRM if there is no RDBNAM on either ACCSEC or SECCHK.

          kmarsden Katherine Marsden added a comment - Here is a preliminary patch for this issue. I am running tests now. The change makes RDBNAM optional on ACCSEC and initialize the datbase name on SECCHK instead. I added two protocol tests. One for successful connection if there is no RDBNAM on ACCSEC but we send it on SECCHK. Another to test that we throw a SYNTAXRM if there is no RDBNAM on either ACCSEC or SECCHK.

          Regression tests passed. Please Review the patch derby-4004_diff.txt

          Thanks

          Kathey

          kmarsden Katherine Marsden added a comment - Regression tests passed. Please Review the patch derby-4004_diff.txt Thanks Kathey
          myrna Myrna van Lunteren added a comment - - edited

          I have not looked at this thoroughly, but I have one comment: are the new .inc files run as part of the ProtocolTest
          (see DERBY-2031)?

          myrna Myrna van Lunteren added a comment - - edited I have not looked at this thoroughly, but I have one comment: are the new .inc files run as part of the ProtocolTest (see DERBY-2031 )?

          The patch 'derby-4004-fix_new_protcoltest.diff' makes the new protocol test run successfully with the new tests added.
          Before the change it ran 150 tests, after Katheys patch it runs 151 (two added, one removed).

          Changes in "protocol.tests" are picked up automatically, but if new files are added java/testing/org/apache/derby/impl/drda/ProtocolTest.java must be updated.
          This test still has the problem that it cannot be run as part of suites.All, but it can be run with 'ant junit-pptesting'.
          Note that the old version of the test is still being run as part of a suite.

          kristwaa Kristian Waagan added a comment - The patch 'derby-4004-fix_new_protcoltest.diff' makes the new protocol test run successfully with the new tests added. Before the change it ran 150 tests, after Katheys patch it runs 151 (two added, one removed). Changes in "protocol.tests" are picked up automatically, but if new files are added java/testing/org/apache/derby/impl/drda/ProtocolTest.java must be updated. This test still has the problem that it cannot be run as part of suites.All, but it can be run with 'ant junit-pptesting'. Note that the old version of the test is still being run as part of a suite.

          Thanks Kristian and Myrna for the information on ProtocolTest. I added Kristian's patch to the original patch. If there are no review comments I will commit this patch tomorrow. I would, however most appreciate a review if anyone has the time.

          Kathey

          kmarsden Katherine Marsden added a comment - Thanks Kristian and Myrna for the information on ProtocolTest. I added Kristian's patch to the original patch. If there are no review comments I will commit this patch tomorrow. I would, however most appreciate a review if anyone has the time. Kathey

          Kathey and I did a patch-code-walk-through on IRC.
          We noticed the following nits:

          • Improve javadoc for dbname on initializeDatabase
          • Add javadoc for Database.setDatabaseName
          • fix whitespace before call to initializeDefaultStatement.
          • in in excsat_secchk_nordbonaccsec.inc, remove comment //writeScalarString RDBNAM "wombat;create=true

          All looks good otherwise.

          myrna Myrna van Lunteren added a comment - Kathey and I did a patch-code-walk-through on IRC. We noticed the following nits: Improve javadoc for dbname on initializeDatabase Add javadoc for Database.setDatabaseName fix whitespace before call to initializeDefaultStatement. in in excsat_secchk_nordbonaccsec.inc, remove comment //writeScalarString RDBNAM "wombat;create=true All looks good otherwise.

          Attached is a patch derby-4004_10_4_diff.txt which backports this change to 10.4. I am putting the change to 10.4 and 10.3 to avoid the change in error message when attempting to connect with multibyte characters in the database name after the fix for derby-4008 for new clients connecting to back rev servers.

          Running tests and will commit Monday if there are no objections.

          kmarsden Katherine Marsden added a comment - Attached is a patch derby-4004_10_4_diff.txt which backports this change to 10.4. I am putting the change to 10.4 and 10.3 to avoid the change in error message when attempting to connect with multibyte characters in the database name after the fix for derby-4008 for new clients connecting to back rev servers. Running tests and will commit Monday if there are no objections.

          People

            kmarsden Katherine Marsden
            kmarsden Katherine Marsden
            Votes:
            0 Vote for this issue
            Watchers:
            Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment

                  Client must be authenticated to access this resource.