Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve/Suppress remaining Cloud Manager Code Scanning Blocker/Criticals as of 2022-01-18 #2772

Closed
3 tasks done
adamcin opened this issue Jan 18, 2022 · 7 comments
Closed
3 tasks done
Assignees

Comments

@adamcin
Copy link
Contributor

adamcin commented Jan 18, 2022

Required Information

  • AEM Version, including Service Packs, Cumulative Fix Packs, etc: Cloud Service
  • ACS AEM Commons Version: <=5.1.0
  • Reproducible on Latest? yes

Expected Behavior

Customers deploying ACS AEM Commons should not have override Blocker/Critical issues related to as-is code.

Actual Behavior

After merge of #2771 , some remaining Blocker/Critical Java issues are still flagged in CM:

Creation Date File Location Line Number Issue Type Severity Effort Rule Tags Documentation
2022-01-18T18:14:40.933Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/packaging/impl/AbstractPackagerServlet.java 115 Use try-with-resources or close this "JcrPackage" in a "finally" clause. Bug Blocker 5min squid:S2095 cert,cwe,denial-of-service,leak https://www.adobe.com/go/aem_cmcq_s2095_en
2022-01-18T18:14:40.933Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/packaging/impl/PackageHelperImpl.java 136 Use try-with-resources or close this "JcrPackage" in a "finally" clause. Bug Blocker 5min squid:S2095 cert,cwe,denial-of-service,leak https://www.adobe.com/go/aem_cmcq_s2095_en
2022-01-18T18:14:40.934Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/packaging/impl/PackageHelperImpl.java 215 Use try-with-resources or close this "JcrPackage" in a "finally" clause. Bug Blocker 5min squid:S2095 cert,cwe,denial-of-service,leak https://www.adobe.com/go/aem_cmcq_s2095_en
2022-01-18T18:14:40.934Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/packaging/impl/PackageHelperImpl.java 282 Use try-with-resources or close this "JcrPackage" in a "finally" clause. Bug Blocker 5min squid:S2095 cert,cwe,denial-of-service,leak https://www.adobe.com/go/aem_cmcq_s2095_en
2022-01-18T18:14:40.934Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/replication/packages/automatic/impl/AutomaticPackageReplicatorJob.java 80 Use try-with-resources or close this "JcrPackage" in a "finally" clause. Bug Blocker 5min squid:S2095 cert,cwe,denial-of-service,leak https://www.adobe.com/go/aem_cmcq_s2095_en
2022-01-18T18:14:41.509Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/marketo/client/impl/MarketoClientImpl.java 143 HttpClient instances should always have socket and connect timeouts Bug Critical 15min CQRules:ConnectionTimeoutMechanism   https://www.adobe.com/go/aem_cmcq_connectiontimeoutmec_en
2022-01-18T18:14:41.51Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/oak/impl/EnsureOakIndexServlet.java 60 Remove this misleading mutable servlet instance field or make it "static" and/or "final" Bug Critical 30min squid:S2226 cert,multi-threading,struts https://www.adobe.com/go/aem_cmcq_s2226_en
2022-01-18T18:14:41.512Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/util/ResourceServiceManager.java 87 Inconsistent synchronization of com.adobe.acs.commons.util.ResourceServiceManager.bctx; locked 66% of time Bug Critical 1h findbugs:IS2_INCONSISTENT_SYNC multi-threading https://www.adobe.com/go/aem_cmcq_is2_inconsistent_syn_en
2022-01-18T18:14:41.513Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/util/impl/DiscoveryServiceHelper.java 61 Inconsistent synchronization of com.adobe.acs.commons.util.impl.DiscoveryServiceHelper.bundleContext; locked 50% of time Bug Critical 1h findbugs:IS2_INCONSISTENT_SYNC multi-threading https://www.adobe.com/go/aem_cmcq_is2_inconsistent_syn_en
2022-01-18T18:14:41.51Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/mcp/impl/processes/renovator/MovingPage.java 86 Add a default case to this switch. Code Smell Critical 5min squid:SwitchLastCaseIsDefaultCheck cert,cwe,misra https://www.adobe.com/go/aem_cmcq_switchlastcaseisdefa_en
2022-01-18T18:14:41.51Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/redirectmaps/impl/AddEntryServlet.java 50 Make "requireAem" transient or serializable. Code Smell Critical 30min squid:S1948 cwe,serialization https://www.adobe.com/go/aem_cmcq_s1948_en
2022-01-18T18:14:41.511Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/redirectmaps/impl/RedirectEntriesServlet.java 45 Make "requireAem" transient or serializable. Code Smell Critical 30min squid:S1948 cwe,serialization https://www.adobe.com/go/aem_cmcq_s1948_en
2022-01-18T18:14:41.511Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/redirectmaps/impl/RedirectMapServlet.java 52 Make "requireAem" transient or serializable. Code Smell Critical 30min squid:S1948 cwe,serialization https://www.adobe.com/go/aem_cmcq_s1948_en
2022-01-18T18:14:41.511Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/redirectmaps/impl/RemoveEntryServlet.java 50 Make "requireAem" transient or serializable. Code Smell Critical 30min squid:S1948 cwe,serialization https://www.adobe.com/go/aem_cmcq_s1948_en
2022-01-18T18:14:41.512Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/redirectmaps/impl/UpdateEntryServlet.java 50 Make "requireAem" transient or serializable. Code Smell Critical 30min squid:S1948 cwe,serialization https://www.adobe.com/go/aem_cmcq_s1948_en
2022-01-18T18:14:41.512Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/redirects/servlets/CreateRedirectConfigurationServlet.java 65 Make "redirectFilter" transient or serializable. Code Smell Critical 30min squid:S1948 cwe,serialization https://www.adobe.com/go/aem_cmcq_s1948_en
2022-01-18T18:14:41.516Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/wcm/impl/RobotsServlet.java 85 Make "rules" transient or serializable. Code Smell Critical 30min squid:S1948 cwe,serialization https://www.adobe.com/go/aem_cmcq_s1948_en
2022-01-18T18:14:41.516Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/wcm/impl/RobotsServlet.java 88 Make "externalizer" transient or serializable. Code Smell Critical 30min squid:S1948 cwe,serialization https://www.adobe.com/go/aem_cmcq_s1948_en
2022-01-18T18:14:41.516Z com.adobe.acs:acs-aem-commons-bundle:src/main/java/com/adobe/acs/commons/wcm/impl/RobotsServlet.java 91 Make "pageManagerFactory" transient or serializable. Code Smell Critical 30min squid:S1948 cwe,serialization https://www.adobe.com/go/aem_cmcq_s1948_en

