Improved missing ping, fixed fortify issue

pom.xml

@@ -262,10 +262,10 @@
     </dependency>
 
     <!-- JMX -->
-<!--     <dependency> -->
+    <!-- <dependency> -->
-<!--       <groupId>org.jolokia</groupId> -->
+    <!-- <groupId>org.jolokia</groupId> -->
-<!--       <artifactId>jolokia-core</artifactId> -->
+    <!-- <artifactId>jolokia-core</artifactId> -->
-<!--     </dependency> -->
+    <!-- </dependency> -->
 
     <!-- Apache HTTP -->
     <dependency>
@@ -303,6 +303,11 @@
       <artifactId>jncryptor</artifactId>
       <version>1.2.0</version>
     </dependency>
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-text</artifactId>
+      <version>1.8</version>
+    </dependency>
 
 
     <!-- Testing -->

src/main/java/ch/ethz/seb/sebserver/ClientHttpRequestFactoryService.java

@@ -183,7 +183,7 @@ public class ClientHttpRequestFactoryService {
                     .toCharArray();
 
             if (password.length < 3) {
-                log.error("Missing or incorrect trust-store password: " + String.valueOf(password));
+                log.error("Missing or incorrect trust-store password");
                 throw new IllegalArgumentException("Missing or incorrect trust-store password");
             }
 

src/main/java/ch/ethz/seb/sebserver/gbl/api/JSONMapper.java

@@ -11,6 +11,7 @@ package ch.ethz.seb.sebserver.gbl.api;
 import org.springframework.context.annotation.Lazy;
 import org.springframework.stereotype.Component;
 
+import com.fasterxml.jackson.annotation.JsonInclude.Include;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.datatype.joda.JodaModule;
 
@@ -29,6 +30,7 @@ public class JSONMapper extends ObjectMapper {
         super.configure(
                 com.fasterxml.jackson.databind.SerializationFeature.WRITE_DATES_WITH_ZONE_ID,
                 false);
+        super.setSerializationInclusion(Include.NON_NULL);
     }
 
 }

src/main/java/ch/ethz/seb/sebserver/gbl/model/session/ClientConnectionData.java

