modules/enterprise/binding/src/main/java/org/rhq/bindings/ScriptEngineFactory.java | 36 +- modules/enterprise/binding/src/main/java/org/rhq/bindings/StandardBindings.java | 35 +- modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/ClientMain.java | 155 +++++----- modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/LoginCommand.java | 11 modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/LogoutCommand.java | 7 modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/ScriptCommand.java | 48 ++- modules/enterprise/server/plugins/alert-cli/src/main/java/org/rhq/enterprise/server/plugins/alertCli/CliSender.java | 13 7 files changed, 180 insertions(+), 125 deletions(-)
New commits: commit d003658a94b0f88852f469e9f473eee418377849 Author: Jay Shaughnessy jshaughn@redhat.com Date: Wed Feb 1 14:28:41 2012 -0500
[Bug 785773 - CLI exporter variable not respecting settings] This was rooted in the handling of the standard bindings we inject into the script engine in an interactive CLI session. See the BZ for details but in short this commit: - stops treating every command line entry as a new script with new bindings - ensures that non-standard bindings (like var x = 10) and non-client-dependent standard bindings (like exporter) are maintained between command line entries as well as through login/logout. - treat 'exec -f' commands like extendsions of the command line session. They inherit the current bindings and any modifications made by the script file will affect the command line session after the script file has completed. - ensure that login/logout update client-related bindings as needed, and no others. - don't leave behind manager bindings when the client changes (like on logout) - protect against NPE for consecutive logout commands - trivial: removed unused import from CliSender
diff --git a/modules/enterprise/binding/src/main/java/org/rhq/bindings/ScriptEngineFactory.java b/modules/enterprise/binding/src/main/java/org/rhq/bindings/ScriptEngineFactory.java index 5834922..106c2ae 100644 --- a/modules/enterprise/binding/src/main/java/org/rhq/bindings/ScriptEngineFactory.java +++ b/modules/enterprise/binding/src/main/java/org/rhq/bindings/ScriptEngineFactory.java @@ -88,7 +88,7 @@ public class ScriptEngineFactory { public static ScriptEngine getScriptEngine(String language, PackageFinder packageFinder, StandardBindings bindings) throws ScriptException, IOException { ScriptEngineInitializer initializer = getInitializer(language); - + if (initializer == null) { return null; } @@ -101,7 +101,7 @@ public class ScriptEngineFactory {
return engine; } - + /** * This method is similar to the {@link #getScriptEngine(String, PackageFinder, StandardBindings)} method * but additionally applies a security wrapper on the returned script engine so that the scripts execute @@ -109,9 +109,10 @@ public class ScriptEngineFactory { * * @see #getScriptEngine(String, PackageFinder, StandardBindings) */ - public static ScriptEngine getSecuredScriptEngine(final String language, final PackageFinder packageFinder, final StandardBindings bindings, final PermissionCollection permissions) throws ScriptException, IOException { + public static ScriptEngine getSecuredScriptEngine(final String language, final PackageFinder packageFinder, + final StandardBindings bindings, final PermissionCollection permissions) throws ScriptException, IOException { CodeSource src = new CodeSource(new URL("http://rhq-project.org/scripting"), (Certificate[]) null); - ProtectionDomain scriptDomain = new ProtectionDomain(src, permissions); + ProtectionDomain scriptDomain = new ProtectionDomain(src, permissions); AccessControlContext ctx = new AccessControlContext(new ProtectionDomain[] { scriptDomain }); try { return AccessController.doPrivileged(new PrivilegedExceptionAction<ScriptEngine>() { @@ -121,10 +122,10 @@ public class ScriptEngineFactory { //change in security handling in the rhino script engine //that occured in Java6u27 (due to a CVE desribed here: //https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2011-3544) - + //In Java 6u26 and earlier, it was enough to wrap a script engine //in the sandbox and everything would work. - + //Java 6u27 introduced new behavior where the rhino script engine //remembers the access control context with which it has been //constructed and combines that with the callers protection domain @@ -132,7 +133,7 @@ public class ScriptEngineFactory { //all the code in RHQ that called ScriptEngine.eval* also //had all perms, the scripts would never be sandboxed even if the call //was pushed through the SandboxedScriptEngine. - + //This means that the below wrapping is necessary for the security //to work in java6 pre u27 while the surrounding privileged block //is necessary for the security to be applied in java6 u27 and later. @@ -164,8 +165,8 @@ public class ScriptEngineFactory { boolean deleteExistingBindings) { bindings.preInject(engine);
- Bindings engineBindings = - deleteExistingBindings ? engine.createBindings() : engine.getBindings(ScriptContext.ENGINE_SCOPE); + Bindings engineBindings = deleteExistingBindings ? engine.createBindings() : engine + .getBindings(ScriptContext.ENGINE_SCOPE);
for (Map.Entry<String, Object> entry : bindings.entrySet()) { engineBindings.put(entry.getKey(), entry.getValue()); @@ -177,6 +178,23 @@ public class ScriptEngineFactory { }
/** + * Remove the specified bindings from the engine. + * + * @param engine the engine + * @param keySet the binding keys to be removed + */ + public static void removeBindings(ScriptEngine engine, Set<String> keySet) { + + Bindings engineBindings = engine.getBindings(ScriptContext.ENGINE_SCOPE); + + for (String key : keySet) { + engineBindings.remove(key); + } + + engine.setBindings(engineBindings, ScriptContext.ENGINE_SCOPE); + } + + /** * Goes through the methods of the object found in the <code>scriptEngine</code>'s ENGINE_SCOPE * and for each of them generates a top-level function that is called the same name and accepts the same * parameters. diff --git a/modules/enterprise/binding/src/main/java/org/rhq/bindings/StandardBindings.java b/modules/enterprise/binding/src/main/java/org/rhq/bindings/StandardBindings.java index ad29dc3..fd84c6d 100644 --- a/modules/enterprise/binding/src/main/java/org/rhq/bindings/StandardBindings.java +++ b/modules/enterprise/binding/src/main/java/org/rhq/bindings/StandardBindings.java @@ -97,9 +97,25 @@ public class StandardBindings extends HashMap<String, Object> { put(PRETTY, new TabularWriter(output)); put(ASSERT, new ScriptAssert());
+ setFacade(output, rhqFacade); + } + + /** + * If you want to preserve non-client-dependent bindings when the facade changes, call this as opposed to + * constructing new StandardBindings. + */ + public void setFacade(PrintWriter output, RhqFacade rhqFacade) { + // remove any existing managers + if (null != managers) { + for (String manager : managers.keySet()) { + remove(manager); + } + managers.clear(); + } + //script utils can handle a null facade put(SCRIPT_UTIL, new ScriptUtil(rhqFacade)); - + if (rhqFacade != null) { managers = rhqFacade.getManagers();
@@ -112,15 +128,6 @@ public class StandardBindings extends HashMap<String, Object> { }
putAll(managers); - - //I don't think these should belong in the standard bindings - //because all the controller does is "login/logout" as methods - //of the "rhq" object. ConfigurationEditor is interactive and - //thus unusable outside of console bound CLI. - - //(the below were originally defined in the ScriptCommand) - //bindObjectAndGlobalFuctions(new Controller(client), "rhq"); - //bindObjectAndGlobalFuctions(new ConfigurationEditor(client), "configurationEditor"); }
public void preInject(ScriptEngine scriptEngine) { @@ -128,11 +135,11 @@ public class StandardBindings extends HashMap<String, Object> { ((ScriptAssert) get(ASSERT)).init(scriptEngine); }
- public void postInject(ScriptEngine scriptEngine) { + public void postInject(ScriptEngine scriptEngine) { ScriptEngineFactory.bindIndirectionMethods(scriptEngine, SCRIPT_UTIL); ScriptEngineFactory.bindIndirectionMethods(scriptEngine, ASSERT); } - + public Map.Entry<String, PageControl> getUnlimitedPC() { return castEntry(UNLIMITED_PC, PageControl.class); } @@ -162,8 +169,8 @@ public class StandardBindings extends HashMap<String, Object> { }
public Map<String, Object> getManagers() { - //XXX ideally this should be a projection into our map - return managers; + //XXX ideally this should be a projection into our map + return (null == managers) ? managers = Collections.emptyMap() : managers; }
private Map.Entry<String, Object> getEntry(String key) { diff --git a/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/ClientMain.java b/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/ClientMain.java old mode 100755 new mode 100644 index 7323ecd..0444c3b --- a/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/ClientMain.java +++ b/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/ClientMain.java @@ -80,7 +80,7 @@ public class ClientMain { private String user; private String pass; private ArrayList<String> notes = new ArrayList<String>(); - private boolean showDetailedVersion; + private boolean showDetailedVersion;
// reference to the webservice reference factory private RemoteClient remoteClient; @@ -96,7 +96,7 @@ public class ClientMain {
// Entrance to main. public static void main(String[] args) throws Exception { - initCommands(); + initCommands();
// instantiate ClientMain main = new ClientMain(); @@ -126,10 +126,10 @@ public class ClientMain {
private void initScriptCommandAndServiceCompletor() { ScriptCommand sc = (ScriptCommand) commands.get("exec"); - sc.initBindings(this); - + sc.initClient(this); + this.serviceCompletor.setContext(sc.getContext()); - + if (remoteClient != null) { this.serviceCompletor.setServices(remoteClient.getManagers()); } @@ -156,12 +156,12 @@ public class ClientMain { this.serviceCompletor = new InteractiveJavascriptCompletor(consoleReader); consoleReader.addCompletor(new MultiCompletor(new Completor[] { serviceCompletor, helpCompletor, commandCompletor })); - + //ScriptCommand is super special because it handles executing all the code for us initScriptCommandAndServiceCompletor();
// enable pagination - consoleReader.setUsePagination(true); + consoleReader.setUsePagination(true); }
public String getUserInput(String prompt) { @@ -288,7 +288,8 @@ public class ClientMain { outputWriter.println(command.getPromptCommandString() + ": " + e.getMessage()); outputWriter.println("Usage: " + command.getSyntax()); } catch (ArrayIndexOutOfBoundsException e) { - outputWriter.println(command.getPromptCommandString() + ": An incorrect number of arguments was specified."); + outputWriter.println(command.getPromptCommandString() + + ": An incorrect number of arguments was specified."); outputWriter.println("Usage: " + command.getSyntax()); } } else { @@ -373,7 +374,8 @@ public class ClientMain { }
private void displayUsage() { - outputWriter.println("rhq-cli.sh [-h] [-u user] [-p pass] [-P] [-s host] [-t port] [-v] [-f file]|[-c command]"); + outputWriter + .println("rhq-cli.sh [-h] [-u user] [-p pass] [-P] [-s host] [-t port] [-v] [-f file]|[-c command]"); }
void processArguments(String[] args) throws IllegalArgumentException, IOException { @@ -398,79 +400,80 @@ public class ClientMain {
while ((code = getopt.getopt()) != -1) { switch (code) { - case ':': - case '?': { - // for now both of these should exit - displayUsage(); - throw new IllegalArgumentException(MSG.getMsg(ClientI18NResourceKeys.BAD_ARGS)); - } + case ':': + case '?': { + // for now both of these should exit + displayUsage(); + throw new IllegalArgumentException(MSG.getMsg(ClientI18NResourceKeys.BAD_ARGS)); + }
- case 1: { - // this catches non-option arguments which can be passed when running a script in non-interactive mode - // with -f or running a single command in non-interactive mode with -c. - execCmdLine.add(getopt.getOptarg()); - break; - } + case 1: { + // this catches non-option arguments which can be passed when running a script in non-interactive mode + // with -f or running a single command in non-interactive mode with -c. + execCmdLine.add(getopt.getOptarg()); + break; + }
- case 'h': { - displayUsage(); - break; - } + case 'h': { + displayUsage(); + break; + }
- case 'u': { - this.user = getopt.getOptarg(); - break; - } - case 'p': { - this.pass = getopt.getOptarg(); - break; - } - case 'P': { - this.pass = this.consoleReader.readLine("password: ", (char) 0); - break; - } - case 'c': { - interactiveMode = false; - execCmdLine.add(getopt.getOptarg()); - break; - } - case 'f': { - interactiveMode = false; - execCmdLine.add("-f"); - execCmdLine.add(getopt.getOptarg()); - break; - } - case -2: { - execCmdLine.add("--args-style=" + getopt.getOptarg()); - break; - } - case 's': { - setHost(getopt.getOptarg()); - break; - } - case 'r': { - setTransport(getopt.getOptarg()); - break; - } - case 't': { - String portArg = getopt.getOptarg(); - try { - setPort(Integer.parseInt(portArg)); - } catch (Exception e) { - outputWriter.println("Invalid port [" + portArg + "]"); - } - break; - } - case 'v': { - showDetailedVersion = true; - break; + case 'u': { + this.user = getopt.getOptarg(); + break; + } + case 'p': { + this.pass = getopt.getOptarg(); + break; + } + case 'P': { + this.pass = this.consoleReader.readLine("password: ", (char) 0); + break; + } + case 'c': { + interactiveMode = false; + execCmdLine.add(getopt.getOptarg()); + break; + } + case 'f': { + interactiveMode = false; + execCmdLine.add("-f"); + execCmdLine.add(getopt.getOptarg()); + break; + } + case -2: { + execCmdLine.add("--args-style=" + getopt.getOptarg()); + break; + } + case 's': { + setHost(getopt.getOptarg()); + break; + } + case 'r': { + setTransport(getopt.getOptarg()); + break; + } + case 't': { + String portArg = getopt.getOptarg(); + try { + setPort(Integer.parseInt(portArg)); + } catch (Exception e) { + outputWriter.println("Invalid port [" + portArg + "]"); } + break; + } + case 'v': { + String versionString = Version.getProductNameAndVersionBuildInfo(); + outputWriter.println(versionString); + break; + } } }
if (interactiveMode) { - String version = (showDetailedVersion) ? Version.getProductNameAndVersionBuildInfo() : - Version.getProductNameAndVersion(); + String version = (showDetailedVersion) ? Version.getProductNameAndVersionBuildInfo() : Version + .getProductNameAndVersion(); outputWriter.println(version); if (showDetailedVersion && args.length == 1) { // If -v was the only option specified, exit after printing the version. @@ -501,10 +504,10 @@ public class ClientMain {
public void setRemoteClient(RemoteClient remoteClient) { this.remoteClient = remoteClient; - + initScriptCommandAndServiceCompletor(); } - + public Subject getSubject() { return subject; } diff --git a/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/LoginCommand.java b/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/LoginCommand.java index 72bd8e1..97221af 100644 --- a/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/LoginCommand.java +++ b/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/LoginCommand.java @@ -112,23 +112,16 @@ public class LoginCommand implements ClientCommand { Subject subject = remoteClient.login(username, password);
ProductInfo info = remoteClient.getSystemManager().getProductInfo(subject); - String version = info.getVersion() - + " (" + info.getBuildNumber() + ")"; + String version = info.getVersion() + " (" + info.getBuildNumber() + ")"; client.getPrintWriter().println("Remote server version is: " + version);
+ // this call has the side effect of setting bindings for the new remote client and its subject client.setRemoteClient(remoteClient); client.setSubject(subject);
- bindSubject(client, subject); - return subject; }
- private void bindSubject(ClientMain client, Subject subject) { - ScriptCommand cmd = (ScriptCommand) client.getCommands().get("exec"); - cmd.initBindings(client); - } - private String usage() { return "Usage: " + getSyntax(); } diff --git a/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/LogoutCommand.java b/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/LogoutCommand.java index 6966508..a29731f 100644 --- a/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/LogoutCommand.java +++ b/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/LogoutCommand.java @@ -19,6 +19,7 @@ package org.rhq.enterprise.client.commands;
import org.rhq.enterprise.client.ClientMain; +import org.rhq.enterprise.clientapi.RemoteClient;
/** * @author Greg Hinkle @@ -40,7 +41,11 @@ public class LogoutCommand implements ClientCommand { client.setTransport(null); client.setHost(null); client.setPort(0); - client.getRemoteClient().logout(); + RemoteClient remoteClient = client.getRemoteClient(); + if (null != remoteClient) { + remoteClient.logout(); + } + // this call has the side effect of setting bindings for the new remote client and its subject client.setRemoteClient(null); client.setSubject(null); client.setUser(null); diff --git a/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/ScriptCommand.java b/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/ScriptCommand.java index 4a99316..4e7e79a 100644 --- a/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/ScriptCommand.java +++ b/modules/enterprise/remoting/cli/src/main/java/org/rhq/enterprise/client/commands/ScriptCommand.java @@ -69,15 +69,15 @@ public class ScriptCommand implements ClientCommand { public ScriptEngine getScriptEngine() { if (jsEngine == null) { try { - jsEngine = ScriptEngineFactory.getScriptEngine("JavaScript", new PackageFinder(Arrays - .asList(getLibDir())), null); + jsEngine = ScriptEngineFactory.getScriptEngine("JavaScript", + new PackageFinder(Arrays.asList(getLibDir())), null); } catch (ScriptException e) { e.printStackTrace(); } catch (IOException e) { e.printStackTrace(); } } - + return jsEngine; }
@@ -86,7 +86,14 @@ public class ScriptCommand implements ClientCommand { }
public boolean execute(ClientMain client, String[] args) { - initBindings(client); + // for a command line session we don't want to reset the bindings for each executed command line, the + // state, e.g. exporter settings, should be maintained from line to line. Note that scriptFiles + // executed via 'exec -f' are treated like extensions of the command line session. They inherit the + // current bindings and any modifications made by the script file will affect the command line session + // after the script file has completed. + if (null == bindings) { + initBindings(client); + }
if (isScriptFileCommandLine(args)) { try { @@ -177,29 +184,50 @@ public class ScriptCommand implements ClientCommand { return true; }
- public void initBindings(ClientMain client) { + private void initBindings(ClientMain client) { bindings = new StandardBindings(client.getPrintWriter(), client.getRemoteClient()); - bindings.getSubject().setValue(client.getSubject()); + // init pretty width with console setting bindings.getPretty().getValue().setWidth(client.getConsoleWidth()); + // replace ResourceClientFactory with Editable version or none at all if (client.getRemoteClient() != null) { bindings.getProxyFactory().setValue(new EditableResourceClientFactory(client)); } else { bindings.getProxyFactory().setValue(null); }
- //non-standard bindings + //non-standard bindings for console bindings.put("configurationEditor", new ConfigurationEditor(client)); bindings.put("rhq", new Controller(client));
ScriptEngine engine = getScriptEngine(); - + ScriptEngineFactory.injectStandardBindings(engine, bindings, false);
- ScriptEngineFactory - .bindIndirectionMethods(engine, "configurationEditor"); + ScriptEngineFactory.bindIndirectionMethods(engine, "configurationEditor"); ScriptEngineFactory.bindIndirectionMethods(engine, "rhq"); }
+ public void initClient(ClientMain client) { + if (null == bindings) { + initBindings(client); + + } else { + ScriptEngine engine = getScriptEngine(); + + // remove any current manager bindings from the engine, they may not be valid for the + // new client. The new standard bindings will include any new managers. + ScriptEngineFactory.removeBindings(engine, bindings.getManagers().keySet()); + + bindings.setFacade(client.getPrintWriter(), client.getRemoteClient()); + + // update the engine with the new client bindings. Keep the existing engine bindings as they + // may contain bindings outside this standard set (like any var created by the script or command line user) + ScriptEngineFactory.injectStandardBindings(engine, bindings, false); + } + + return; + } + private void executeUtilScripts() { InputStream stream = getClass().getResourceAsStream("test_utils.js"); InputStreamReader reader = new InputStreamReader(stream); diff --git a/modules/enterprise/server/plugins/alert-cli/src/main/java/org/rhq/enterprise/server/plugins/alertCli/CliSender.java b/modules/enterprise/server/plugins/alert-cli/src/main/java/org/rhq/enterprise/server/plugins/alertCli/CliSender.java index 43090c7..deba524 100644 --- a/modules/enterprise/server/plugins/alert-cli/src/main/java/org/rhq/enterprise/server/plugins/alertCli/CliSender.java +++ b/modules/enterprise/server/plugins/alert-cli/src/main/java/org/rhq/enterprise/server/plugins/alertCli/CliSender.java @@ -40,7 +40,6 @@ import javax.script.ScriptException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory;
-import org.rhq.bindings.SandboxedScriptEngine; import org.rhq.bindings.ScriptEngineFactory; import org.rhq.bindings.StandardBindings; import org.rhq.bindings.StandardScriptPermissions; @@ -135,7 +134,7 @@ public class CliSender extends AlertSender<CliComponent> {
final ExceptionHolder exceptionHolder = new ExceptionHolder();
- final ScriptEngine e = engine; + final ScriptEngine e = engine; Thread scriptRunner = new Thread(new Runnable() { public void run() { try { @@ -336,8 +335,10 @@ public class CliSender extends AlertSender<CliComponent> {
if (versionToUse != null) { ret = ret.replace("$packageName", versionToUse.getDisplayName()); - ret = ret.replace("$packageVersion", versionToUse.getDisplayVersion() == null ? versionToUse - .getVersion() : versionToUse.getDisplayVersion()); + ret = ret.replace( + "$packageVersion", + versionToUse.getDisplayVersion() == null ? versionToUse.getVersion() : versionToUse + .getDisplayVersion()); } else { ret = ret.replace("$packageName", "unknown script with package id " + config.packageId); ret = ret.replace("$packageVersion", "no version"); @@ -413,8 +414,8 @@ public class CliSender extends AlertSender<CliComponent> { ScriptEngine engine = SCRIPT_ENGINES.poll();
if (engine == null) { - engine = ScriptEngineFactory.getSecuredScriptEngine(ENGINE_NAME, new PackageFinder(Collections - .<File> emptyList()), bindings, new StandardScriptPermissions()); + engine = ScriptEngineFactory.getSecuredScriptEngine(ENGINE_NAME, + new PackageFinder(Collections.<File> emptyList()), bindings, new StandardScriptPermissions()); } else { ScriptEngineFactory.injectStandardBindings(engine, bindings, true); }
rhq-commits@lists.fedorahosted.org