Derby
  1. Derby
  2. DERBY-4795

Starting network server with -ssl turns SSL off

    Details

    • Urgency:
      Normal
    • Issue & fix info:
      Newcomer, Repro attached

      Description

      If you start the network server with the -ssl option and no <sslmode> argument, the server will be started in plain text mode.

      For example:

      java -jar derbynet.jar start -ssl

      If -ssl is specified without <sslmode>, I would have expected that SSL was enabled (not sure which SSL mode, basic or peerAuthentication, is more appropriate) or that the command failed because of the missing argument. Treating "-ssl" as an alias for "-ssl off" sounds unintuitive to me.

      1. d4795.diff
        2 kB
        Siddharth Srivastava

        Issue Links

          Activity

          Hide
          Myrna van Lunteren added a comment -

          I got muddled. It's fine as it is. Closing again.

          Show
          Myrna van Lunteren added a comment - I got muddled. It's fine as it is. Closing again.
          Hide
          Myrna van Lunteren added a comment - - edited

          I was considering this change for backport to 10.8, but as it changes the message file for English only, I prefer to, because of the completed translations for 10.8.2, so I only added the label.

          Show
          Myrna van Lunteren added a comment - - edited I was considering this change for backport to 10.8, but as it changes the message file for English only, I prefer to, because of the completed translations for 10.8.2, so I only added the label.
          Hide
          Knut Anders Hatlen added a comment -

          Verified that the problem is fixed. Thanks, Siddharth!

          I've filed DERBY-5261 for the problem with the usage message getting printed twice.

          Show
          Knut Anders Hatlen added a comment - Verified that the problem is fixed. Thanks, Siddharth! I've filed DERBY-5261 for the problem with the usage message getting printed twice.
          Hide
          Tiago R. Espinha added a comment -

          Thank you Kathey. I've fixed it with that command.

          Show
          Tiago R. Espinha added a comment - Thank you Kathey. I've fixed it with that command.
          Hide
          Kathey Marsden added a comment -

          You can fix the svn comment with:
          svn propedit svn:log --revprop -r 1130743

          That won't immediately fix JIRA subversion commits but next time someone builds the index it might.

          Show
          Kathey Marsden added a comment - You can fix the svn comment with: svn propedit svn:log --revprop -r 1130743 That won't immediately fix JIRA subversion commits but next time someone builds the index it might.
          Hide
          Tiago R. Espinha added a comment -

          I forgot to include the JIRA issue in the patch description too late now...

          Show
          Tiago R. Espinha added a comment - I forgot to include the JIRA issue in the patch description too late now...
          Hide
          Tiago R. Espinha added a comment -

          I think this behavior is logical. Usually in usage descriptions, square brackets mean an optional argument and the diamond symbols mean something that is mandatory. Looking at the -ssl switch, the mode is mandatory.

          suites.All has passed and no one has objected to this change, so I committed this patch with revision 1130743.

          @ Siddharth, I'm going to resolve this issue. When this changes get captured by the daily regression runs, and provided that everything goes well, please go ahead and close it.

          Show
          Tiago R. Espinha added a comment - I think this behavior is logical. Usually in usage descriptions, square brackets mean an optional argument and the diamond symbols mean something that is mandatory. Looking at the -ssl switch, the mode is mandatory. suites.All has passed and no one has objected to this change, so I committed this patch with revision 1130743. @ Siddharth, I'm going to resolve this issue. When this changes get captured by the daily regression runs, and provided that everything goes well, please go ahead and close it.
          Hide
          Siddharth Srivastava added a comment -

          Status Update: The current patch has passed the regression test (suites.All)

          Show
          Siddharth Srivastava added a comment - Status Update: The current patch has passed the regression test (suites.All)
          Hide
          Tiago R. Espinha added a comment -

          Is it really intended though? Should we file a new issue to make it only show once?

          I mean, am I missing something or is it only supposed to be shown once?

          Show
          Tiago R. Espinha added a comment - Is it really intended though? Should we file a new issue to make it only show once? I mean, am I missing something or is it only supposed to be shown once?
          Hide
          Knut Anders Hatlen added a comment -

          I don't think the "No command given" message or the extra usage information (it's printed twice) is intended. But the same thing happens if you use the -p or -h option with no value, so it looks like an existing problem and not something with your patch.

          Show
          Knut Anders Hatlen added a comment - I don't think the "No command given" message or the extra usage information (it's printed twice) is intended. But the same thing happens if you use the -p or -h option with no value, so it looks like an existing problem and not something with your patch.
          Hide
          Siddharth Srivastava added a comment -

          Is the line:
          Wed Jun 01 02:45:21 IST 2011 : No command given
          intended ?

          Show
          Siddharth Srivastava added a comment - Is the line: Wed Jun 01 02:45:21 IST 2011 : No command given intended ?
          Hide
          Siddharth Srivastava added a comment -

          Attached is the fix for this issue.

          The output with: java -jar derbynet.jar start -ssl is

          Wed Jun 01 02:45:21 IST 2011 : Missing required value for sslmode.
          Usage: NetworkServerControl <commands>
          Commands:
          start [-h <host>] [-p <portnumber>] [-noSecurityManager] [-ssl <sslmode>]
          shutdown [-h <host>][-p <portnumber>] [-ssl <sslmode>] [-user <username>] [-pass
          word <password>]
          ping [-h <host>][-p <portnumber>] [-ssl <sslmode>]
          sysinfo [-h <host>][-p <portnumber>] [-ssl <sslmode>]
          runtimeinfo [-h <host>][-p <portnumber>] [-ssl <sslmode>]
          logconnections

          {on|off} [-h <host>][-p <portnumber>] [-ssl <sslmode>]
          maxthreads <max>[-h <host>][-p <portnumber>] [-ssl <sslmode>]
          timeslice <milliseconds>[-h <host>][-p <portnumber>] [-ssl <sslmode>]
          trace {on|off}

          [-s <session id>][-h <host>][-p <portnumber>] [-ssl <sslmode>]
          tracedirectory <traceDirectory>[-h <host>][-p <portnumber>] [-ssl <sslmode>]
          Wed Jun 01 02:45:21 IST 2011 : No command given.
          Usage: NetworkServerControl <commands>
          Commands:
          start [-h <host>] [-p <portnumber>] [-noSecurityManager] [-ssl <sslmode>]
          shutdown [-h <host>][-p <portnumber>] [-ssl <sslmode>] [-user <username>] [-pass
          word <password>]
          ping [-h <host>][-p <portnumber>] [-ssl <sslmode>]
          sysinfo [-h <host>][-p <portnumber>] [-ssl <sslmode>]
          runtimeinfo [-h <host>][-p <portnumber>] [-ssl <sslmode>]
          logconnections

          {on|off} [-h <host>][-p <portnumber>] [-ssl <sslmode>]
          maxthreads <max>[-h <host>][-p <portnumber>] [-ssl <sslmode>]
          timeslice <milliseconds>[-h <host>][-p <portnumber>] [-ssl <sslmode>]
          trace {on|off}

          [-s <session id>][-h <host>][-p <portnumber>] [-ssl <sslmode>]
          tracedirectory <traceDirectory>[-h <host>][-p <portnumber>] [-ssl <sslmode>]

          The regressions (suites.All) is curretly being performed. I would update the status of regression test once it is complete.

          Show
          Siddharth Srivastava added a comment - Attached is the fix for this issue. The output with: java -jar derbynet.jar start -ssl is Wed Jun 01 02:45:21 IST 2011 : Missing required value for sslmode. Usage: NetworkServerControl <commands> Commands: start [-h <host>] [-p <portnumber>] [-noSecurityManager] [-ssl <sslmode>] shutdown [-h <host>] [-p <portnumber>] [-ssl <sslmode>] [-user <username>] [-pass word <password>] ping [-h <host>] [-p <portnumber>] [-ssl <sslmode>] sysinfo [-h <host>] [-p <portnumber>] [-ssl <sslmode>] runtimeinfo [-h <host>] [-p <portnumber>] [-ssl <sslmode>] logconnections {on|off} [-h <host>] [-p <portnumber>] [-ssl <sslmode>] maxthreads <max> [-h <host>] [-p <portnumber>] [-ssl <sslmode>] timeslice <milliseconds> [-h <host>] [-p <portnumber>] [-ssl <sslmode>] trace {on|off} [-s <session id>] [-h <host>] [-p <portnumber>] [-ssl <sslmode>] tracedirectory <traceDirectory> [-h <host>] [-p <portnumber>] [-ssl <sslmode>] Wed Jun 01 02:45:21 IST 2011 : No command given. Usage: NetworkServerControl <commands> Commands: start [-h <host>] [-p <portnumber>] [-noSecurityManager] [-ssl <sslmode>] shutdown [-h <host>] [-p <portnumber>] [-ssl <sslmode>] [-user <username>] [-pass word <password>] ping [-h <host>] [-p <portnumber>] [-ssl <sslmode>] sysinfo [-h <host>] [-p <portnumber>] [-ssl <sslmode>] runtimeinfo [-h <host>] [-p <portnumber>] [-ssl <sslmode>] logconnections {on|off} [-h <host>] [-p <portnumber>] [-ssl <sslmode>] maxthreads <max> [-h <host>] [-p <portnumber>] [-ssl <sslmode>] timeslice <milliseconds> [-h <host>] [-p <portnumber>] [-ssl <sslmode>] trace {on|off} [-s <session id>] [-h <host>] [-p <portnumber>] [-ssl <sslmode>] tracedirectory <traceDirectory> [-h <host>] [-p <portnumber>] [-ssl <sslmode>] The regressions (suites.All) is curretly being performed. I would update the status of regression test once it is complete.
          Hide
          Knut Anders Hatlen added a comment -

          I don't think error ids are used in the messages generated by the argument processing. A good starting point may be to look at NetworkServerControlImpl.processDashArg() and see if it can be changed to handle -ssl the same way as other options with required arguments (like -h).

          Show
          Knut Anders Hatlen added a comment - I don't think error ids are used in the messages generated by the argument processing. A good starting point may be to look at NetworkServerControlImpl.processDashArg() and see if it can be changed to handle -ssl the same way as other options with required arguments (like -h).
          Hide
          Siddharth Srivastava added a comment - - edited

          Hi

          I too think a usage error would be appropriate as if some sort of default is there then we'll have to show a message that which sslmode has been used and user may not comply with that.

          If we go with the usage error, in my opinion following error would be appropriate:

          Error <Error Id>: Missing <sslmode> argument
          Usage: -ssl <sslmode>
          SSLMODE can be any of the following:
          SSL_OFF: Turns off SSL. No SSL encryption would be used
          SSL_BASIC: Use basic SSL Encryption
          SSL_PEER_AUTHENTICATION: Use SSL Encryption with Peer Authentication

          I am not very sure about the error id (is it possible to generate an error id ? )

          Please share your opinion.

          Show
          Siddharth Srivastava added a comment - - edited Hi I too think a usage error would be appropriate as if some sort of default is there then we'll have to show a message that which sslmode has been used and user may not comply with that. If we go with the usage error, in my opinion following error would be appropriate: Error <Error Id>: Missing <sslmode> argument Usage: -ssl <sslmode> SSLMODE can be any of the following: SSL_OFF: Turns off SSL. No SSL encryption would be used SSL_BASIC: Use basic SSL Encryption SSL_PEER_AUTHENTICATION: Use SSL Encryption with Peer Authentication I am not very sure about the error id (is it possible to generate an error id ? ) Please share your opinion.
          Hide
          Kathey Marsden added a comment -

          Triage for 10.8. Marked newcomer, but newcomer will have to discuss with community whether a usage error or some sort of default is appropriate. I tend to think a usage error is cleanest.

          Show
          Kathey Marsden added a comment - Triage for 10.8. Marked newcomer, but newcomer will have to discuss with community whether a usage error or some sort of default is appropriate. I tend to think a usage error is cleanest.

            People

            • Assignee:
              Siddharth Srivastava
              Reporter:
              Knut Anders Hatlen
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development