Steps to Reproduce

Push affected tip to a CM repo and trigger a Code Quality pipeline.

@davidjgonzalez
Copy link
Contributor

@adamcin you want help w/ any of these? and/or want me to assign to you?

Once these are finished we can cut a release and clear out the CM speedbump.

@adamcin
Copy link
Contributor Author

adamcin commented Jan 18, 2022

@davidjgonzalez Yep, I'm running through them in our sandbox.

@adamcin
Copy link
Contributor Author

adamcin commented Jan 19, 2022

@davidjgonzalez I have a passing Code Quality check with 2 commits on top of PR #2771 . I'm afraid to push them to the PR branch because of the inevitably huge coverage requirement :)

image

@kwin
Copy link
Contributor

kwin commented Jan 19, 2022

The coverage requirement can be adjusted. This is not set in stone. @adamcin What metric value do you consider reasonable in .codecov.yml

adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Jan 19, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Jan 19, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Jan 19, 2022
@adamcin
Copy link
Contributor Author

adamcin commented Jan 19, 2022

I'll push the commits and just wait to see what happens to the PR patch coverage score.

@adamcin
Copy link
Contributor Author

adamcin commented Jan 19, 2022

image

@adamcin
Copy link
Contributor Author

adamcin commented Jan 19, 2022

@davidjgonzalez @kwin @joerghoh It looks like no coverage requirement adjustment is necessary. All checks in #2771 passed with the added commits, and the branch passes a CM Code Quality pipeline with no overrides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants