Spotbugs + Checkstyle config TODO

[lesv]

OPM_OVERLY_PERMISSIVE_METHOD

[ace-n]

Checkstyle config additions

Likely useful FinalLocalVariableCheck ConstantName LocalVariableName // should check case of logger and gson (no LOGGER or GSON)

Maybe useful NewlineAtEndOfFile AvoidEscapedUnicodeCharacters ? AvoidNestedBlocks ?

Useful, but more work ban JSON strings via regex...?

• Example regex: "({|\s)+('|\\").*}"
• Use gson (and Map.of(), if possible) instead!

[lesv]

@kurtisvg list of suppression recommendations:

• SE_NO_SERIALVERSIONID
• SERVLET_PARAMETER
• WEM_WEAK_EXCEPTION_MESSAGING

Debatable:

• IMC_IMMATURE_CLASS_NO_TOSTRING

False positives:

• ODR_OPEN_DATABASE_RESOURCE
• XSS_SERVLET
• HARD_CODE_PASSWORD

[ace-n]

What I whitelisted globally for the functions snippets:

• CRLF_INJECTION_LOGS
• CLI_CONSTANT_LIST_INDEX
• LI_LAZY_INIT_STATIC
• PCOA_PARTIALLY_CONSTRUCTED_OBJECT_ACCESS
• RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE

Also - make sure we have access to a spotbugs.xml file in each repo for more fine-grained (e.g. per-file) whitelisting.

[lesv]

@ace-n created a great list in the referenced PR.

<?xml version="1.0" encoding="UTF-8"?>
 <FindBugsFilter
              xmlns="https://github.com/spotbugs/filter/3.0.0"
              xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
              xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0 https://raw.githubusercontent.com/spotbugs/spotbugs/3.1.0/spotbugs/etc/findbugsfilter.xsd">

    <Match> <!-- Global excludes -->
        <Or>
            <Bug pattern="IMC_IMMATURE_CLASS_NO_TOSTRING" />
            <Bug pattern="CRLF_INJECTION_LOGS" />
            <Bug pattern="CLI_CONSTANT_LIST_INDEX" />
            <Bug pattern="LI_LAZY_INIT_STATIC" />
            <Bug pattern="PCOA_PARTIALLY_CONSTRUCTED_OBJECT_ACCESS" />
            <Bug pattern="RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE" />
        </Or>
    </Match>

    <!-- Functions samples -->
    <Match>
        <Or>
            <Class name="com.example.functions.helloworld.GroovyHelloWorld" />
            <Class name="com.example.functions.helloworld.GroovyHelloBackground" />
        </Or>
        <Bug pattern="MS_SHOULD_BE_FINAL" />
    </Match>

    <Match>
        <Or>
            <Class name="com.example.functions.helloworld.HelloError" />
            <Class name="com.example.functions.concepts.RetryPubSub" />
            <Class name="com.example.functions.ocr.OcrProcessImage" />
            <Class name="com.example.functions.ocr.OcrTranslateApiMessage" />
        </Or>
        <Bug pattern="WEM_WEAK_EXCEPTION_MESSAGING" />
    </Match>

    <Match>
        <Class name="com.example.functions.helloworld.KotlinHelloBackground" />
        <Bug pattern="DLS_DEAD_LOCAL_STORE" />
    </Match>

    <Match>
        <Class name="com.example.functions.imagemagick.ImageMagick" />
        <Or>
            <Bug pattern="PATH_TRAVERSAL_IN" />
            <Bug pattern="DMI_HARDCODED_ABSOLUTE_FILENAME" />
            <Bug pattern="COMMAND_INJECTION" />
        </Or>
    </Match>

    <Match>
        <Class name="com.example.functions.concepts.LazyFields" />
        <Bug pattern="UPM_UNCALLED_PRIVATE_METHOD" />
    </Match>

    <Match>
        <Class name="com.example.functions.concepts.FileSystem" />
        <Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE" />
    </Match>

    <!-- TODO(ace-n): switch Date objects in eventpojos to java.time ones -->
    <Match>
        <Or>
            <Bug pattern="EI_EXPOSE_REP" />
            <Bug pattern="EI_EXPOSE_REP2" />
        </Or>
    </Match>

</FindBugsFilter>

[lesv]

• <Bug pattern="IMC_IMMATURE_CLASS_NO_TOSTRING" /> <!-- Let me skip toString() --> seems Ok.
• <Bug pattern="CRLF_INJECTION_LOGS" /> <!-- Let me inject /n/r into logs --> Not sure why this is wanted.
• <Bug pattern="CLI_CONSTANT_LIST_INDEX" /> <!-- I like using constants in my arrays --> We do this a lot.
• <Bug pattern="LI_LAZY_INIT_STATIC" /> lesv: I don't think this is a good idea.
• <Bug pattern="PCOA_PARTIALLY_CONSTRUCTED_OBJECT_ACCESS" /> This appears to have been fixed - so Not adding. It's a very bad thing to do. If you run into this ask one of the reviewers for help.
• <Bug pattern="RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE" /> Appears to have been fixed.

[lesv]

Should be fixed in the next PR

[ace-n]

Off the top of my head, I think the CRLF_INJECTION_LOGS occurs when logging parameter values.