@@ -16,8 +16,6 @@ import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
-import ch.ethz.seb.sebserver.gbl.model.exam.Indicator.IndicatorType;
-import ch.ethz.seb.sebserver.gbl.model.session.ClientConnection.ConnectionStatus;
 import ch.ethz.seb.sebserver.gbl.util.Utils;
 
 public class ClientConnectionData {
@@ -26,31 +24,32 @@ public class ClientConnectionData {
     public final ClientConnection clientConnection;
     @JsonProperty("indicatorValues")
     public final List<? extends IndicatorValue> indicatorValues;
-    @JsonIgnore
+
-    public final boolean missingPing;
+    public final Boolean missingPing;
 
     @JsonCreator
-    protected ClientConnectionData(
+    public ClientConnectionData(
+            @JsonProperty("missingPing") final Boolean missingPing,
             @JsonProperty("clientConnection") final ClientConnection clientConnection,
             @JsonProperty("indicatorValues") final Collection<? extends SimpleIndicatorValue> indicatorValues) {
 
+        this.missingPing = missingPing;
         this.clientConnection = clientConnection;
         this.indicatorValues = Utils.immutableListOf(indicatorValues);
-        this.missingPing = clientConnection.status == ConnectionStatus.ACTIVE &&
-                this.indicatorValues.stream()
-                        .filter(ind -> ind.getType() == IndicatorType.LAST_PING)
-                        .findFirst()
-                        .map(ind -> (long) ind.getValue())
-                        .orElse(0L) > 5000;
     }
 
     protected ClientConnectionData(
-            @JsonProperty("clientConnection") final ClientConnection clientConnection,
+            final ClientConnection clientConnection,
-            @JsonProperty("indicatorValues") final List<? extends IndicatorValue> indicatorValues) {
+            final List<? extends IndicatorValue> indicatorValues) {
 
+        this.missingPing = null;
         this.clientConnection = clientConnection;
         this.indicatorValues = Utils.immutableListOf(indicatorValues);
-        this.missingPing = false;
+    }
+
+    @JsonProperty("missingPing")
+    public Boolean getMissingPing() {
+        return this.missingPing;
     }
 
     @JsonIgnore

src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java

@@ -28,6 +28,7 @@ import java.util.stream.Collector;
 import java.util.stream.Collectors;
 
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.text.StringEscapeUtils;
 import org.eclipse.swt.graphics.RGB;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
@@ -375,6 +376,13 @@ public final class Utils {
         return builder.toString();
     }
 
+    public static String escapeHTML_XML_EcmaScript(final String string) {
+        return StringEscapeUtils.escapeXml11(
+                StringEscapeUtils.escapeHtml4(
+                        StringEscapeUtils.escapeEcmaScript(string)));
+    }
+
+    // https://www.owasp.org/index.php/HTTP_Response_Splitting
     public static String preventResponseSplittingAttack(final String string) {
         final int xni = string.indexOf('\n');
         final int xri = string.indexOf('\r');

src/main/java/ch/ethz/seb/sebserver/gui/service/remote/download/AbstractDownloadServiceHandler.java

@@ -61,8 +61,9 @@ public abstract class AbstractDownloadServiceHandler implements DownloadServiceH
                         downloadFileName);
             }
 
-            final String header =
+            final String header = "attachment; filename=\"" +
-                    "attachment; filename=\"" + Utils.preventResponseSplittingAttack(downloadFileName) + "\"";
+                    Utils.escapeHTML_XML_EcmaScript(Utils.preventResponseSplittingAttack(downloadFileName)) +
+                    "\"";
             response.setHeader(HttpHeaders.CONTENT_DISPOSITION, header);
             response.setContentType(MediaType.APPLICATION_OCTET_STREAM_VALUE);
 

src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ClientConnectionDataInternal.java

@@ -14,6 +14,8 @@ import java.util.Collections;
 import java.util.EnumMap;
 import java.util.List;
 
+import com.fasterxml.jackson.annotation.JsonProperty;
+
 import ch.ethz.seb.sebserver.gbl.model.session.ClientConnection;
 import ch.ethz.seb.sebserver.gbl.model.session.ClientConnectionData;
 import ch.ethz.seb.sebserver.gbl.model.session.ClientEvent.EventType;
@@ -61,4 +63,10 @@ public class ClientConnectionDataInternal extends ClientConnectionData {
         return this.indicatorMapping.get(eventType);
     }
 
+    @Override
+    @JsonProperty("missingPing")
+    public Boolean getMissingPing() {
+        return this.pingIndicator.missingPing;
+    }
+
 }

src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/PingIntervalClientIndicator.java

@@ -36,8 +36,8 @@ public final class PingIntervalClientIndicator extends AbstractPingIndicator {
 
     private static final Logger log = LoggerFactory.getLogger(PingIntervalClientIndicator.class);
 
-    private long pingErrorThreshold;
+    long pingErrorThreshold;
-    private boolean isOnError = false;
+    boolean missingPing = false;
 
     boolean hidden = false;
 
@@ -84,9 +84,9 @@ public final class PingIntervalClientIndicator extends AbstractPingIndicator {
     public ClientEventRecord updateLogEvent() {
         final long now = DateTime.now(DateTimeZone.UTC).getMillis();
         final long value = now - (long) super.currentValue;
-        if (this.isOnError) {
+        if (this.missingPing) {
             if (this.pingErrorThreshold > value) {
-                this.isOnError = false;
+                this.missingPing = false;
                 return new ClientEventRecord(
                         null,
                         this.connectionId,
@@ -98,7 +98,7 @@ public final class PingIntervalClientIndicator extends AbstractPingIndicator {
             }
         } else {
             if (this.pingErrorThreshold < value) {
-                this.isOnError = true;
+                this.missingPing = true;
                 return new ClientEventRecord(
                         null,
                         this.connectionId,

src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/SebClientConnectionServiceImpl.java

@@ -13,6 +13,7 @@ import java.util.Objects;
 import java.util.UUID;
 import java.util.function.Predicate;
 
+import org.apache.commons.lang3.BooleanUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -25,6 +26,7 @@ import ch.ethz.seb.sebserver.gbl.model.exam.Exam;
 import ch.ethz.seb.sebserver.gbl.model.exam.Exam.ExamType;
 import ch.ethz.seb.sebserver.gbl.model.session.ClientConnection;
 import ch.ethz.seb.sebserver.gbl.model.session.ClientConnection.ConnectionStatus;
+import ch.ethz.seb.sebserver.gbl.model.session.ClientConnectionData;
 import ch.ethz.seb.sebserver.gbl.model.session.ClientEvent;
 import ch.ethz.seb.sebserver.gbl.profile.WebServiceProfile;
 import ch.ethz.seb.sebserver.gbl.util.Result;
@@ -385,6 +387,17 @@ public class SebClientConnectionServiceImpl implements SebClientConnectionServic
     @Override
     public Result<ClientConnection> disableConnection(final String connectionToken, final Long institutionId) {
         return Result.tryCatch(() -> {
+            final ClientConnectionData connectionData = getExamSessionService()
+                    .getConnectionData(connectionToken)
+                    .getOrThrow();
+
+            // An active connection can only be disabled if we have a missing ping
+            if (connectionData.clientConnection.status == ConnectionStatus.ACTIVE &&
+                    !BooleanUtils.isTrue(connectionData.getMissingPing())) {
+
+                return connectionData.clientConnection;
+            }
+
             if (log.isDebugEnabled()) {
                 log.debug("SEB client connection: SEB Server disable attempt for "
                         + "instituion {} "

src/test/java/ch/ethz/seb/sebserver/gbl/model/institution/InstitutionTest.java

@@ -62,4 +62,13 @@ public class InstitutionTest {
                 json);
     }
 
+    @Test
+    public void testNullValues() throws Exception {
+        final JSONMapper jsonMapper = new JSONMapper();
+
+        final Institution inst = new Institution(1L, null, "suffix", "logo", "theme", null);
+        final String jsonString = jsonMapper.writeValueAsString(inst);
+        assertEquals("{\"id\":1,\"urlSuffix\":\"suffix\",\"logoImage\":\"logo\",\"themeName\":\"theme\"}", jsonString);
+    }
+
 }