Merge "Handle invalid protolog message format more gracefully" into main (2ba3a051) · Commits · e / os / android_frameworks_base

core/java/com/android/internal/protolog/PerfettoProtoLogImpl.java

+8 −2

Original line number	Diff line number	Diff line
		@@ -66,6 +66,7 @@ import com.android.internal.protolog.IProtoLogConfigurationService.RegisterClien
		import com.android.internal.protolog.common.ILogger;
		import com.android.internal.protolog.common.IProtoLog;
		import com.android.internal.protolog.common.IProtoLogGroup;
		import com.android.internal.protolog.common.InvalidFormatStringException;
		import com.android.internal.protolog.common.LogDataType;
		import com.android.internal.protolog.common.LogLevel;

		@@ -207,7 +208,12 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen

		@Override
		public void log(LogLevel logLevel, IProtoLogGroup group, String messageString, Object... args) {
		try {
		log(logLevel, group, new Message(messageString), args);
		} catch (InvalidFormatStringException e) {
		Slog.e(LOG_TAG, "Invalid protolog string format", e);
		log(logLevel, group, new Message("INVALID MESSAGE"), new Object[0]);
		}
		}

		/**
		@@ -831,7 +837,7 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen
		this.mMessageString = null;
		}

		private Message(@NonNull String messageString) {
		private Message(@NonNull String messageString) throws InvalidFormatStringException {
		this.mMessageHash = null;
		final List<Integer> argTypes = LogDataType.parseFormatString(messageString);
		this.mMessageMask = LogDataType.logDataTypesToBitMask(argTypes);

core/java/com/android/internal/protolog/common/LogDataType.java

+2 −2

Original line number	Diff line number	Diff line
		@@ -86,8 +86,8 @@ public class LogDataType {
		case '%':
		break;
		default:
		throw new InvalidFormatStringException("Invalid format string field"
		+ " %${messageString[i + 1]}");
		throw new InvalidFormatStringException("Invalid Protolog message format in "
		+ "\"" + messageString + "\" at index " + i + ".");
		}
		i += 2;
		} else {

tools/protologtool/src/com/android/protolog/tool/exceptions.kt→tools/protologtool/src/com/android/protolog/tool/Exceptions.kt

+14 −11

Original line number	Diff line number	Diff line
		@@ -18,21 +18,24 @@ package com.android.protolog.tool

		import java.lang.Exception

		open class CodeProcessingException(message: String, context: ParsingContext)
		: Exception("Code processing error in ${context.filePath}:${context.lineNumber}:\n" +
		" $message")
		open class CodeProcessingException(
		message: String, context: ParsingContext, cause: Throwable? = null
		) : Exception("Code processing error in ${context.filePath}:${context.lineNumber}:\n" +
		" $message", cause)

		class HashCollisionException(message: String, context: ParsingContext) :
		CodeProcessingException(message, context)
		class HashCollisionException(
		message: String, context: ParsingContext, cause: Throwable? = null
		) : CodeProcessingException(message, context, cause)

		class IllegalImportException(message: String, context: ParsingContext) :
		CodeProcessingException("Illegal import: $message", context)
		class IllegalImportException(message: String, context: ParsingContext, cause: Throwable? = null) :
		CodeProcessingException("Illegal import: $message", context, cause)

		class InvalidProtoLogCallException(message: String, context: ParsingContext)
		: CodeProcessingException("InvalidProtoLogCall: $message", context)
		class InvalidProtoLogCallException(
		message: String, context: ParsingContext, cause: Throwable? = null
		) : CodeProcessingException("InvalidProtoLogCall: $message", context, cause)

		class ParsingException(message: String, context: ParsingContext)
		: CodeProcessingException(message, context)
		class ParsingException(message: String, context: ParsingContext, cause: Throwable? = null) :
		CodeProcessingException(message, context, cause)

		class InvalidViewerConfigException(message: String) : Exception(message)

tools/protologtool/src/com/android/protolog/tool/ProtoLogCallProcessorImpl.kt

+11 −3

Original line number	Diff line number	Diff line
		@@ -118,9 +118,17 @@ class ProtoLogCallProcessorImpl(
		"- not a ProtoLogGroup enum member: $call", context)
		}

		logCallVisitor?.processCall(call, messageString, getLevelForMethodName(
		call.name.toString(), call, context), groupMap.getValue(groupName),
		context.lineNumber)
		try {
		logCallVisitor?.processCall(
		call, messageString, getLevelForMethodName(
		call.name.toString(), call, context
		), groupMap.getValue(groupName),
		context.lineNumber
		)
		} catch (e: Throwable) {
		throw InvalidProtoLogCallException("Error processing log call: $call",
		context, e)
		}
		} else if (call.name.id == "init") {
		// No processing
		} else {

tools/protologtool/tests/com/android/protolog/tool/ProtoLogCallProcessorImplTest.kt

+37 −0

Original line number	Diff line number	Diff line
		@@ -16,11 +16,14 @@

		package com.android.protolog.tool

		import com.android.internal.protolog.common.InvalidFormatStringException
		import com.android.internal.protolog.common.LogLevel
		import com.github.javaparser.StaticJavaParser
		import com.github.javaparser.ast.expr.MethodCallExpr
		import org.junit.Assert.assertEquals
		import org.junit.Assert.assertThrows
		import org.junit.Test
		import com.google.common.truth.Truth

		class ProtoLogCallProcessorImplTest {
		private data class LogCall(
		@@ -228,4 +231,38 @@ class ProtoLogCallProcessorImplTest {
		visitor.process(StaticJavaParser.parse(code), processor, "")
		checkCalls()
		}

		@Test
		fun throws_clear_error_message_on_invalid_format_exception() {
		val code = """
		package org.example;

		class Test {
		void test() {
		ProtoLog.d(ProtoLogGroup.TEST, "Invalid message %9 %");
		}
		}
		"""
		groupMap["TEST"] = LogGroup("TEST", false, true, "WindowManager")

		val processor = object : ProtoLogCallVisitor {
		override fun processCall(
		call: MethodCallExpr,
		messageString: String,
		level: LogLevel,
		group: LogGroup,
		lineNumber: Int,
		) {
		throw InvalidFormatStringException("Invalid Protolog message format")
		}
		}

		val exception = assertThrows(InvalidProtoLogCallException::class.java) {
		visitor.process(StaticJavaParser.parse(code), processor, "MyTestFile.java")
		}
		Truth.assertThat(exception).hasMessageThat()
		.contains("Code processing error in MyTestFile.java:6")
		Truth.assertThat(exception.cause).hasMessageThat()
		.contains("Invalid Protolog message format")
		}
		}