Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
<module>steve-ocpp-2-x</module>
<module>steve-ocpp-soap</module>
<module>steve-ocpp-websocket</module>
<module>steve-ui</module>
<module>steve-ui-jsp</module>
<module>steve-ui-thymeleaf</module>
</modules>

<properties>
Expand Down Expand Up @@ -85,11 +87,21 @@
<artifactId>steve-api</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>de.rwth.idsg</groupId>
<artifactId>steve-ui</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>de.rwth.idsg</groupId>
<artifactId>steve-ui-jsp</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>de.rwth.idsg</groupId>
<artifactId>steve-ui-thymeleaf</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-framework-bom</artifactId>
Expand All @@ -104,6 +116,13 @@
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.ee10</groupId>
<artifactId>jetty-ee10-bom</artifactId>
<version>${jetty.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<groupId>org.junit</groupId>
<artifactId>junit-bom</artifactId>
Expand Down
8 changes: 8 additions & 0 deletions steve-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
</dependency>
Comment on lines +61 to +64
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Don’t pin a logging backend in core; make logback non-transitive

Core libraries should depend on slf4j-api only. Make logback optional so downstream runnable modules decide the backend.

Apply:

-    <dependency>
-      <groupId>ch.qos.logback</groupId>
-      <artifactId>logback-classic</artifactId>
-    </dependency>
+    <dependency>
+      <groupId>ch.qos.logback</groupId>
+      <artifactId>logback-classic</artifactId>
+      <optional>true</optional>
+    </dependency>

🏁 Script executed:

#!/bin/bash
# Check for multiple SLF4J bindings across modules
rg -n -C2 -g '**/pom.xml' 'logback-classic|slf4j-simple|log4j-to-slf4j|jul-to-slf4j'

Length of output: 1065


🏁 Script executed:

#!/bin/bash
# Check for explicit slf4j-api declaration in steve-core/pom.xml
rg -n -C2 '<artifactId>slf4j-api</artifactId>' steve-core/pom.xml

Length of output: 210


Mark logback-classic as optional in steve-core/pom.xml
Replace the direct logback binding so core only brings in slf4j-api by default:

--- steve-core/pom.xml
@@ -61,3 +61,5 @@
     <dependency>
       <groupId>ch.qos.logback</groupId>
       <artifactId>logback-classic</artifactId>
+      <optional>true</optional>
     </dependency>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<optional>true</optional>
</dependency>
🤖 Prompt for AI Agents
In steve-core/pom.xml around lines 61 to 64, the pom currently declares
logback-classic as a regular dependency which forces a concrete SLF4J binding
into consumers; mark that dependency optional so the module only brings in
slf4j-api by default by adding an <optional>true</optional> element to the
logback-classic dependency (or remove it and let consumers choose their
binding), leaving slf4j-api as the provided API.


<dependency>
<groupId>com.google.guava</groupId>
Expand All @@ -67,5 +71,9 @@
<groupId>com.neovisionaries</groupId>
<artifactId>nv-i18n</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.ee10</groupId>
<artifactId>jetty-ee10-webapp</artifactId>
</dependency>
</dependencies>
</project>
25 changes: 25 additions & 0 deletions steve-core/src/main/java/de/rwth/idsg/steve/JettyCustomizer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* SteVe - SteckdosenVerwaltung - https://github.com/steve-community/steve
* Copyright (C) 2013-2025 SteVe Community Team
* All Rights Reserved.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
package de.rwth.idsg.steve;

import org.eclipse.jetty.ee10.webapp.WebAppContext;

public interface JettyCustomizer {
void configure(WebAppContext ctx);
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ public static class Paths {
private final String routerEndpointPath;

private final @Nullable String contextPath;

public String getLocation() {
return contextPath + managerMapping + "/home";
}

Comment on lines +52 to +55
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Handle nullable contextPath and slash normalization in getLocation().

Current code risks "null..." and double slashes.

-        public String getLocation() {
-            return contextPath + managerMapping + "/home";
-        }
+        public String getLocation() {
+            String ctx = (contextPath == null || "/".equals(contextPath)) ? "" : contextPath;
+            if (!managerMapping.startsWith("/")) {
+                // Ensure managerMapping is absolute
+                ctx = ctx + "/";
+            }
+            String base = (ctx.endsWith("/")) ? ctx.substring(0, ctx.length() - 1) : ctx;
+            return base + managerMapping + "/home";
+        }

Add unit tests for contextPath = null, "", and "/foo".

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In steve-core/src/main/java/de/rwth/idsg/steve/SteveConfiguration.java around
lines 52-55, getLocation() currently concatenates contextPath and managerMapping
directly which can produce "null" or double slashes; normalize by treating a
null contextPath as empty string, strip any trailing slash from contextPath,
ensure it has a leading slash only if non-empty, and join pieces so you never
produce duplicate slashes (e.g., ensure managerMapping begins with a single '/'
or use a safe join). Update getLocation() to apply this normalization and always
return a canonical path ending with "/home". Add unit tests covering contextPath
= null, "" and "/foo" to assert expected outputs (no "null" and no double
slashes).

public String getWsPathInfix() {
return routerEndpointPath + websocketMapping;
}
Comment on lines +56 to +58
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Normalize join for WebSocket path infix.

Avoid missing or duplicate slashes.

-        public String getWsPathInfix() {
-            return routerEndpointPath + websocketMapping;
-        }
+        public String getWsPathInfix() {
+            String left = routerEndpointPath.endsWith("/") ? routerEndpointPath.substring(0, routerEndpointPath.length()-1) : routerEndpointPath;
+            String right = websocketMapping.startsWith("/") ? websocketMapping : "/" + websocketMapping;
+            return left + right;
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public String getWsPathInfix() {
return routerEndpointPath + websocketMapping;
}
public String getWsPathInfix() {
String left = routerEndpointPath.endsWith("/")
? routerEndpointPath.substring(0, routerEndpointPath.length() - 1)
: routerEndpointPath;
String right = websocketMapping.startsWith("/")
? websocketMapping
: "/" + websocketMapping;
return left + right;
}
🤖 Prompt for AI Agents
In steve-core/src/main/java/de/rwth/idsg/steve/SteveConfiguration.java around
lines 56 to 58, the getWsPathInfix() simply concatenates routerEndpointPath and
websocketMapping which can produce missing or duplicate slashes; fix by
normalizing both parts before joining: trim any trailing slash from
routerEndpointPath and any leading slash from websocketMapping, then return the
two parts joined with a single "/" between them (ensure the result starts with
"/" if routerEndpointPath is empty or originally started with "/"); also handle
null or empty websocketMapping gracefully by returning the normalized
routerEndpointPath or "/" as appropriate.

}

private final Paths paths;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import java.time.OffsetDateTime;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;

/**
Expand Down Expand Up @@ -149,9 +148,4 @@ public static String timeElapsed(Instant from, Instant to) {

return sb.toString().trim();
}

public static long getOffsetFromUtcInSeconds() {
var offset = ZonedDateTime.now().getOffset();
return offset.getTotalSeconds();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package de.rwth.idsg.steve.config;

import com.google.common.collect.Lists;
import de.rwth.idsg.steve.SteveConfiguration;
import de.rwth.idsg.steve.ocpp.ws.OcppWebSocketHandshakeHandler;
import de.rwth.idsg.steve.ocpp.ws.ocpp12.Ocpp12WebSocketEndpoint;
import de.rwth.idsg.steve.ocpp.ws.ocpp15.Ocpp15WebSocketEndpoint;
Expand All @@ -45,13 +46,13 @@
@RequiredArgsConstructor
public class OcppWebSocketConfiguration implements WebSocketConfigurer {

public static final String PATH_INFIX = "/websocket/CentralSystemService/";
public static final Duration PING_INTERVAL = Duration.ofMinutes(15);
public static final Duration IDLE_TIMEOUT = Duration.ofHours(2);
public static final int MAX_MSG_SIZE = 8_388_608; // 8 MB for max message size
public static final Duration WS_PING_INTERVAL = Duration.ofMinutes(15);
public static final Duration WS_IDLE_TIMEOUT = Duration.ofHours(2);
public static final int WS_MAX_MSG_SIZE = 8_388_608; // 8 MB for max message size

private final ChargePointRegistrationService chargePointRegistrationService;
private final ChargeBoxIdValidator chargeBoxIdValidator;
private final SteveConfiguration config;

private final Ocpp12WebSocketEndpoint ocpp12WebSocketEndpoint;
private final Ocpp15WebSocketEndpoint ocpp15WebSocketEndpoint;
Expand All @@ -60,13 +61,16 @@ public class OcppWebSocketConfiguration implements WebSocketConfigurer {
@Override
public void registerWebSocketHandlers(WebSocketHandlerRegistry registry) {

OcppWebSocketHandshakeHandler handshakeHandler = new OcppWebSocketHandshakeHandler(
var handshakeHandler = new OcppWebSocketHandshakeHandler(
chargeBoxIdValidator,
new DefaultHandshakeHandler(),
Lists.newArrayList(ocpp16WebSocketEndpoint, ocpp15WebSocketEndpoint, ocpp12WebSocketEndpoint),
chargePointRegistrationService);
chargePointRegistrationService,
config.getPaths().getWsPathInfix() + "/");

registry.addHandler(handshakeHandler.getDummyWebSocketHandler(), PATH_INFIX + "*")
registry.addHandler(
handshakeHandler.getDummyWebSocketHandler(),
config.getPaths().getWsPathInfix() + "/*")
.setHandshakeHandler(handshakeHandler)
.setAllowedOrigins("*");
}
Comment on lines +71 to 76
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Restrict WebSocket allowed origins.

setAllowedOrigins("*") enables any origin. Prefer explicit origins or config-driven patterns.

-                .setAllowedOrigins("*");
+                // Prefer patterns or a configured list
+                .setAllowedOriginPatterns(config.getSecurity().getAllowedWsOrigins());

If no such config exists, introduce one or restrict to known UI origins.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
steve-ocpp-websocket/src/main/java/de/rwth/idsg/steve/config/OcppWebSocketConfiguration.java
around lines 71 to 76, the WebSocket mapping currently calls
setAllowedOrigins("*"), which allows any origin; replace this with a
config-driven, explicit allowed-origins value (e.g. config.getAllowedOrigins())
or introduce a new property if it doesn't exist, validate/parse it
(comma-separated list or patterns) and pass the resulting origin array to
setAllowedOrigins; ensure a sensible default (restrict to known UI host(s) or
fail fast) and avoid using the wildcard.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ public void onOpen(WebSocketSession session) throws Exception {
// the connection because of a idle timeout, we ping-pong at fixed intervals.
var pingSchedule = asyncTaskScheduler.scheduleAtFixedRate(
new PingTask(chargeBoxId, session),
Instant.now().plus(OcppWebSocketConfiguration.PING_INTERVAL),
OcppWebSocketConfiguration.PING_INTERVAL);
Instant.now().plus(OcppWebSocketConfiguration.WS_PING_INTERVAL),
OcppWebSocketConfiguration.WS_PING_INTERVAL);

futureResponseContextStore.addSession(session);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
public abstract class ConcurrentWebSocketHandler implements WebSocketHandler {

private static final int SEND_TIME_LIMIT = (int) TimeUnit.SECONDS.toMillis(10);
private static final int BUFFER_SIZE_LIMIT = 5 * OcppWebSocketConfiguration.MAX_MSG_SIZE;
private static final int BUFFER_SIZE_LIMIT = 5 * OcppWebSocketConfiguration.WS_MAX_MSG_SIZE;

private final Map<String, ConcurrentWebSocketSessionDecorator> sessions = new ConcurrentHashMap<>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package de.rwth.idsg.steve.ocpp.ws;

import com.google.common.base.Strings;
import de.rwth.idsg.steve.config.OcppWebSocketConfiguration;
import de.rwth.idsg.steve.service.ChargePointRegistrationService;
import de.rwth.idsg.steve.web.validation.ChargeBoxIdValidator;
import lombok.RequiredArgsConstructor;
Expand Down Expand Up @@ -51,6 +50,7 @@ public class OcppWebSocketHandshakeHandler implements HandshakeHandler {
private final DefaultHandshakeHandler delegate;
private final List<AbstractWebSocketEndpoint> endpoints;
private final ChargePointRegistrationService chargePointRegistrationService;
private final String wsPathInfix;

/**
* We need some WebSocketHandler just for Spring to register it for the path. We will not use it for the actual
Expand All @@ -73,7 +73,7 @@ public boolean doHandshake(
// 1. Check the chargeBoxId
// -------------------------------------------------------------------------

var chargeBoxId = getLastBitFromUrl(request.getURI().getPath());
var chargeBoxId = getLastBitFromUrl(wsPathInfix, request.getURI().getPath());
var isValid = chargeBoxIdValidator.isValid(chargeBoxId);
if (!isValid) {
log.error("ChargeBoxId '{}' violates the configured pattern.", chargeBoxId);
Expand Down Expand Up @@ -133,17 +133,15 @@ public boolean doHandshake(
return null;
}

public static String getLastBitFromUrl(final String input) {
public static String getLastBitFromUrl(String path, String input) {
if (Strings.isNullOrEmpty(input)) {
return "";
}

var substring = OcppWebSocketConfiguration.PATH_INFIX;

var index = input.indexOf(substring);
var index = input.indexOf(path);
if (index == -1) {
return "";
}
return input.substring(index + substring.length());
return input.substring(index + path.length());
}
Comment on lines +136 to 146
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

getLastBitFromUrl returns more than the last segment when extra path parts exist

Ensure only the next segment after the infix is returned (the chargeBoxId), ignoring trailing segments.

-    public static String getLastBitFromUrl(String path, String input) {
+    public static String getLastBitFromUrl(String path, String input) {
         if (Strings.isNullOrEmpty(input)) {
             return "";
         }
-
-        var index = input.indexOf(path);
-        if (index == -1) {
-            return "";
-        }
-        return input.substring(index + path.length());
+        int start = input.indexOf(path);
+        if (start == -1) {
+            return "";
+        }
+        int from = start + path.length();
+        // Skip any leading slash just in case
+        if (from < input.length() && input.charAt(from) == '/') {
+            from++;
+        }
+        int next = input.indexOf('/', from);
+        return next == -1 ? input.substring(from) : input.substring(from, next);
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public static String getLastBitFromUrl(String path, String input) {
if (Strings.isNullOrEmpty(input)) {
return "";
}
var substring = OcppWebSocketConfiguration.PATH_INFIX;
var index = input.indexOf(substring);
var index = input.indexOf(path);
if (index == -1) {
return "";
}
return input.substring(index + substring.length());
return input.substring(index + path.length());
}
public static String getLastBitFromUrl(String path, String input) {
if (Strings.isNullOrEmpty(input)) {
return "";
}
int start = input.indexOf(path);
if (start == -1) {
return "";
}
int from = start + path.length();
// Skip any leading slash just in case
if (from < input.length() && input.charAt(from) == '/') {
from++;
}
int next = input.indexOf('/', from);
return next == -1
? input.substring(from)
: input.substring(from, next);
}
🤖 Prompt for AI Agents
In
steve-ocpp-websocket/src/main/java/de/rwth/idsg/steve/ocpp/ws/OcppWebSocketHandshakeHandler.java
around lines 136 to 146, getLastBitFromUrl currently returns everything after
the infix path, which can include multiple trailing segments; change it to
return only the next path segment (the chargeBoxId). After locating the infix,
take the substring starting at index + path.length(), trim any leading '/'
characters, then split that remainder on '/' and return the first element (or
empty string if none); preserve the existing null/empty checks and handle cases
where the infix is not present by returning empty string.

}
17 changes: 12 additions & 5 deletions steve-ui-jsp/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
<artifactId>steve-ui-jsp</artifactId>

<dependencies>
<dependency>
<groupId>de.rwth.idsg</groupId>
<artifactId>steve-ui</artifactId>
</dependency>
<dependency>
<groupId>de.rwth.idsg</groupId>
<artifactId>steve-core</artifactId>
Expand Down Expand Up @@ -55,7 +59,6 @@
<dependency>
<groupId>org.eclipse.jetty.ee10</groupId>
<artifactId>jetty-ee10-apache-jsp</artifactId>
<version>${jetty.version}</version>
</dependency>
<!-- <dependency>-->
<!-- <groupId>org.eclipse.jetty</groupId>-->
Expand All @@ -67,10 +70,6 @@
<artifactId>encoder-jakarta-jsp</artifactId>
<version>${owasp-encoder.version}</version>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
</dependency>
<dependency>
<groupId>com.neovisionaries</groupId>
<artifactId>nv-i18n</artifactId>
Expand All @@ -91,6 +90,14 @@
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.ee10</groupId>
<artifactId>jetty-ee10-servlet</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.ee10</groupId>
<artifactId>jetty-ee10-webapp</artifactId>
</dependency>
</dependencies>

<build>
Expand Down

This file was deleted.

Loading
Loading