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_r1022704382
##########
src/main/java/org/apache/sling/tooling/support/install/impl/InstallServlet.java:
##########
@@ -69,104 +70,95 @@
private static final String DIR = "dir";
- private static final int UPLOAD_IN_MEMORY_SIZE_THRESHOLD = 512 * 1024 *
1024;
+ public static final int UPLOAD_IN_MEMORY_SIZE_THRESHOLD = 512 * 1024 *
1024;
+ public static final int MANIFEST_SIZE_IN_INPUTSTREAM = 2 * 1024 * 1024;
- private BundleContext bundleContext;
-
- @Reference
- private PackageAdmin packageAdmin;
+ private final BundleContext bundleContext;
@Activate
- protected void activate(final BundleContext bundleContext) {
+ public InstallServlet(final BundleContext bundleContext) {
this.bundleContext = bundleContext;
}
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
final String dirPath = req.getParameter(DIR);
+ final boolean refreshPackages =
Boolean.parseBoolean(req.getParameter(dirPath));
+ boolean isMultipart = req.getContentType() != null &&
req.getContentType().toLowerCase().indexOf("multipart/form-data") > -1;
+ try {
+ if (dirPath == null && !isMultipart) {
+ logger.error("No dir parameter specified : {} and no multipart
content found", req.getParameterMap());
+ resp.setStatus(500);
+ InstallationResult result = new InstallationResult(false, "No
dir parameter specified: "
+ + req.getParameterMap() + " and no multipart content
found");
+ result.render(resp.getWriter());
+ return;
+ }
+ if (isMultipart) {
+ installBundleFromJar(req, resp, refreshPackages);
+ } else {
+ installBundleFromDirectory(resp, Paths.get(dirPath),
refreshPackages);
+ }
+ } catch (IOException e) {
+ throw new ServletException(e);
Review Comment:
## Exceptions should not be thrown from servlet methods
<!--SONAR_ISSUE_KEY:AYR7J6LgSRd6ZNJ6ShSd-->Handle the "ServletException"
thrown here in a "try/catch" block. <p>See more on <a
href="https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-tooling-support-install&issues=AYR7J6LgSRd6ZNJ6ShSd&open=AYR7J6LgSRd6ZNJ6ShSd&pullRequest=4";>SonarCloud</a></p>
[Show more
details](https://github.com/apache/sling-org-apache-sling-tooling-support-install/security/code-scanning/18)
##########
src/main/java/org/apache/sling/tooling/support/install/impl/InstallServlet.java:
##########
@@ -176,79 +168,87 @@
new InstallationResult(false, message).render(resp.getWriter());
}
- private void logAndWriteError(String message, Exception e,
HttpServletResponse resp) throws IOException {
- logger.info(message, e);
+ private void logAndWriteError(Exception e, HttpServletResponse resp)
throws IOException {
+ logger.info(e.getMessage(), e);
resp.setStatus(500);
- new InstallationResult(false, message + " : " +
e.getMessage()).render(resp.getWriter());
+ new InstallationResult(false, e.getMessage()).render(resp.getWriter());
}
- private void installBasedOnDirectory(HttpServletResponse resp, final File
dir) throws FileNotFoundException,
- IOException {
-
- InstallationResult result = null;
+ private void installBundleFromDirectory(HttpServletResponse resp, final
Path dir, boolean refreshPackages) throws IOException {
+ try {
+ installBundleFromDirectory(dir, refreshPackages);
+ InstallationResult result = new InstallationResult(true, null);
+ resp.setStatus(200);
+ result.render(resp.getWriter());
+ } catch (IllegalArgumentException e) {
+ logAndWriteError(e, resp);
+ }
+ }
- if ( dir.exists() && dir.isDirectory() ) {
+ /**
+ *
+ * @param dir
+ * @param refreshPackages
+ * @throws IOException
+ * @throws IllegalArgumentException if the provided directory does not
contain a valid exploded OSGi bundle
+ */
+ Bundle installBundleFromDirectory(final Path dir, boolean refreshPackages)
throws IOException {
+ 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:AYR7J6LgSRd6ZNJ6ShSf-->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=AYR7J6LgSRd6ZNJ6ShSf&open=AYR7J6LgSRd6ZNJ6ShSf&pullRequest=4";>SonarCloud</a></p>
[Show more
details](https://github.com/apache/sling-org-apache-sling-tooling-support-install/security/code-scanning/16)
##########
src/main/java/org/apache/sling/tooling/support/install/impl/InstallServlet.java:
##########
@@ -69,104 +70,95 @@
private static final String DIR = "dir";
- private static final int UPLOAD_IN_MEMORY_SIZE_THRESHOLD = 512 * 1024 *
1024;
+ public static final int UPLOAD_IN_MEMORY_SIZE_THRESHOLD = 512 * 1024 *
1024;
+ public static final int MANIFEST_SIZE_IN_INPUTSTREAM = 2 * 1024 * 1024;
- private BundleContext bundleContext;
-
- @Reference
- private PackageAdmin packageAdmin;
+ private final BundleContext bundleContext;
@Activate
- protected void activate(final BundleContext bundleContext) {
+ public InstallServlet(final BundleContext bundleContext) {
this.bundleContext = bundleContext;
}
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
final String dirPath = req.getParameter(DIR);
+ final boolean refreshPackages =
Boolean.parseBoolean(req.getParameter(dirPath));
+ boolean isMultipart = req.getContentType() != null &&
req.getContentType().toLowerCase().indexOf("multipart/form-data") > -1;
+ try {
+ if (dirPath == null && !isMultipart) {
+ logger.error("No dir parameter specified : {} and no multipart
content found", req.getParameterMap());
+ resp.setStatus(500);
+ InstallationResult result = new InstallationResult(false, "No
dir parameter specified: "
+ + req.getParameterMap() + " and no multipart content
found");
+ result.render(resp.getWriter());
+ return;
+ }
+ if (isMultipart) {
+ installBundleFromJar(req, resp, refreshPackages);
Review Comment:
## Exceptions should not be thrown from servlet methods
<!--SONAR_ISSUE_KEY:AYRsOqoHkHWZEk4hmwoQ-->Handle the following exception
that could be thrown by "installBundleFromJar": ServletException. <p>See more
on <a
href="https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-tooling-support-install&issues=AYRsOqoHkHWZEk4hmwoQ&open=AYRsOqoHkHWZEk4hmwoQ&pullRequest=4";>SonarCloud</a></p>
[Show more
details](https://github.com/apache/sling-org-apache-sling-tooling-support-install/security/code-scanning/17)
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
