glassfish
  1. glassfish
  2. GLASSFISH-20510

URISyntaxException getting monitoring data

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 4.0_b88_RC4
    • Fix Version/s: 4.0_b89_RC5, 4.1
    • Component/s: monitoring
    • Labels:
      None

      Description

      1. start GF and derby
      2. asadmin set configs.config.server-config.monitoring-service.module-monitoring-levels.ejb-container=HIGH
      3. In devtests/ejb/timer/timertests do 'ant build deploy run'
      4. asadmin get -m "*"
      remote failure: Error during authorization
      java.net.URISyntaxException: Illegal character in path at index 100: admin:/server/applications/ejb-timer-service-app/TimerBean/bean-methods/countTimersOwnedByServerIds-[Ljava%5C/lang%5C/String;/dotted-name
      Command get failed.

      Full stack trace:
      java.net.URISyntaxException: Illegal character in path at index 100: admin:/server/applications/ejb-timer-service-app/TimerBean/bean-methods/countTimersOwnedByServerIds-[Ljava%5C/lang%5C/String;/dotted-name
      at java.net.URI$Parser.fail(URI.java:2829)
      at java.net.URI$Parser.checkChars(URI.java:3002)
      at java.net.URI$Parser.parseHierarchical(URI.java:3086)
      at java.net.URI$Parser.parse(URI.java:3034)
      at java.net.URI.<init>(URI.java:824)
      at com.sun.enterprise.admin.util.CommandSecurityChecker.resourceURIFromAccessCheck(CommandSecurityChecker.java:363)
      at com.sun.enterprise.admin.util.CommandSecurityChecker.checkAccessRequired(CommandSecurityChecker.java:253)
      at com.sun.enterprise.admin.util.CommandSecurityChecker.authorize(CommandSecurityChecker.java:193)
      at com.sun.enterprise.v3.admin.CommandRunnerImpl.doCommand(CommandRunnerImpl.java:1203)
      at com.sun.enterprise.v3.admin.CommandRunnerImpl.access$1500(CommandRunnerImpl.java:108)
      at com.sun.enterprise.v3.admin.CommandRunnerImpl$ExecutionContext.execute(CommandRunnerImpl.java:1762)
      at com.sun.enterprise.v3.admin.CommandRunnerImpl$ExecutionContext.execute(CommandRunnerImpl.java:1674)
      at org.glassfish.admin.rest.resources.admin.CommandResource.executeCommand(CommandResource.java:396)
      at org.glassfish.admin.rest.resources.admin.CommandResource.execCommandSimpInMultOut(CommandResource.java:234)
      at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
      at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      at java.lang.reflect.Method.invoke(Method.java:601)
      at org.glassfish.jersey.server.model.internal.ResourceMethodInvocationHandlerFactory$1.invoke(ResourceMethodInvocationHandlerFactory.java:81)
      at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.invoke(AbstractJavaResourceMethodDispatcher.java:125)
      at org.glassfish.jersey.server.model.internal.JavaResourceMethodDispatcherProvider$ResponseOutInvoker.doDispatch(JavaResourceMethodDispatcherProvider.java:152)
      at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.dispatch(AbstractJavaResourceMethodDispatcher.java:91)
      at org.glassfish.jersey.server.model.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:346)
      at org.glassfish.jersey.server.model.ResourceMethodInvoker.apply(ResourceMethodInvoker.java:341)
      at org.glassfish.jersey.server.model.ResourceMethodInvoker.apply(ResourceMethodInvoker.java:101)
      at org.glassfish.jersey.server.ServerRuntime$1.run(ServerRuntime.java:224)
      at org.glassfish.jersey.internal.Errors$1.call(Errors.java:271)
      at org.glassfish.jersey.internal.Errors$1.call(Errors.java:267)
      at org.glassfish.jersey.internal.Errors.process(Errors.java:315)
      at org.glassfish.jersey.internal.Errors.process(Errors.java:297)
      at org.glassfish.jersey.internal.Errors.process(Errors.java:267)
      at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:317)
      at org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:198)
      at org.glassfish.jersey.server.ApplicationHandler.handle(ApplicationHandler.java:946)
      at org.glassfish.jersey.grizzly2.httpserver.GrizzlyHttpContainer.service(GrizzlyHttpContainer.java:331)
      at org.glassfish.admin.rest.adapter.JerseyContainerCommandService$1.service(JerseyContainerCommandService.java:169)
      at org.glassfish.admin.rest.adapter.RestAdapter.service(RestAdapter.java:179)
      at com.sun.enterprise.v3.services.impl.ContainerMapper.service(ContainerMapper.java:246)
      at org.glassfish.grizzly.http.server.HttpHandler.runService(HttpHandler.java:191)
      at org.glassfish.grizzly.http.server.HttpHandler.doHandle(HttpHandler.java:168)
      at org.glassfish.grizzly.http.server.HttpServerFilter.handleRead(HttpServerFilter.java:189)
      at org.glassfish.grizzly.filterchain.ExecutorResolver$9.execute(ExecutorResolver.java:119)
      at org.glassfish.grizzly.filterchain.DefaultFilterChain.executeFilter(DefaultFilterChain.java:288)
      at org.glassfish.grizzly.filterchain.DefaultFilterChain.executeChainPart(DefaultFilterChain.java:206)
      at org.glassfish.grizzly.filterchain.DefaultFilterChain.execute(DefaultFilterChain.java:136)
      at org.glassfish.grizzly.filterchain.DefaultFilterChain.process(DefaultFilterChain.java:114)
      at org.glassfish.grizzly.ProcessorExecutor.execute(ProcessorExecutor.java:77)
      at org.glassfish.grizzly.nio.transport.TCPNIOTransport.fireIOEvent(TCPNIOTransport.java:838)
      at org.glassfish.grizzly.strategies.AbstractIOStrategy.fireIOEvent(AbstractIOStrategy.java:113)
      at org.glassfish.grizzly.strategies.WorkerThreadIOStrategy.run0(WorkerThreadIOStrategy.java:115)
      at org.glassfish.grizzly.strategies.WorkerThreadIOStrategy.access$100(WorkerThreadIOStrategy.java:55)
      at org.glassfish.grizzly.strategies.WorkerThreadIOStrategy$WorkerThreadRunnable.run(WorkerThreadIOStrategy.java:135)
      at org.glassfish.grizzly.threadpool.AbstractThreadPool$Worker.doWork(AbstractThreadPool.java:564)
      at org.glassfish.grizzly.threadpool.AbstractThreadPool$Worker.run(AbstractThreadPool.java:544)
      at java.lang.Thread.run(Thread.java:722)

        Issue Links

          Activity

          Hide
          Tim Quinn added a comment -

          Because command authorization is involved here, I looked at this problem. With Marina's suggestion to add

          asadmin set configs.config.server-config.monitoring-service.module-monitoring-levels.ejb-container=HIGH

          before trying the "asadmin get" command, I was able to reproduce the problem.

          This seems to be a new failure that does not seem to be related to a change in command authorization itself.

          The "get" command creates AccessCheck objects dynamically based on what specific data is retrieved (rather than, for example, using the @AccessRequired annotation which would require knowing at build-time what resource name is being accessed). As part of creating the AccessCheck object, the resource name is converted into a URI which the authorization service can consume.

          For the "get" command the resource name is derived from the dotted name of the data item being accessed. In this case, the dotted name is

          "server.applications.ejb-timer-service-app.TimerBean.bean-methods.countTimersOwnedByServerIds-[Ljava\.lang\.String;.dotted-name"

          It's the MonitoringReporter class which converts the dotted names into resource names. Up until now at least the dotted names processed during "get" have been valid Java identifiers, so that class's getAccessChecks method did not need to do any URI encoding to handle special characters.

          Is this dotted name what's intended? If so, then the MonitoringReporter.getAccessChecks method can be revised to encode the URI.

          If not then whatever is creating the dotted name should be changed to create a valid string.

          Show
          Tim Quinn added a comment - Because command authorization is involved here, I looked at this problem. With Marina's suggestion to add asadmin set configs.config.server-config.monitoring-service.module-monitoring-levels.ejb-container=HIGH before trying the "asadmin get" command, I was able to reproduce the problem. This seems to be a new failure that does not seem to be related to a change in command authorization itself. The "get" command creates AccessCheck objects dynamically based on what specific data is retrieved (rather than, for example, using the @AccessRequired annotation which would require knowing at build-time what resource name is being accessed). As part of creating the AccessCheck object, the resource name is converted into a URI which the authorization service can consume. For the "get" command the resource name is derived from the dotted name of the data item being accessed. In this case, the dotted name is "server.applications.ejb-timer-service-app.TimerBean.bean-methods.countTimersOwnedByServerIds-[Ljava\.lang\.String;.dotted-name" It's the MonitoringReporter class which converts the dotted names into resource names. Up until now at least the dotted names processed during "get" have been valid Java identifiers, so that class's getAccessChecks method did not need to do any URI encoding to handle special characters. Is this dotted name what's intended? If so, then the MonitoringReporter.getAccessChecks method can be revised to encode the URI. If not then whatever is creating the dotted name should be changed to create a valid string.
          Hide
          marina vatkina added a comment -

          Fixed the steps in the description.

          Show
          marina vatkina added a comment - Fixed the steps in the description.
          Hide
          Byron Nevins added a comment -

          Following the instructions this looks fishy –

          --libraries $

          {libraries}

          Is this right?


          deploy-common-pe:
          [exec] asadmin --host localhost --port 4848 --user admin --passwordfile /Users/wnevins/dev/v2/appserv-tests/config/adminpassword.txt --interactive=false --echo=true --terse=true deploy --libraries ${libraries}

          --force=false --precompilejsp=false --verify=false --retrieve /Users/wnevins/dev/v2/appserv-tests/build/module/archive --generatermistubs=false --availabilityenabled=false --asyncreplication=true --target server --keepreposdir=false --keepfailedstubs=false --isredeploy=false --logreportederrors=true --_classicstyle=false /Users/wnevins/dev/v2/appserv-tests/build/module/archive/ejb-timer-timertestsApp.ear
          [exec] Application deployed with name ejb-timer-timertestsApp.
          [echo] Deployment on target server server successful

          Show
          Byron Nevins added a comment - Following the instructions this looks fishy – --libraries $ {libraries} Is this right? deploy-common-pe: [exec] asadmin --host localhost --port 4848 --user admin --passwordfile /Users/wnevins/dev/v2/appserv-tests/config/adminpassword.txt --interactive=false --echo=true --terse=true deploy --libraries ${libraries} --force=false --precompilejsp=false --verify=false --retrieve /Users/wnevins/dev/v2/appserv-tests/build/module/archive --generatermistubs=false --availabilityenabled=false --asyncreplication=true --target server --keepreposdir=false --keepfailedstubs=false --isredeploy=false --logreportederrors=true --_classicstyle=false /Users/wnevins/dev/v2/appserv-tests/build/module/archive/ejb-timer-timertestsApp.ear [exec] Application deployed with name ejb-timer-timertestsApp. [echo] Deployment on target server server successful
          Hide
          marina vatkina added a comment -

          This doesn't affect monitoring

          Show
          marina vatkina added a comment - This doesn't affect monitoring
          Hide
          Byron Nevins added a comment -

          This probably has nothing to do with Monitoring, and everything to do with the authorization code that was added to the get command.

          Show
          Byron Nevins added a comment - This probably has nothing to do with Monitoring, and everything to do with the authorization code that was added to the get command.
          Hide
          Byron Nevins added a comment -

          Analysis:

          EjbMonitoringUtils.stringify() – it sets the name of the probe to be the method name with the args appended.

          If the args happen to have an array then a "[" character will be in the name.
          Which URI parsing code barfs on and throws an Exception.
          The security code doesn't handle that exception – it just reports it and the command fails.

          It is unusual to create a probe name in this fashion. It is only done by ejb-container.
          Nevertheless I think that the security code ought to handle it in any case – even if the above method is changed to NOT put "[" into names.

          – that's why I've assigned it to Tim.

          Marina could provide a work-around by not allowing the "[" into names by,say, search & replace of that character with something else.

          Note:
          The probes of interest are in ejb container ->

          ~/dev/bg/main/appserver/ejb> tg countTimersOwnedByServerIds
          /Users/wnevins/dev/bg/main/appserver/ejb/ejb-full-container/src/main/java/org/glassfish/ejb/persistent/timer/PersistentEJBTimerService.java[189]:
          totalTimers = timerLocal_.countTimersOwnedByServerIds(serverIds);
          /Users/wnevins/dev/bg/main/appserver/ejb/ejb-full-container/src/main/java/org/glassfish/ejb/persistent/timer/TimerBean.java[588]:
          public String[] countTimersOwnedByServerIds(String[] serverIds) {
          /Users/wnevins/dev/bg/main/appserver/ejb/ejb-full-container/src/main/java/org/glassfish/ejb/persistent/timer/TimerLocal.java[148]:
          String[] countTimersOwnedByServerIds(String[] serverIds);

          Show
          Byron Nevins added a comment - Analysis: EjbMonitoringUtils.stringify() – it sets the name of the probe to be the method name with the args appended. If the args happen to have an array then a "[" character will be in the name. Which URI parsing code barfs on and throws an Exception. The security code doesn't handle that exception – it just reports it and the command fails. It is unusual to create a probe name in this fashion. It is only done by ejb-container. Nevertheless I think that the security code ought to handle it in any case – even if the above method is changed to NOT put "[" into names. – that's why I've assigned it to Tim. Marina could provide a work-around by not allowing the "[" into names by,say, search & replace of that character with something else. Note: The probes of interest are in ejb container -> ~/dev/bg/main/appserver/ejb> tg countTimersOwnedByServerIds /Users/wnevins/dev/bg/main/appserver/ejb/ejb-full-container/src/main/java/org/glassfish/ejb/persistent/timer/PersistentEJBTimerService.java [189] : totalTimers = timerLocal_.countTimersOwnedByServerIds(serverIds); /Users/wnevins/dev/bg/main/appserver/ejb/ejb-full-container/src/main/java/org/glassfish/ejb/persistent/timer/TimerBean.java [588] : public String[] countTimersOwnedByServerIds(String[] serverIds) { /Users/wnevins/dev/bg/main/appserver/ejb/ejb-full-container/src/main/java/org/glassfish/ejb/persistent/timer/TimerLocal.java [148] : String[] countTimersOwnedByServerIds(String[] serverIds);
          Hide
          Byron Nevins added a comment -

          Note that this issue is a great candidate for adding a permanent regression Dev Test...

          Show
          Byron Nevins added a comment - Note that this issue is a great candidate for adding a permanent regression Dev Test...
          Hide
          Byron Nevins added a comment -

          I made a simple change/built/tested. It fixes the problem just fine:

          I did NOT check it in though...

          ~/dev/v2/appserv-tests/devtests/ejb/timer/timertests> getm "*" | wc -l
          8525

          ====================

          Index: src/main/java/com/sun/ejb/monitoring/stats/EjbMonitoringUtils.java
          ===================================================================
          — src/main/java/com/sun/ejb/monitoring/stats/EjbMonitoringUtils.java (revision 61944)
          +++ src/main/java/com/sun/ejb/monitoring/stats/EjbMonitoringUtils.java (working copy)
          @@ -133,6 +133,7 @@
          sb.append(SEP).append(c.getName().replaceAll("_", "
          ."));
          }
          String result = sb.toString().replaceAll("
          .", "\\\\.");
          + result = StringUtils.replace(result, "[", "_ARRAY_");
          if (_logger.isLoggable(Level.FINE))

          { _logger.fine("==> Converted method String: " + result); }
          Show
          Byron Nevins added a comment - I made a simple change/built/tested. It fixes the problem just fine: I did NOT check it in though... ~/dev/v2/appserv-tests/devtests/ejb/timer/timertests> getm "*" | wc -l 8525 ==================== Index: src/main/java/com/sun/ejb/monitoring/stats/EjbMonitoringUtils.java =================================================================== — src/main/java/com/sun/ejb/monitoring/stats/EjbMonitoringUtils.java (revision 61944) +++ src/main/java/com/sun/ejb/monitoring/stats/EjbMonitoringUtils.java (working copy) @@ -133,6 +133,7 @@ sb.append(SEP).append(c.getName().replaceAll("_", " .")); } String result = sb.toString().replaceAll(" .", "\\\\."); + result = StringUtils.replace(result, "[", "_ ARRAY _"); if (_logger.isLoggable(Level.FINE)) { _logger.fine("==> Converted method String: " + result); }
          Hide
          Byron Nevins added a comment -

          In case you use my work-around code change in the previous comment, you'd also need this import:

          import com.sun.enterprise.util.StringUtils;

          Show
          Byron Nevins added a comment - In case you use my work-around code change in the previous comment, you'd also need this import: import com.sun.enterprise.util.StringUtils;
          Hide
          Tim Quinn added a comment -

          I expect we will use standard URI encoding to deal with this.

          Show
          Tim Quinn added a comment - I expect we will use standard URI encoding to deal with this.
          Hide
          Tim Quinn added a comment -

          What is the impact on the customer of the bug?
          The 'asadmin get -m ' command can incorrectly report an authorization failure, depending on what monitored properties are being accessed.

          How likely is it that a customer will see the bug and how serious is the bug?
          This is a regression which causes at least one EJB devtest to fail.

          What is the cost/risk of fixing the bug?
          low

          How risky is the fix? How much work is the fix? Is the fix complicated?
          low - We will use the standard Java SE URLEncoding class to encode the URI that is based on the property.

          Is there an impact on documentation or message strings?
          No

          Which tests should QA (re)run to verify the fix did not destabilize GlassFish?
          Any tests which use asadmin commands. (The EJB devtest which Marina described earlier will show whether the bug has been fixed or not.)

          Which is the targeted build of 4.0 for this fix?
          4.0_b89

          If this an integration of a new version of a component from another project,
          what are the changes that are being brought in? This might be list of
          Jira issues from that project or a list of revision messages.
          N/A

          Show
          Tim Quinn added a comment - What is the impact on the customer of the bug? The 'asadmin get -m ' command can incorrectly report an authorization failure, depending on what monitored properties are being accessed. How likely is it that a customer will see the bug and how serious is the bug? This is a regression which causes at least one EJB devtest to fail. What is the cost/risk of fixing the bug? low How risky is the fix? How much work is the fix? Is the fix complicated? low - We will use the standard Java SE URLEncoding class to encode the URI that is based on the property. Is there an impact on documentation or message strings? No Which tests should QA (re)run to verify the fix did not destabilize GlassFish? Any tests which use asadmin commands. (The EJB devtest which Marina described earlier will show whether the bug has been fixed or not.) Which is the targeted build of 4.0 for this fix? 4.0_b89 If this an integration of a new version of a component from another project, what are the changes that are being brought in? This might be list of Jira issues from that project or a list of revision messages. N/A
          Hide
          Tom Mueller added a comment -

          Approved for 4.0.

          Show
          Tom Mueller added a comment - Approved for 4.0.
          Hide
          Tim Quinn added a comment -

          Fixes checked into the branch and the trunk.

          Project: glassfish
          Repository: svn
          Revision: 61977
          Author: tjquinn
          Date: 2013-05-14 14:55:27 UTC
          Link:

          Log Message:
          ------------
          Fix for GLASSFISH-20510 URISyntaxException getting monitoring data

          The "get" command, as some other asadmin commands, must compute the admin security access checks dynamically based on exactly which resources that invocation of the command accesses. The CommandSecurityChecker class then submits each individual resource separately to the authorization service, passing each resource as a URI. In the case of "get" the resource names come from the dotted names for the items reported.

          Some EJB monitoring probes (reported using 'get -m "*"' for example) contain characters that are not legal in a URI, but CommandSecurityChecker did not encode such names.

          With this fix, such encoding takes place using the standard SE URLEncoding class. The effect is a no-op if the resource name already conforms to URI/URL rules and encodes the resource name otherwise.

          Approved for 4.0: Tom
          Reviewed: Tom
          Test: Passed QL tests, the sequence of commands identified by Marina in the issue

          Revisions:
          ----------
          61977

          Modified Paths:
          ---------------
          branches/4.0/nucleus/admin/util/src/main/java/com/sun/enterprise/admin/util/CommandSecurityChecker.java

          ======
          Revisions:
          ----------
          61976

          Modified Paths:
          ---------------
          trunk/main/nucleus/admin/util/src/main/java/com/sun/enterprise/admin/util/CommandSecurityChecker.java

          Show
          Tim Quinn added a comment - Fixes checked into the branch and the trunk. Project: glassfish Repository: svn Revision: 61977 Author: tjquinn Date: 2013-05-14 14:55:27 UTC Link: Log Message: ------------ Fix for GLASSFISH-20510 URISyntaxException getting monitoring data The "get" command, as some other asadmin commands, must compute the admin security access checks dynamically based on exactly which resources that invocation of the command accesses. The CommandSecurityChecker class then submits each individual resource separately to the authorization service, passing each resource as a URI. In the case of "get" the resource names come from the dotted names for the items reported. Some EJB monitoring probes (reported using 'get -m "*"' for example) contain characters that are not legal in a URI, but CommandSecurityChecker did not encode such names. With this fix, such encoding takes place using the standard SE URLEncoding class. The effect is a no-op if the resource name already conforms to URI/URL rules and encodes the resource name otherwise. Approved for 4.0: Tom Reviewed: Tom Test: Passed QL tests, the sequence of commands identified by Marina in the issue Revisions: ---------- 61977 Modified Paths: --------------- branches/4.0/nucleus/admin/util/src/main/java/com/sun/enterprise/admin/util/CommandSecurityChecker.java ====== Revisions: ---------- 61976 Modified Paths: --------------- trunk/main/nucleus/admin/util/src/main/java/com/sun/enterprise/admin/util/CommandSecurityChecker.java
          Hide
          Tim Quinn added a comment -

          The change I made earlier side-stepped the problem but in a way that causes problems in the resource names that are constructed for admin access control. The problem does NOT affect 4.0 but should be fixed.

          Show
          Tim Quinn added a comment - The change I made earlier side-stepped the problem but in a way that causes problems in the resource names that are constructed for admin access control. The problem does NOT affect 4.0 but should be fixed.
          Hide
          Tim Quinn added a comment -

          I decided to open a new issue, targeting a release after 4.0, rather than confusing the sequence of events on the old issue.

          Show
          Tim Quinn added a comment - I decided to open a new issue, targeting a release after 4.0, rather than confusing the sequence of events on the old issue.

            People

            • Assignee:
              Tim Quinn
              Reporter:
              marina vatkina
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: