diff options
author | Jeff Sharkey <jsharkey@android.com> | 2011-11-29 15:31:59 -0800 |
---|---|---|
committer | Android (Google) Code Review <android-gerrit@google.com> | 2011-11-29 15:31:59 -0800 |
commit | b468a8fd2b6fba095372afef9272024c9385688d (patch) | |
tree | a5f9f85890df80662e5f00edbbb7b9ddf7e39c05 | |
parent | a5d24d42ff8b99383a8c3051b9459e5248cf8536 (diff) | |
parent | 31c6e4817f6c967fc4f61c4f1d9f25743958f7de (diff) | |
download | frameworks_base-b468a8fd2b6fba095372afef9272024c9385688d.zip frameworks_base-b468a8fd2b6fba095372afef9272024c9385688d.tar.gz frameworks_base-b468a8fd2b6fba095372afef9272024c9385688d.tar.bz2 |
Merge "Move NativeDaemonConnector to varargs."
6 files changed, 386 insertions, 164 deletions
diff --git a/services/java/com/android/server/MountService.java b/services/java/com/android/server/MountService.java index 5425813..022b13a 100644 --- a/services/java/com/android/server/MountService.java +++ b/services/java/com/android/server/MountService.java @@ -1831,7 +1831,11 @@ class MountService extends IMountService.Stub // to let the UI to clear itself mHandler.postDelayed(new Runnable() { public void run() { - mConnector.doCommand(String.format("cryptfs restart")); + try { + mConnector.doCommand(String.format("cryptfs restart")); + } catch (NativeDaemonConnectorException e) { + Slog.e(TAG, "problem executing in background", e); + } } }, 1000); // 1 second } diff --git a/services/java/com/android/server/NativeDaemonConnector.java b/services/java/com/android/server/NativeDaemonConnector.java index 28013bd..1e98f93 100644 --- a/services/java/com/android/server/NativeDaemonConnector.java +++ b/services/java/com/android/server/NativeDaemonConnector.java @@ -24,6 +24,8 @@ import android.os.Message; import android.os.SystemClock; import android.util.Slog; +import com.google.android.collect.Lists; + import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -32,49 +34,33 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; /** - * Generic connector class for interfacing with a native - * daemon which uses the libsysutils FrameworkListener - * protocol. + * Generic connector class for interfacing with a native daemon which uses the + * {@code libsysutils} FrameworkListener protocol. */ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdog.Monitor { - private static final boolean LOCAL_LOGD = false; + private static final boolean LOGD = false; - private BlockingQueue<String> mResponseQueue; - private OutputStream mOutputStream; - private String TAG = "NativeDaemonConnector"; - private String mSocket; - private INativeDaemonConnectorCallbacks mCallbacks; - private Handler mCallbackHandler; + private final String TAG; - /** Lock held whenever communicating with native daemon. */ - private Object mDaemonLock = new Object(); - - private final int BUFFER_SIZE = 4096; + private String mSocket; + private OutputStream mOutputStream; - class ResponseCode { - public static final int ActionInitiated = 100; + private final BlockingQueue<NativeDaemonEvent> mResponseQueue; - public static final int CommandOkay = 200; - - // The range of 400 -> 599 is reserved for cmd failures - public static final int OperationFailed = 400; - public static final int CommandSyntaxError = 500; - public static final int CommandParameterError = 501; + private INativeDaemonConnectorCallbacks mCallbacks; + private Handler mCallbackHandler; - public static final int UnsolicitedInformational = 600; + /** Lock held whenever communicating with native daemon. */ + private final Object mDaemonLock = new Object(); - // - public static final int FailedRangeStart = 400; - public static final int FailedRangeEnd = 599; - } + private final int BUFFER_SIZE = 4096; - NativeDaemonConnector(INativeDaemonConnectorCallbacks callbacks, - String socket, int responseQueueSize, String logTag) { + NativeDaemonConnector(INativeDaemonConnectorCallbacks callbacks, String socket, + int responseQueueSize, String logTag) { mCallbacks = callbacks; - if (logTag != null) - TAG = logTag; mSocket = socket; - mResponseQueue = new LinkedBlockingQueue<String>(responseQueueSize); + mResponseQueue = new LinkedBlockingQueue<NativeDaemonEvent>(responseQueueSize); + TAG = logTag != null ? logTag : "NativeDaemonConnector"; } @Override @@ -136,26 +122,26 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo for (int i = 0; i < count; i++) { if (buffer[i] == 0) { - String event = new String(buffer, start, i - start); - if (LOCAL_LOGD) Slog.d(TAG, String.format("RCV <- {%s}", event)); + final String rawEvent = new String(buffer, start, i - start); + if (LOGD) Slog.d(TAG, "RCV <- " + rawEvent); - String[] tokens = event.split(" ", 2); try { - int code = Integer.parseInt(tokens[0]); - - if (code >= ResponseCode.UnsolicitedInformational) { - mCallbackHandler.sendMessage( - mCallbackHandler.obtainMessage(code, event)); + final NativeDaemonEvent event = NativeDaemonEvent.parseRawEvent( + rawEvent); + if (event.isClassUnsolicited()) { + mCallbackHandler.sendMessage(mCallbackHandler.obtainMessage( + event.getCode(), event.getRawEvent())); } else { try { mResponseQueue.put(event); } catch (InterruptedException ex) { - Slog.e(TAG, "Failed to put response onto queue", ex); + Slog.e(TAG, "Failed to put response onto queue: " + ex); } } - } catch (NumberFormatException nfe) { - Slog.w(TAG, String.format("Bad msg (%s)", event)); + } catch (IllegalArgumentException e) { + Slog.w(TAG, "Problem parsing message: " + rawEvent, e); } + start = i + 1; } } @@ -195,133 +181,174 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo } } - private void sendCommandLocked(String command) throws NativeDaemonConnectorException { - sendCommandLocked(command, null); - } - /** - * Sends a command to the daemon with a single argument + * Send command to daemon, escaping arguments as needed. * - * @param command The command to send to the daemon - * @param argument The argument to send with the command (or null) + * @return the final command issued. */ - private void sendCommandLocked(String command, String argument) + private String sendCommandLocked(String cmd, Object... args) throws NativeDaemonConnectorException { - if (command != null && command.indexOf('\0') >= 0) { - throw new IllegalArgumentException("unexpected command: " + command); + // TODO: eventually enforce that cmd doesn't contain arguments + if (cmd.indexOf('\0') >= 0) { + throw new IllegalArgumentException("unexpected command: " + cmd); } - if (argument != null && argument.indexOf('\0') >= 0) { - throw new IllegalArgumentException("unexpected argument: " + argument); + + final StringBuilder builder = new StringBuilder(cmd); + for (Object arg : args) { + final String argString = String.valueOf(arg); + if (argString.indexOf('\0') >= 0) { + throw new IllegalArgumentException("unexpected argument: " + arg); + } + + builder.append(' '); + appendEscaped(builder, argString); } - if (LOCAL_LOGD) Slog.d(TAG, String.format("SND -> {%s} {%s}", command, argument)); + final String unterminated = builder.toString(); + if (LOGD) Slog.d(TAG, "SND -> " + unterminated); + + builder.append('\0'); + if (mOutputStream == null) { - Slog.e(TAG, "No connection to daemon", new IllegalStateException()); - throw new NativeDaemonConnectorException("No output stream!"); + throw new NativeDaemonConnectorException("missing output stream"); } else { - StringBuilder builder = new StringBuilder(command); - if (argument != null) { - builder.append(argument); - } - builder.append('\0'); - try { mOutputStream.write(builder.toString().getBytes()); - } catch (IOException ex) { - Slog.e(TAG, "IOException in sendCommand", ex); + } catch (IOException e) { + throw new NativeDaemonConnectorException("problem sending command", e); } } + + return unterminated; } /** - * Issue a command to the native daemon and return the responses + * Issue a command to the native daemon and return the responses. */ - public ArrayList<String> doCommand(String cmd) throws NativeDaemonConnectorException { + public NativeDaemonEvent[] execute(String cmd, Object... args) + throws NativeDaemonConnectorException { synchronized (mDaemonLock) { - return doCommandLocked(cmd); + return executeLocked(cmd, args); } } - private ArrayList<String> doCommandLocked(String cmd) throws NativeDaemonConnectorException { + private NativeDaemonEvent[] executeLocked(String cmd, Object... args) + throws NativeDaemonConnectorException { + final ArrayList<NativeDaemonEvent> events = Lists.newArrayList(); + mResponseQueue.clear(); - sendCommandLocked(cmd); - ArrayList<String> response = new ArrayList<String>(); - boolean complete = false; - int code = -1; + final String sentCommand = sendCommandLocked(cmd, args); - while (!complete) { + NativeDaemonEvent event = null; + do { try { - // TODO - this should not block forever - String line = mResponseQueue.take(); - if (LOCAL_LOGD) Slog.d(TAG, String.format("RSP <- {%s}", line)); - String[] tokens = line.split(" "); - try { - code = Integer.parseInt(tokens[0]); - } catch (NumberFormatException nfe) { - throw new NativeDaemonConnectorException( - String.format("Invalid response from daemon (%s)", line)); - } - - if ((code >= 200) && (code < 600)) { - complete = true; - } - response.add(line); - } catch (InterruptedException ex) { - Slog.e(TAG, "Failed to process response", ex); + event = mResponseQueue.take(); + } catch (InterruptedException e) { + Slog.w(TAG, "interrupted waiting for event line"); + continue; } + events.add(event); + } while (event.isClassContinue()); + + if (event.isClassClientError()) { + throw new NativeDaemonArgumentException(sentCommand, event); + } + if (event.isClassServerError()) { + throw new NativeDaemonFailureException(sentCommand, event); } - if (code >= ResponseCode.FailedRangeStart && - code <= ResponseCode.FailedRangeEnd) { - /* - * Note: The format of the last response in this case is - * "NNN <errmsg>" - */ - throw new NativeDaemonConnectorException( - code, cmd, response.get(response.size()-1).substring(4)); + return events.toArray(new NativeDaemonEvent[events.size()]); + } + + /** + * Issue a command to the native daemon and return the raw responses. + * + * @deprecated callers should move to {@link #execute(String, Object...)} + * which returns parsed {@link NativeDaemonEvent}. + */ + @Deprecated + public ArrayList<String> doCommand(String cmd) throws NativeDaemonConnectorException { + final ArrayList<String> rawEvents = Lists.newArrayList(); + final NativeDaemonEvent[] events = execute(cmd); + for (NativeDaemonEvent event : events) { + rawEvents.add(event.getRawEvent()); } - return response; + return rawEvents; } /** - * Issues a list command and returns the cooked list + * Issues a list command and returns the cooked list of all + * {@link NativeDaemonEvent#getMessage()} which match requested code. */ - public String[] doListCommand(String cmd, int expectedResponseCode) + public String[] doListCommand(String cmd, int expectedCode) throws NativeDaemonConnectorException { + final ArrayList<String> list = Lists.newArrayList(); + + final NativeDaemonEvent[] events = execute(cmd); + for (int i = 0; i < events.length - 1; i++) { + final NativeDaemonEvent event = events[i]; + final int code = event.getCode(); + if (code == expectedCode) { + list.add(event.getMessage()); + } else { + throw new NativeDaemonConnectorException( + "unexpected list response " + code + " instead of " + expectedCode); + } + } - ArrayList<String> rsp = doCommand(cmd); - String[] rdata = new String[rsp.size()-1]; - int idx = 0; + final NativeDaemonEvent finalEvent = events[events.length - 1]; + if (!finalEvent.isClassOk()) { + throw new NativeDaemonConnectorException("unexpected final event: " + finalEvent); + } - for (int i = 0; i < rsp.size(); i++) { - String line = rsp.get(i); - try { - String[] tok = line.split(" "); - int code = Integer.parseInt(tok[0]); - if (code == expectedResponseCode) { - rdata[idx++] = line.substring(tok[0].length() + 1); - } else if (code == NativeDaemonConnector.ResponseCode.CommandOkay) { - if (LOCAL_LOGD) Slog.d(TAG, String.format("List terminated with {%s}", line)); - int last = rsp.size() -1; - if (i != last) { - Slog.w(TAG, String.format("Recv'd %d lines after end of list {%s}", (last-i), cmd)); - for (int j = i; j <= last ; j++) { - Slog.w(TAG, String.format("ExtraData <%s>", rsp.get(i))); - } - } - return rdata; - } else { - throw new NativeDaemonConnectorException( - String.format("Expected list response %d, but got %d", - expectedResponseCode, code)); - } - } catch (NumberFormatException nfe) { - throw new NativeDaemonConnectorException( - String.format("Error reading code '%s'", line)); + return list.toArray(new String[list.size()]); + } + + /** + * Append the given argument to {@link StringBuilder}, escaping as needed, + * and surrounding with quotes when it contains spaces. + */ + // @VisibleForTesting + static void appendEscaped(StringBuilder builder, String arg) { + final boolean hasSpaces = arg.indexOf(' ') >= 0; + if (hasSpaces) { + builder.append('"'); + } + + final int length = arg.length(); + for (int i = 0; i < length; i++) { + final char c = arg.charAt(i); + + if (c == '"') { + builder.append("\\\""); + } else if (c == '\\') { + builder.append("\\\\"); + } else { + builder.append(c); } } - throw new NativeDaemonConnectorException("Got an empty response"); + + if (hasSpaces) { + builder.append('"'); + } + } + + private static class NativeDaemonArgumentException extends NativeDaemonConnectorException { + public NativeDaemonArgumentException(String command, NativeDaemonEvent event) { + super(command, event); + } + + @Override + public IllegalArgumentException rethrowAsParcelableException() { + throw new IllegalArgumentException(getMessage(), this); + } + } + + private static class NativeDaemonFailureException extends NativeDaemonConnectorException { + public NativeDaemonFailureException(String command, NativeDaemonEvent event) { + super(command, event); + } } /** {@inheritDoc} */ diff --git a/services/java/com/android/server/NativeDaemonConnectorException.java b/services/java/com/android/server/NativeDaemonConnectorException.java index 426742b..590bbcc 100644 --- a/services/java/com/android/server/NativeDaemonConnectorException.java +++ b/services/java/com/android/server/NativeDaemonConnectorException.java @@ -16,33 +16,43 @@ package com.android.server; +import android.os.Parcel; + /** - * An exception that indicates there was an error with a NativeDaemonConnector operation + * An exception that indicates there was an error with a + * {@link NativeDaemonConnector} operation. */ -public class NativeDaemonConnectorException extends RuntimeException -{ - private int mCode = -1; +public class NativeDaemonConnectorException extends Exception { private String mCmd; + private NativeDaemonEvent mEvent; - public NativeDaemonConnectorException() {} + public NativeDaemonConnectorException(String detailMessage) { + super(detailMessage); + } - public NativeDaemonConnectorException(String error) - { - super(error); + public NativeDaemonConnectorException(String detailMessage, Throwable throwable) { + super(detailMessage, throwable); } - public NativeDaemonConnectorException(int code, String cmd, String error) - { - super(String.format("Cmd {%s} failed with code %d : {%s}", cmd, code, error)); - mCode = code; + public NativeDaemonConnectorException(String cmd, NativeDaemonEvent event) { + super("command '" + cmd + "' failed with '" + event + "'"); mCmd = cmd; + mEvent = event; } public int getCode() { - return mCode; + return mEvent.getCode(); } public String getCmd() { return mCmd; } + + /** + * Rethrow as a {@link RuntimeException} subclass that is handled by + * {@link Parcel#writeException(Exception)}. + */ + public IllegalArgumentException rethrowAsParcelableException() { + throw new IllegalStateException(getMessage(), this); + } } diff --git a/services/java/com/android/server/NativeDaemonEvent.java b/services/java/com/android/server/NativeDaemonEvent.java new file mode 100644 index 0000000..b1d0788 --- /dev/null +++ b/services/java/com/android/server/NativeDaemonEvent.java @@ -0,0 +1,113 @@ +/* + * Copyright (C) 2011 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server; + +/** + * Parsed event from native side of {@link NativeDaemonConnector}. + */ +public class NativeDaemonEvent { + + // TODO: keep class ranges in sync with ResponseCode.h + // TODO: swap client and server error ranges to roughly mirror HTTP spec + + private final int mCode; + private final String mMessage; + private final String mRawEvent; + + private NativeDaemonEvent(int code, String message, String rawEvent) { + mCode = code; + mMessage = message; + mRawEvent = rawEvent; + } + + public int getCode() { + return mCode; + } + + public String getMessage() { + return mMessage; + } + + @Deprecated + public String getRawEvent() { + return mRawEvent; + } + + @Override + public String toString() { + return mRawEvent; + } + + /** + * Test if event represents a partial response which is continued in + * additional subsequent events. + */ + public boolean isClassContinue() { + return mCode >= 100 && mCode < 200; + } + + /** + * Test if event represents a command success. + */ + public boolean isClassOk() { + return mCode >= 200 && mCode < 300; + } + + /** + * Test if event represents a remote native daemon error. + */ + public boolean isClassServerError() { + return mCode >= 400 && mCode < 500; + } + + /** + * Test if event represents a command syntax or argument error. + */ + public boolean isClassClientError() { + return mCode >= 500 && mCode < 600; + } + + /** + * Test if event represents an unsolicited event from native daemon. + */ + public boolean isClassUnsolicited() { + return mCode >= 600 && mCode < 700; + } + + /** + * Parse the given raw event into {@link NativeDaemonEvent} instance. + * + * @throws IllegalArgumentException when line doesn't match format expected + * from native side. + */ + public static NativeDaemonEvent parseRawEvent(String rawEvent) { + final int splitIndex = rawEvent.indexOf(' '); + if (splitIndex == -1) { + throw new IllegalArgumentException("unable to find ' ' separator"); + } + + final int code; + try { + code = Integer.parseInt(rawEvent.substring(0, splitIndex)); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("problem parsing code", e); + } + + final String message = rawEvent.substring(splitIndex + 1); + return new NativeDaemonEvent(code, message, rawEvent); + } +} diff --git a/services/java/com/android/server/NetworkManagementService.java b/services/java/com/android/server/NetworkManagementService.java index 9d808e1..e97f719 100644 --- a/services/java/com/android/server/NetworkManagementService.java +++ b/services/java/com/android/server/NetworkManagementService.java @@ -442,27 +442,17 @@ public class NetworkManagementService extends INetworkManagementService.Stub @Override public void setInterfaceDown(String iface) { mContext.enforceCallingOrSelfPermission(CHANGE_NETWORK_STATE, TAG); - try { - InterfaceConfiguration ifcg = getInterfaceConfig(iface); - ifcg.interfaceFlags = ifcg.interfaceFlags.replace("up", "down"); - setInterfaceConfig(iface, ifcg); - } catch (NativeDaemonConnectorException e) { - throw new IllegalStateException( - "Unable to communicate with native daemon for interface down - " + e); - } + final InterfaceConfiguration ifcg = getInterfaceConfig(iface); + ifcg.interfaceFlags = ifcg.interfaceFlags.replace("up", "down"); + setInterfaceConfig(iface, ifcg); } @Override public void setInterfaceUp(String iface) { mContext.enforceCallingOrSelfPermission(CHANGE_NETWORK_STATE, TAG); - try { - InterfaceConfiguration ifcg = getInterfaceConfig(iface); - ifcg.interfaceFlags = ifcg.interfaceFlags.replace("down", "up"); - setInterfaceConfig(iface, ifcg); - } catch (NativeDaemonConnectorException e) { - throw new IllegalStateException( - "Unable to communicate with native daemon for interface up - " + e); - } + final InterfaceConfiguration ifcg = getInterfaceConfig(iface); + ifcg.interfaceFlags = ifcg.interfaceFlags.replace("down", "up"); + setInterfaceConfig(iface, ifcg); } @Override @@ -733,7 +723,11 @@ public class NetworkManagementService extends INetworkManagementService.Stub @Override public void setIpForwardingEnabled(boolean enable) { mContext.enforceCallingOrSelfPermission(CHANGE_NETWORK_STATE, TAG); - mConnector.doCommand(String.format("ipfwd %sable", (enable ? "en" : "dis"))); + try { + mConnector.doCommand(String.format("ipfwd %sable", (enable ? "en" : "dis"))); + } catch (NativeDaemonConnectorException e) { + e.rethrowAsParcelableException(); + } } @Override @@ -875,7 +869,11 @@ public class NetworkManagementService extends INetworkManagementService.Stub } } - mConnector.doCommand(cmd); + try { + mConnector.doCommand(cmd); + } catch (NativeDaemonConnectorException e) { + e.rethrowAsParcelableException(); + } } @Override diff --git a/services/tests/servicestests/src/com/android/server/NativeDaemonConnectorTest.java b/services/tests/servicestests/src/com/android/server/NativeDaemonConnectorTest.java new file mode 100644 index 0000000..275d807 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/NativeDaemonConnectorTest.java @@ -0,0 +1,70 @@ +/* + * Copyright (C) 2011 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server; + +import static com.android.server.NativeDaemonConnector.appendEscaped; + +import android.test.AndroidTestCase; +import android.test.suitebuilder.annotation.MediumTest; + +/** + * Tests for {@link NativeDaemonConnector}. + */ +@MediumTest +public class NativeDaemonConnectorTest extends AndroidTestCase { + private static final String TAG = "NativeDaemonConnectorTest"; + + public void testArgumentNormal() throws Exception { + final StringBuilder builder = new StringBuilder(); + + builder.setLength(0); + appendEscaped(builder, ""); + assertEquals("", builder.toString()); + + builder.setLength(0); + appendEscaped(builder, "foo"); + assertEquals("foo", builder.toString()); + + builder.setLength(0); + appendEscaped(builder, "foo\"bar"); + assertEquals("foo\\\"bar", builder.toString()); + + builder.setLength(0); + appendEscaped(builder, "foo\\bar\\\"baz"); + assertEquals("foo\\\\bar\\\\\\\"baz", builder.toString()); + } + + public void testArgumentWithSpaces() throws Exception { + final StringBuilder builder = new StringBuilder(); + + builder.setLength(0); + appendEscaped(builder, "foo bar"); + assertEquals("\"foo bar\"", builder.toString()); + + builder.setLength(0); + appendEscaped(builder, "foo\"bar\\baz foo"); + assertEquals("\"foo\\\"bar\\\\baz foo\"", builder.toString()); + } + + public void testArgumentWithUtf() throws Exception { + final StringBuilder builder = new StringBuilder(); + + builder.setLength(0); + appendEscaped(builder, "caf\u00E9 c\u00F6ffee"); + assertEquals("\"caf\u00E9 c\u00F6ffee\"", builder.toString()); + } +} |