You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2022/11/11 19:22:39 UTC

[GitHub] [sling-org-apache-sling-tooling-support-install] github-code-scanning[bot] commented on a diff in pull request #4: SLING-11672 Migrate from Felix HTTP Whiteboard to OSGi Whiteboard

github-code-scanning[bot] commented on code in PR #4:
URL: https://github.com/apache/sling-org-apache-sling-tooling-support-install/pull/4#discussion_r1020503172


##########
src/main/java/org/apache/sling/tooling/support/install/impl/InstallServlet.java:
##########
@@ -262,49 +235,45 @@
         return found;
     }
 
-    private static void createJar(final File sourceDir, final File jarFile, final Manifest mf)
-    throws IOException {
-        final JarOutputStream zos = new JarOutputStream(new FileOutputStream(jarFile));
-        try {
+    private static void createJar(final Path sourceDir, final Path jarFile, final Manifest mf) throws IOException {
+        try (JarOutputStream zos = new JarOutputStream(Files.newOutputStream(jarFile))) {
             zos.setLevel(Deflater.NO_COMPRESSION);
             // manifest first
             final ZipEntry anEntry = new ZipEntry(JarFile.MANIFEST_NAME);
             zos.putNextEntry(anEntry);
             mf.write(zos);
             zos.closeEntry();
             zipDir(sourceDir, zos, "");
-        } finally {
-            try {
-                zos.close();
-            } catch ( final IOException ignore ) {
-                // ignore
-            }
         }
     }
 
-    public static void zipDir(final File sourceDir, final ZipOutputStream zos, final String path)
-    throws IOException {
-        final byte[] readBuffer = new byte[8192];
-        int bytesIn = 0;
+    public static void zipDir(final Path sourceDir, final ZipOutputStream zos, final String prefix) throws IOException {
+        try (Stream<Path> stream = Files.list(sourceDir)) {

Review Comment:
   ## I/O function calls should not be vulnerable to path injection attacks
   
   <!--SONAR_ISSUE_KEY:AYRoJJXWSHsDn9lThPYU-->Change this code to not construct the path from user-controlled data. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-tooling-support-install&issues=AYRoJJXWSHsDn9lThPYU&open=AYRoJJXWSHsDn9lThPYU&pullRequest=4">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/sling-org-apache-sling-tooling-support-install/security/code-scanning/10)



##########
src/main/java/org/apache/sling/tooling/support/install/impl/InstallServlet.java:
##########
@@ -96,77 +93,57 @@
             result.render(resp.getWriter());
             return;
         }
-
         if (isMultipart) {
-            installBasedOnUploadedJar(req, resp);
+            installBasedOnUploadedJar(req, resp, refreshPackages);
         } else {
-            installBasedOnDirectory(resp, new File(dirPath));
+            installBasedOnDirectory(resp, Paths.get(dirPath), refreshPackages);

Review Comment:
   ## Exceptions should not be thrown from servlet methods
   
   <!--SONAR_ISSUE_KEY:AYRoJJXWSHsDn9lThPYR-->Handle the following exception that could be thrown by "installBasedOnDirectory": IOException. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-tooling-support-install&issues=AYRoJJXWSHsDn9lThPYR&open=AYRoJJXWSHsDn9lThPYR&pullRequest=4">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/sling-org-apache-sling-tooling-support-install/security/code-scanning/12)



##########
src/main/java/org/apache/sling/tooling/support/install/impl/InstallServlet.java:
##########
@@ -96,77 +93,57 @@
             result.render(resp.getWriter());
             return;
         }
-
         if (isMultipart) {
-            installBasedOnUploadedJar(req, resp);
+            installBasedOnUploadedJar(req, resp, refreshPackages);

Review Comment:
   ## Exceptions should not be thrown from servlet methods
   
   <!--SONAR_ISSUE_KEY:AYRoJJXWSHsDn9lThPYQ-->Handle the following exceptions that could be thrown by "installBasedOnUploadedJar": IOException, ServletException. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-tooling-support-install&issues=AYRoJJXWSHsDn9lThPYQ&open=AYRoJJXWSHsDn9lThPYQ&pullRequest=4">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/sling-org-apache-sling-tooling-support-install/security/code-scanning/13)



##########
src/main/java/org/apache/sling/tooling/support/install/impl/InstallServlet.java:
##########
@@ -182,49 +159,39 @@
         new InstallationResult(false, message + " : " + e.getMessage()).render(resp.getWriter());
     }
 
-    private void installBasedOnDirectory(HttpServletResponse resp, final File dir) throws FileNotFoundException,
-            IOException {
-
+    private void installBasedOnDirectory(HttpServletResponse resp, final Path dir, boolean refreshPackages) throws IOException {
         InstallationResult result = null;
 
-        if ( dir.exists() && dir.isDirectory() ) {
+        if (Files.isDirectory(dir)) {
             logger.info("Checking dir {} for bundle install", dir);
-            final File manifestFile = new File(dir, JarFile.MANIFEST_NAME);
-            if ( manifestFile.exists() ) {
-                FileInputStream fis = null;
-                try {
-                    fis = new FileInputStream(manifestFile);
+            final Path manifestFile = dir.resolve(JarFile.MANIFEST_NAME);
+            if (Files.exists(dir)) {
+                try (InputStream fis = Files.newInputStream(manifestFile)) {

Review Comment:
   ## I/O function calls should not be vulnerable to path injection attacks
   
   <!--SONAR_ISSUE_KEY:AYRoJJXWSHsDn9lThPYT-->Change this code to not construct the path from user-controlled data. <p>See more on <a href="https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-tooling-support-install&issues=AYRoJJXWSHsDn9lThPYT&open=AYRoJJXWSHsDn9lThPYT&pullRequest=4">SonarCloud</a></p>
   
   [Show more details](https://github.com/apache/sling-org-apache-sling-tooling-support-install/security/code-scanning/11)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org