Skip to content

Commit

Permalink
#26277 sonarq feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jdotcms committed Jan 9, 2024
1 parent 15a43ed commit dae12cf
Show file tree
Hide file tree
Showing 19 changed files with 140 additions and 143 deletions.
5 changes: 2 additions & 3 deletions dotCMS/src/main/java/com/dotcms/http/CircuitBreakerUrl.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public class CircuitBreakerUrl {
private static final Lazy<Boolean> allowAccessToPrivateSubnets = Lazy.of(()->Config.getBooleanProperty("ALLOW_ACCESS_TO_PRIVATE_SUBNETS", false));
private static final CircuitBreakerConnectionControl circuitBreakerConnectionControl = new CircuitBreakerConnectionControl(circuitBreakerMaxConnTotal.get());

public static final Response<String> EMPTY_RESPONSE = new Response<>(StringPool.BLANK, 0, new Header[]{});
/**
*
* @param proxyUrl
Expand Down Expand Up @@ -397,7 +398,7 @@ public void end(final long id) {
}
}

public static class Response<T extends Serializable> {
public static class Response<T extends Serializable> implements Serializable {

private final T response;
private final int statusCode;
Expand Down Expand Up @@ -446,6 +447,4 @@ public String toString() {
}
}

public static Response<String> EMPTY_RESPONSE = new Response<>(StringPool.BLANK, 0, new Header[]{});

}
20 changes: 20 additions & 0 deletions dotCMS/src/main/java/com/dotcms/rendering/JsEngineException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.dotcms.rendering;

import com.dotmarketing.exception.DotRuntimeException;

/**
* Generic exception for the JS engine
*/
public class JsEngineException extends DotRuntimeException {
public JsEngineException(String message) {
super(message);
}

public JsEngineException(Throwable cause) {
super(cause);
}

public JsEngineException(String message, Throwable cause) {
super(message, cause);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package com.dotcms.rendering.js;

import org.jetbrains.annotations.NotNull;

import java.io.IOException;
import java.io.OutputStream;
import java.util.Objects;
import java.util.function.Consumer;

/**
Expand Down Expand Up @@ -34,9 +38,25 @@ public void write (final int b) {
}
}

/**
* Writes an array of bytes to the output stream. This method flushes automatically at the end of a line.
* @param bytes the data.
* @param off the start offset in the data.
* @param len the number of bytes to write.
* @throws IOException
*/
@Override
public void write(final byte[] bytes, final int off, final int len) throws IOException {
Objects.checkFromIndexSize(off, len, bytes.length);
for (int i = 0 ; i < len ; i++) {
write(bytes[off + i]);
}
}

/**
* Flushes the output stream.
*/
@Override
public void flush () {
this.loggerConsumer.accept(builder.toString());
builder.setLength(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public static Reader getJavascriptReaderFromPath (final String jsFilePath, final
final Identifier identifier = APILocator.getIdentifierAPI().find(site, jsFilePath);
final Contentlet getFileContent;

if (UtilMethods.isEmpty(()->identifier.getId())) {
if (UtilMethods.isEmpty(identifier::getId)) {

throw new DoesNotExistException ("The Javascript: " + jsFilePath + " does not exists");
}
Expand Down
3 changes: 0 additions & 3 deletions dotCMS/src/main/java/com/dotcms/rendering/js/JsContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,11 @@
import com.dotcms.rendering.js.proxy.JsResponse;
import io.vavr.Tuple;
import io.vavr.Tuple2;
import org.graalvm.polyglot.HostAccess;
import org.graalvm.polyglot.Value;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Objects;

/**
* Encapsulates the context of the Javascript execution.
Expand Down
78 changes: 32 additions & 46 deletions dotCMS/src/main/java/com/dotcms/rendering/js/JsEngine.java
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
package com.dotcms.rendering.js;

import com.dotcms.api.vtl.model.DotJSON;
import com.dotcms.rendering.JsEngineException;
import com.dotcms.rendering.engine.ScriptEngine;
import com.dotcms.rendering.js.viewtools.FetchJsViewTool;
import com.dotcms.rendering.js.proxy.JsProxyFactory;
import com.dotcms.rendering.js.proxy.JsRequest;
import com.dotcms.rendering.js.proxy.JsResponse;
import com.dotcms.rendering.js.viewtools.CacheJsViewTool;
import com.dotcms.rendering.js.viewtools.CategoriesJsViewTool;
import com.dotcms.rendering.js.viewtools.ContainerJsViewTool;
import com.dotcms.rendering.js.viewtools.ContentJsViewTool;
import com.dotcms.rendering.js.viewtools.FetchJsViewTool;
import com.dotcms.rendering.js.viewtools.LanguageJsViewTool;
import com.dotcms.rendering.js.viewtools.SecretJsViewTool;
import com.dotcms.rendering.js.viewtools.SiteJsViewTool;
Expand All @@ -26,24 +27,17 @@
import com.dotmarketing.util.Logger;
import com.dotmarketing.util.VelocityUtil;
import com.liferay.util.FileUtil;
import com.oracle.truffle.api.Assumption;
import com.oracle.truffle.api.object.DynamicObject;
import com.oracle.truffle.api.object.JsDynamicObjectUtils;
import com.oracle.truffle.api.object.Shape;
import com.oracle.truffle.js.lang.JavaScriptLanguage;
import com.oracle.truffle.js.runtime.GraalJSException;
import com.oracle.truffle.js.runtime.builtins.JSErrorObject;
import com.oracle.truffle.js.runtime.builtins.JSPromise;
import com.oracle.truffle.js.runtime.builtins.JSPromiseObject;
import com.oracle.truffle.object.LayoutImpl;
import com.oracle.truffle.object.LayoutStrategy;
import com.oracle.truffle.object.ShapeImpl;
import io.vavr.control.Try;
import org.apache.commons.lang3.StringUtils;
import org.apache.velocity.tools.view.context.ChainedContext;
import org.apache.velocity.tools.view.context.ViewContext;
import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.PolyglotException;
import org.graalvm.polyglot.Source;
import org.graalvm.polyglot.Value;

Expand All @@ -53,8 +47,8 @@
import java.io.IOException;
import java.io.Reader;
import java.io.StringReader;
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -69,12 +63,14 @@
public class JsEngine implements ScriptEngine {

private static final String ENGINE_JS = JavaScriptLanguage.ID;
public static final String DOT_JSON = "dotJSON";
public static final String WEB_INF = "WEB-INF";
private final JsFileSystem jsFileSystem = new JsFileSystem();
private final JsDotLogger jsDotLogger = new JsDotLogger();
private final Map<String, Class> jsRequestViewToolMap = new ConcurrentHashMap<>();
private final Map<String, JsViewTool> jsAplicationViewToolMap = new ConcurrentHashMap<>();

{
public JsEngine () {
try {
this.addJsViewTool(UserJsViewTool.class);
this.addJsViewTool(LanguageJsViewTool.class);
Expand Down Expand Up @@ -111,19 +107,6 @@ public <T extends JsViewTool> void addJsViewTool (final Class<T> jsViewTool) {
}
}

private void initApplicationView() {

this.jsAplicationViewToolMap.entrySet().forEach(entry -> {

final JsViewTool instance = entry.getValue();

if (instance instanceof JsApplicationContextAware) {

JsApplicationContextAware.class.cast(instance).setContext(Config.CONTEXT);
}
});
}

private void initApplicationView(final JsViewTool jsViewToolInstance) {

if (jsViewToolInstance instanceof JsApplicationContextAware) {
Expand All @@ -147,8 +130,8 @@ private Context buildContext () {
.allowIO(true)
.allowExperimentalOptions(true)
.option("js.esm-eval-returns-exports", "true")
.out(new ConsumerOutputStream((msg)->Logger.debug(JsEngine.class, msg)))
.err(new ConsumerOutputStream((msg)->Logger.debug(JsEngine.class, msg)))
.out(new ConsumerOutputStream(msg->Logger.debug(JsEngine.class, msg)))
.err(new ConsumerOutputStream(msg->Logger.debug(JsEngine.class, msg)))
.fileSystem(jsFileSystem)
//.allowHostAccess(HostAccess.ALL) // todo: ask if we want all access to the classpath
//allows access to all Java classes
Expand All @@ -162,7 +145,7 @@ public Object eval(final HttpServletRequest request,
final Reader scriptReader,
final Map<String, Object> contextParams) {

final DotJSON dotJSON = (DotJSON)contextParams.computeIfAbsent("dotJSON", k -> new DotJSON());
final DotJSON dotJSON = (DotJSON)contextParams.computeIfAbsent(DOT_JSON, k -> new DotJSON());
try (Context context = buildContext()) {

final Object fileName = contextParams.getOrDefault("dot:jsfilename", "sample.js");
Expand All @@ -174,11 +157,11 @@ public Object eval(final HttpServletRequest request,

final JsRequest jsRequest = new JsRequest(request, contextParams);
final JsResponse jsResponse = new JsResponse(response);
bindings.putMember("dotJSON", dotJSON);
bindings.putMember(DOT_JSON, dotJSON);
bindings.putMember("request", jsRequest);
bindings.putMember("response", jsResponse);

dotSources.stream().forEach(source -> context.eval(source));
dotSources.stream().forEach(context::eval);
Value eval = context.eval(userSource);
if (eval.canExecute()) {
eval = contextParams.containsKey("dot:arguments")?
Expand All @@ -192,7 +175,7 @@ public Object eval(final HttpServletRequest request,
} catch (final IOException e) {

Logger.error(this, e.getMessage(), e);
throw new RuntimeException(e);
throw new JsEngineException(e);
}
}

Expand Down Expand Up @@ -220,7 +203,7 @@ private Object asValue (final Value eval, final DotJSON dotJSON) {
return CollectionsUtils.toSerializableMap(resultMap); // we need to do that b.c the context will be close after the return and the resultMap won;t be usable.
}

return CollectionsUtils.map("output", eval.asString(), "dotJSON", dotJSON);
return CollectionsUtils.map("output", eval.asString(), DOT_JSON, dotJSON);
}

private void checkRejected(final Value eval) {
Expand Down Expand Up @@ -263,10 +246,16 @@ private String stackTraceToString (final Object[] stackTraceArray) {

private String jsStrackTraceToString(final GraalJSException.JSStackTraceElement element) {

return null == element.getClassName()?"UnknownClass":element.getClassName() + "." + element.getFunctionName().toString() + "(" +
(element.getFileName() != null && element.getLineNumber() >= 0 ?
element.getFileName() + ":" + element.getLineNumber() + ")" :
(element.getFileName() != null ? ""+element.getFileName()+")" : "Unknown Source)"));
return null == element.getClassName()?
"UnknownClass":
element.getClassName() + "." + element.getFunctionName().toString() + getJsStrackTraceFileLineNumber(element);
}

private static String getJsStrackTraceFileLineNumber(final GraalJSException.JSStackTraceElement element) {

return "(" + element.getFileName() != null && element.getLineNumber() >= 0 ?
element.getFileName() + ":" + element.getLineNumber() + ")" :
(element.getFileName() != null ? "" + element.getFileName() + ")" : "Unknown Source)");
}

private List<Source> getDotSources() throws IOException {
Expand All @@ -289,8 +278,8 @@ private void addModules(final List<Source> sources) throws IOException {
Logger.warn(this, "Context is null, can't load modules");
return;
}
final String absoluteWebInfPath = Config.CONTEXT.getRealPath(File.separator + "WEB-INF");
final String relativeModulesPath = File.separator + "WEB-INF" + File.separator + "javascript" + File.separator + "modules" + File.separator;
final String absoluteWebInfPath = Config.CONTEXT.getRealPath(File.separator + WEB_INF);
final String relativeModulesPath = File.separator + WEB_INF + File.separator + "javascript" + File.separator + "modules" + File.separator;
final String absoluteModulesPath = Config.CONTEXT.getRealPath(relativeModulesPath);
FileUtil.walk(absoluteModulesPath,
path -> path.getFileName().toString().endsWith(".mjs"), path -> {
Expand All @@ -307,7 +296,7 @@ private void addModules(final List<Source> sources) throws IOException {
}

private void addFunctions(final List<Source> sources) throws IOException {
final String relativeFunctionsPath = File.separator + "WEB-INF" + File.separator + "javascript" + File.separator + "functions" + File.separator;
final String relativeFunctionsPath = File.separator + WEB_INF + File.separator + "javascript" + File.separator + "functions" + File.separator;
if (Objects.isNull(Config.CONTEXT)) {
Logger.warn(this, "Context is null, can't load functions");
return;
Expand Down Expand Up @@ -367,7 +356,7 @@ public static Source toSource (final String absolutePath, final File file) {

final StringReader stringReader = new StringReader(sourceContent);
source = Try.of(() ->
Source.newBuilder(ENGINE_JS, stringReader, absolutePath).build()).getOrElseThrow(e -> new RuntimeException(e));
Source.newBuilder(ENGINE_JS, stringReader, absolutePath).build()).getOrElseThrow(JsEngineException::new);
}

return source;
Expand Down Expand Up @@ -396,10 +385,6 @@ public static Source toModuleSource (final String absolutePath, final String mod
return source;
}

private boolean isString(final Value eval) {
return eval.isString();
}

private Object[] buildArgs(final JsRequest request,
final JsResponse response,
final Object[] objects) {
Expand All @@ -419,14 +404,15 @@ private void addTools(final HttpServletRequest request,
this.jsRequestViewToolMap.entrySet().forEach(entry -> {

try {
final Object instance = entry.getValue().newInstance();
final Object instance = entry.getValue().getDeclaredConstructor().newInstance();
if (instance instanceof JsViewTool) {

final JsViewTool jsViewTool = (JsViewTool)instance;
initJsViewTool(request, response, jsViewTool);
bindings.putMember(jsViewTool.getName(), instance);
}
} catch (final InstantiationException | IllegalAccessException e) {
} catch (final InstantiationException | IllegalAccessException | NoSuchMethodException |
InvocationTargetException e) {

Logger.error(this, e.getMessage(), e);
}
Expand Down Expand Up @@ -460,8 +446,8 @@ private void initJsViewTool(final HttpServletRequest request,

if (instance instanceof JsViewContextAware) {

final ViewContext velocityContext = new ChainedContext(VelocityUtil.getBasicContext(), request,
response, Config.CONTEXT);
final ViewContext velocityContext = new ChainedContext(
VelocityUtil.getBasicContext(), null, request, response);
JsViewContextAware.class.cast(instance).setViewContext(velocityContext);
}
}
Expand Down
11 changes: 5 additions & 6 deletions dotCMS/src/main/java/com/dotcms/rendering/js/JsFileSystem.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@
import com.dotmarketing.portlets.contentlet.business.DotContentletStateException;
import com.dotmarketing.portlets.contentlet.model.Contentlet;
import com.dotmarketing.portlets.fileassets.business.FileAsset;
import com.dotmarketing.util.Config;
import com.dotmarketing.util.HostUtil;
import com.dotmarketing.util.UtilMethods;
import com.oracle.truffle.js.lang.JavaScriptLanguage;
import io.vavr.Tuple2;
import org.apache.commons.compress.utils.SeekableInMemoryByteChannel;
import org.graalvm.polyglot.io.FileSystem;
Expand Down Expand Up @@ -51,6 +49,7 @@ public class JsFileSystem implements FileSystem {
private static final String APPLICATION_ROOT_PATH = "/application/";
private static final String JAVASCRIPT_ROOT_PATH = "/javascript/";
public static final String DOT_SITE_TOKEN = "/dot-site-token/";
public static final String PATH_IS_ONLY_ALLOWED_ON = "Path is only allowed on: (";

public JsFileSystem() {
}
Expand All @@ -75,7 +74,7 @@ private Path checkAllowedPath(final Path path) {
}

if (!pathString.startsWith(JAVASCRIPT_ROOT_PATH) && !pathString.startsWith(APPLICATION_ROOT_PATH)) {
throw new IllegalArgumentException("Path is only allowed on: (" + JAVASCRIPT_ROOT_PATH + " or " + APPLICATION_ROOT_PATH + ")");
throw new IllegalArgumentException(PATH_IS_ONLY_ALLOWED_ON + JAVASCRIPT_ROOT_PATH + " or " + APPLICATION_ROOT_PATH + ")");
}

return path;
Expand All @@ -92,11 +91,11 @@ private void checkHost(final String pathString) {
}

if (Objects.isNull(pathHostTuple._1())) {
throw new IllegalArgumentException("Path is only allowed on: (" + JAVASCRIPT_ROOT_PATH + " or " + APPLICATION_ROOT_PATH + ")");
throw new IllegalArgumentException(PATH_IS_ONLY_ALLOWED_ON + JAVASCRIPT_ROOT_PATH + " or " + APPLICATION_ROOT_PATH + ")");
}

} catch (Exception e) {
throw new IllegalArgumentException("Path is only allowed on: (" + JAVASCRIPT_ROOT_PATH + " or " + APPLICATION_ROOT_PATH + ")");
throw new IllegalArgumentException(PATH_IS_ONLY_ALLOWED_ON + JAVASCRIPT_ROOT_PATH + " or " + APPLICATION_ROOT_PATH + ")");
}
}

Expand Down Expand Up @@ -181,7 +180,7 @@ private Path applicationFolderToRealPath (final Path path) {
}

if (!modulePath.startsWith(APPLICATION_ROOT_PATH)) {
throw new IllegalArgumentException("Path is only allowed on: (" + JAVASCRIPT_ROOT_PATH + " or " + APPLICATION_ROOT_PATH + ")");
throw new IllegalArgumentException(PATH_IS_ONLY_ALLOWED_ON + JAVASCRIPT_ROOT_PATH + " or " + APPLICATION_ROOT_PATH + ")");
}

final Identifier identifier = APILocator.getIdentifierAPI().find(site, modulePath);
Expand Down
Loading

0 comments on commit dae12cf

Please sign in to comment.