From 39714b90b3b48e45f69468e0c7fdecff5c3c7312 Mon Sep 17 00:00:00 2001 From: trifonovt <87468028+TihomirTrifonov@users.noreply.github.com> Date: Tue, 12 May 2026 14:46:35 +0200 Subject: [PATCH] Harden tachograph file session extraction --- .../DriverCardXmlExtractionService.java | 60 ++++++++++++++++--- .../service/LegalRequirementsClient.java | 11 +++- .../service/TachographFileSessionService.java | 16 ++++- .../service/TachographXmlParser.java | 8 +++ .../DriverCardXmlExtractionServiceTest.java | 5 +- .../TachographFileSessionServiceTest.java | 21 +++++++ .../service/TachographXmlParserTest.java | 13 ++++ 7 files changed, 118 insertions(+), 16 deletions(-) diff --git a/src/main/java/at/procon/eventhub/tachographfilesession/service/DriverCardXmlExtractionService.java b/src/main/java/at/procon/eventhub/tachographfilesession/service/DriverCardXmlExtractionService.java index 768e9c0..322a612 100644 --- a/src/main/java/at/procon/eventhub/tachographfilesession/service/DriverCardXmlExtractionService.java +++ b/src/main/java/at/procon/eventhub/tachographfilesession/service/DriverCardXmlExtractionService.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.TreeSet; import java.util.UUID; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; @@ -275,14 +276,45 @@ public class DriverCardXmlExtractionService { ) { List result = new ArrayList<>(activityIntervals.size()); for (ExtractedCardActivityInterval interval : activityIntervals) { - ExtractedCardVehicleUsageInterval covering = vehicleUsageIntervals.stream() - .filter(usage -> !usage.from().isAfter(interval.from()) && !usage.to().isBefore(interval.from())) - .findFirst() - .orElse(null); - result.add(new ExtractedCardActivityInterval( - interval.intervalId(), - interval.from(), - interval.to(), + result.addAll(splitByVehicleCoverage(interval, vehicleUsageIntervals)); + } + return result; + } + + private List splitByVehicleCoverage( + ExtractedCardActivityInterval interval, + List vehicleUsageIntervals + ) { + Set cutPoints = new TreeSet<>(); + cutPoints.add(interval.from()); + cutPoints.add(interval.to()); + + for (ExtractedCardVehicleUsageInterval usage : vehicleUsageIntervals) { + if (!usage.to().isBefore(interval.from()) && !usage.from().isAfter(interval.to())) { + if (usage.from().isAfter(interval.from()) && usage.from().isBefore(interval.to())) { + cutPoints.add(usage.from()); + } + OffsetDateTime usageEndExclusive = usage.to().plusSeconds(1); + if (usageEndExclusive.isAfter(interval.from()) && usageEndExclusive.isBefore(interval.to())) { + cutPoints.add(usageEndExclusive); + } + } + } + + List orderedCutPoints = List.copyOf(cutPoints); + List segments = new ArrayList<>(Math.max(1, orderedCutPoints.size() - 1)); + for (int i = 0; i < orderedCutPoints.size() - 1; i++) { + OffsetDateTime segmentFrom = orderedCutPoints.get(i); + OffsetDateTime segmentTo = orderedCutPoints.get(i + 1); + if (!segmentFrom.isBefore(segmentTo)) { + continue; + } + ExtractedCardVehicleUsageInterval covering = findVehicleCoverage(segmentFrom, vehicleUsageIntervals); + String intervalId = orderedCutPoints.size() == 2 ? interval.intervalId() : interval.intervalId() + "-" + (i + 1); + segments.add(new ExtractedCardActivityInterval( + intervalId, + segmentFrom, + segmentTo, interval.activityType(), interval.slot(), interval.cardStatus(), @@ -292,7 +324,17 @@ public class DriverCardXmlExtractionService { interval.rawRecordPath() )); } - return result; + return segments; + } + + private ExtractedCardVehicleUsageInterval findVehicleCoverage( + OffsetDateTime timestamp, + List vehicleUsageIntervals + ) { + return vehicleUsageIntervals.stream() + .filter(usage -> !usage.from().isAfter(timestamp) && timestamp.isBefore(usage.to().plusSeconds(1))) + .findFirst() + .orElse(null); } private Element firstElement(Object node, String expression) { diff --git a/src/main/java/at/procon/eventhub/tachographfilesession/service/LegalRequirementsClient.java b/src/main/java/at/procon/eventhub/tachographfilesession/service/LegalRequirementsClient.java index 6612aab..e049650 100644 --- a/src/main/java/at/procon/eventhub/tachographfilesession/service/LegalRequirementsClient.java +++ b/src/main/java/at/procon/eventhub/tachographfilesession/service/LegalRequirementsClient.java @@ -71,9 +71,11 @@ public class LegalRequirementsClient { throw new LegalRequirementsUploadException("LegalRequirements upload response did not contain DataPackageID."); } return dataPackageId; - } catch (IOException | InterruptedException e) { + } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new LegalRequirementsUploadException("Failed to upload tachograph file to LegalRequirements.", e); + } catch (IOException e) { + throw new LegalRequirementsUploadException("Failed to upload tachograph file to LegalRequirements.", e); } } @@ -88,9 +90,11 @@ public class LegalRequirementsClient { throw new LegalRequirementsXmlDownloadException("LegalRequirements XML download failed with status " + response.statusCode()); } return response.body(); - } catch (IOException | InterruptedException e) { + } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new LegalRequirementsXmlDownloadException("Failed to download processed tachograph XML.", e); + } catch (IOException e) { + throw new LegalRequirementsXmlDownloadException("Failed to download processed tachograph XML.", e); } } @@ -101,8 +105,9 @@ public class LegalRequirementsClient { .POST(HttpRequest.BodyPublishers.noBody()) .build(); client.send(request, HttpResponse.BodyHandlers.discarding()); - } catch (IOException | InterruptedException ignored) { + } catch (InterruptedException ignored) { Thread.currentThread().interrupt(); + } catch (IOException ignored) { } } diff --git a/src/main/java/at/procon/eventhub/tachographfilesession/service/TachographFileSessionService.java b/src/main/java/at/procon/eventhub/tachographfilesession/service/TachographFileSessionService.java index 69aa842..51633cf 100644 --- a/src/main/java/at/procon/eventhub/tachographfilesession/service/TachographFileSessionService.java +++ b/src/main/java/at/procon/eventhub/tachographfilesession/service/TachographFileSessionService.java @@ -51,8 +51,9 @@ public class TachographFileSessionService { String sessionLabel ) { try { + validateFile(file); byte[] fileBytes = file.getBytes(); - validateFile(file, fileBytes); + validateFileBytes(fileBytes); LegalRequirementsUploadResult uploadResult = legalRequirementsClient.uploadDriverCard(fileBytes, file.getOriginalFilename()); TachographXmlParser.ParsedTachographXml parsedXml = tachographXmlParser.parseDriverCardXml(uploadResult.xmlContent()); Instant createdAt = Instant.now(); @@ -148,8 +149,17 @@ public class TachographFileSessionService { .toList(); } - private void validateFile(MultipartFile file, byte[] fileBytes) { - if (file == null || file.isEmpty() || fileBytes.length == 0) { + private void validateFile(MultipartFile file) { + if (file == null || file.isEmpty() || file.getSize() == 0L) { + throw new IllegalArgumentException("Tachograph file must not be empty."); + } + if (file.getSize() > properties.getTachographFileSession().getMaxFileSizeBytes()) { + throw new IllegalArgumentException("Tachograph file exceeds the configured size limit."); + } + } + + private void validateFileBytes(byte[] fileBytes) { + if (fileBytes == null || fileBytes.length == 0) { throw new IllegalArgumentException("Tachograph file must not be empty."); } if (fileBytes.length > properties.getTachographFileSession().getMaxFileSizeBytes()) { diff --git a/src/main/java/at/procon/eventhub/tachographfilesession/service/TachographXmlParser.java b/src/main/java/at/procon/eventhub/tachographfilesession/service/TachographXmlParser.java index 2cdf12c..b0ffd76 100644 --- a/src/main/java/at/procon/eventhub/tachographfilesession/service/TachographXmlParser.java +++ b/src/main/java/at/procon/eventhub/tachographfilesession/service/TachographXmlParser.java @@ -33,6 +33,14 @@ public class TachographXmlParser { validate(xmlContent); DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setNamespaceAware(false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + factory.setXIncludeAware(false); + factory.setExpandEntityReferences(false); DocumentBuilder builder = factory.newDocumentBuilder(); Document document = builder.parse(new InputSource(new StringReader(xmlContent))); String rootName = document.getDocumentElement() == null ? null : document.getDocumentElement().getNodeName(); diff --git a/src/test/java/at/procon/eventhub/tachographfilesession/service/DriverCardXmlExtractionServiceTest.java b/src/test/java/at/procon/eventhub/tachographfilesession/service/DriverCardXmlExtractionServiceTest.java index a0cd1a9..c874953 100644 --- a/src/test/java/at/procon/eventhub/tachographfilesession/service/DriverCardXmlExtractionServiceTest.java +++ b/src/test/java/at/procon/eventhub/tachographfilesession/service/DriverCardXmlExtractionServiceTest.java @@ -45,8 +45,11 @@ class DriverCardXmlExtractionServiceTest { assertThat(driver.vehicleRegistrations()).hasSize(2); assertThat(driver.vehicles()).hasSize(2); assertThat(driver.cardVehicleUsageIntervals()).hasSize(2); - assertThat(driver.cardActivityIntervals()).hasSize(3); + assertThat(driver.cardActivityIntervals()).hasSize(5); assertThat(driver.cardActivityIntervals().get(0).registrationKey()).isEqualTo("12:W-12345A"); + assertThat(driver.cardActivityIntervals().get(1).to()).isEqualTo(driver.cardVehicleUsageIntervals().get(0).to().plusSeconds(1)); assertThat(driver.cardActivityIntervals().get(2).registrationKey()).isEqualTo("12:W-54321B"); + assertThat(driver.cardActivityIntervals().get(3).registrationKey()).isEqualTo("12:W-54321B"); + assertThat(driver.cardActivityIntervals().get(4).registrationKey()).isNull(); } } diff --git a/src/test/java/at/procon/eventhub/tachographfilesession/service/TachographFileSessionServiceTest.java b/src/test/java/at/procon/eventhub/tachographfilesession/service/TachographFileSessionServiceTest.java index 8613045..aeff6cb 100644 --- a/src/test/java/at/procon/eventhub/tachographfilesession/service/TachographFileSessionServiceTest.java +++ b/src/test/java/at/procon/eventhub/tachographfilesession/service/TachographFileSessionServiceTest.java @@ -1,8 +1,10 @@ package at.procon.eventhub.tachographfilesession.service; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import at.procon.eventhub.config.EventHubProperties; @@ -49,4 +51,23 @@ class TachographFileSessionServiceTest { assertThat(response.session().sessionId()).isEqualTo(extracted.sessionId()); assertThat(service.getSession(extracted.sessionId()).sessionId()).isEqualTo(extracted.sessionId()); } + + @Test + void rejectsOversizedFileBeforeUpload() { + EventHubProperties properties = new EventHubProperties(); + properties.getTachographFileSession().setMaxFileSizeBytes(1024); + TachographFileSessionRepository repository = new InMemoryTachographFileSessionRepository(properties); + LegalRequirementsClient client = Mockito.mock(LegalRequirementsClient.class); + TachographXmlParser parser = Mockito.mock(TachographXmlParser.class); + DriverCardXmlExtractionService extractor = Mockito.mock(DriverCardXmlExtractionService.class); + TachographFileSessionService service = new TachographFileSessionService(properties, repository, client, parser, extractor); + + MockMultipartFile file = new MockMultipartFile("file", "sample.ddd", "application/octet-stream", new byte[1025]); + + assertThatThrownBy(() -> service.createSession(file, null, null, "sample")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("size limit"); + + verifyNoInteractions(client, parser, extractor); + } } diff --git a/src/test/java/at/procon/eventhub/tachographfilesession/service/TachographXmlParserTest.java b/src/test/java/at/procon/eventhub/tachographfilesession/service/TachographXmlParserTest.java index aab2085..17c72c9 100644 --- a/src/test/java/at/procon/eventhub/tachographfilesession/service/TachographXmlParserTest.java +++ b/src/test/java/at/procon/eventhub/tachographfilesession/service/TachographXmlParserTest.java @@ -24,4 +24,17 @@ class TachographXmlParserTest { assertThatThrownBy(() -> parser.parseDriverCardXml(invalid)) .isInstanceOf(TachographXmlValidationException.class); } + + @Test + void rejectsXmlWithDoctype() { + String xmlWithDoctype = """ + + ]> + + """; + + assertThatThrownBy(() -> parser.parseDriverCardXml(xmlWithDoctype)) + .isInstanceOf(TachographXmlValidationException.class); + } }