Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c90355f7 authored by Pablo Gamito's avatar Pablo Gamito
Browse files

Handle invalid protolog message format more gracefully 

Will now throw error with the following prefix: 'Code processing error in MyFile.java:123'

Bug: 403564102
Test: atest com.android.protolog.tool.ProtoLogCallProcessorImplTest
Flag: EXEMPT minor error handling change
Change-Id: Ifa7d566fae22d1bd2607a41e735113119bfbd7e0
parent 483768d2

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")

    }

}