mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2025-08-27 06:39:24 +00:00
AOP Fixes
This commit is contained in:
parent
bbf5d5f6d4
commit
ae3ed72283
@ -10,6 +10,8 @@ import org.aspectj.lang.ProceedingJoinPoint;
|
|||||||
import org.aspectj.lang.annotation.*;
|
import org.aspectj.lang.annotation.*;
|
||||||
import org.springframework.core.annotation.Order;
|
import org.springframework.core.annotation.Order;
|
||||||
import org.springframework.stereotype.Component;
|
import org.springframework.stereotype.Component;
|
||||||
|
import org.springframework.web.context.request.RequestContextHolder;
|
||||||
|
import org.springframework.web.context.request.ServletRequestAttributes;
|
||||||
import org.springframework.web.multipart.MultipartFile;
|
import org.springframework.web.multipart.MultipartFile;
|
||||||
|
|
||||||
import jakarta.servlet.http.HttpServletRequest;
|
import jakarta.servlet.http.HttpServletRequest;
|
||||||
@ -33,7 +35,6 @@ public class AutoJobAspect {
|
|||||||
private static final Duration RETRY_BASE_DELAY = Duration.ofMillis(100);
|
private static final Duration RETRY_BASE_DELAY = Duration.ofMillis(100);
|
||||||
|
|
||||||
private final JobExecutorService jobExecutorService;
|
private final JobExecutorService jobExecutorService;
|
||||||
private final HttpServletRequest request;
|
|
||||||
private final FileOrUploadService fileOrUploadService;
|
private final FileOrUploadService fileOrUploadService;
|
||||||
private final FileStorage fileStorage;
|
private final FileStorage fileStorage;
|
||||||
|
|
||||||
@ -42,7 +43,17 @@ public class AutoJobAspect {
|
|||||||
ProceedingJoinPoint joinPoint, AutoJobPostMapping autoJobPostMapping) {
|
ProceedingJoinPoint joinPoint, AutoJobPostMapping autoJobPostMapping) {
|
||||||
// This aspect will run before any audit aspects due to @Order(0)
|
// This aspect will run before any audit aspects due to @Order(0)
|
||||||
// Extract parameters from the request and annotation
|
// Extract parameters from the request and annotation
|
||||||
boolean async = Boolean.parseBoolean(request.getParameter("async"));
|
boolean async = false;
|
||||||
|
try {
|
||||||
|
ServletRequestAttributes attrs = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
|
||||||
|
if (attrs != null) {
|
||||||
|
HttpServletRequest request = attrs.getRequest();
|
||||||
|
async = Boolean.parseBoolean(request.getParameter("async"));
|
||||||
|
log.debug("AutoJobAspect: Processing {} {} with async={}", request.getMethod(), request.getRequestURI(), async);
|
||||||
|
}
|
||||||
|
} catch (Exception e) {
|
||||||
|
log.debug("Could not retrieve async parameter from request: {}", e.getMessage());
|
||||||
|
}
|
||||||
long timeout = autoJobPostMapping.timeout();
|
long timeout = autoJobPostMapping.timeout();
|
||||||
int retryCount = autoJobPostMapping.retryCount();
|
int retryCount = autoJobPostMapping.retryCount();
|
||||||
boolean trackProgress = autoJobPostMapping.trackProgress();
|
boolean trackProgress = autoJobPostMapping.trackProgress();
|
||||||
@ -54,19 +65,8 @@ public class AutoJobAspect {
|
|||||||
retryCount,
|
retryCount,
|
||||||
trackProgress);
|
trackProgress);
|
||||||
|
|
||||||
// Copy and process arguments
|
// Process arguments in-place to avoid type mismatch issues
|
||||||
// In a test environment, we might need to update the original objects for verification
|
Object[] args = processArgsInPlace(joinPoint.getArgs(), async);
|
||||||
boolean isTestEnvironment = false;
|
|
||||||
try {
|
|
||||||
isTestEnvironment = Class.forName("org.junit.jupiter.api.Test") != null;
|
|
||||||
} catch (ClassNotFoundException e) {
|
|
||||||
// Not in a test environment
|
|
||||||
}
|
|
||||||
|
|
||||||
Object[] args =
|
|
||||||
isTestEnvironment
|
|
||||||
? processArgsInPlace(joinPoint.getArgs(), async)
|
|
||||||
: copyAndProcessArgs(joinPoint.getArgs(), async);
|
|
||||||
|
|
||||||
// Extract queueable and resourceWeight parameters and validate
|
// Extract queueable and resourceWeight parameters and validate
|
||||||
boolean queueable = autoJobPostMapping.queueable();
|
boolean queueable = autoJobPostMapping.queueable();
|
||||||
@ -229,79 +229,10 @@ public class AutoJobAspect {
|
|||||||
resourceWeight);
|
resourceWeight);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Creates deep copies of arguments when needed to avoid mutating the original objects
|
|
||||||
* Particularly important for PDFFile objects that might be reused by Spring
|
|
||||||
*
|
|
||||||
* @param originalArgs The original arguments
|
|
||||||
* @param async Whether this is an async operation
|
|
||||||
* @return A new array with safely processed arguments
|
|
||||||
*/
|
|
||||||
private Object[] copyAndProcessArgs(Object[] originalArgs, boolean async) {
|
|
||||||
if (originalArgs == null || originalArgs.length == 0) {
|
|
||||||
return originalArgs;
|
|
||||||
}
|
|
||||||
|
|
||||||
Object[] processedArgs = new Object[originalArgs.length];
|
|
||||||
|
|
||||||
// Copy all arguments
|
|
||||||
for (int i = 0; i < originalArgs.length; i++) {
|
|
||||||
Object arg = originalArgs[i];
|
|
||||||
|
|
||||||
if (arg instanceof PDFFile pdfFile) {
|
|
||||||
// Create a copy of PDFFile to avoid mutating the original
|
|
||||||
// Using direct property access instead of reflection for better performance
|
|
||||||
PDFFile pdfFileCopy = new PDFFile();
|
|
||||||
pdfFileCopy.setFileId(pdfFile.getFileId());
|
|
||||||
pdfFileCopy.setFileInput(pdfFile.getFileInput());
|
|
||||||
|
|
||||||
// Case 1: fileId is provided but no fileInput
|
|
||||||
if (pdfFileCopy.getFileInput() == null && pdfFileCopy.getFileId() != null) {
|
|
||||||
try {
|
|
||||||
log.debug("Using fileId {} to get file content", pdfFileCopy.getFileId());
|
|
||||||
MultipartFile file = fileStorage.retrieveFile(pdfFileCopy.getFileId());
|
|
||||||
pdfFileCopy.setFileInput(file);
|
|
||||||
} catch (Exception e) {
|
|
||||||
throw new RuntimeException(
|
|
||||||
"Failed to resolve file by ID: " + pdfFileCopy.getFileId(), e);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
// Case 2: For async requests, we need to make a copy of the MultipartFile
|
|
||||||
else if (async && pdfFileCopy.getFileInput() != null) {
|
|
||||||
try {
|
|
||||||
log.debug("Making persistent copy of uploaded file for async processing");
|
|
||||||
MultipartFile originalFile = pdfFileCopy.getFileInput();
|
|
||||||
String fileId = fileStorage.storeFile(originalFile);
|
|
||||||
|
|
||||||
// Store the fileId for later reference
|
|
||||||
pdfFileCopy.setFileId(fileId);
|
|
||||||
|
|
||||||
// Replace the original MultipartFile with our persistent copy
|
|
||||||
MultipartFile persistentFile = fileStorage.retrieveFile(fileId);
|
|
||||||
pdfFileCopy.setFileInput(persistentFile);
|
|
||||||
|
|
||||||
log.debug("Created persistent file copy with fileId: {}", fileId);
|
|
||||||
} catch (IOException e) {
|
|
||||||
throw new RuntimeException(
|
|
||||||
"Failed to create persistent copy of uploaded file", e);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
processedArgs[i] = pdfFileCopy;
|
|
||||||
} else {
|
|
||||||
// For non-PDFFile objects, just pass the original reference
|
|
||||||
// If other classes need copy-on-write, add them here
|
|
||||||
processedArgs[i] = arg;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return processedArgs;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Processes arguments in-place for testing purposes This is similar to our original
|
* Processes arguments in-place to handle file resolution and async file persistence.
|
||||||
* implementation before introducing copy-on-write It's only used in test environments to
|
* This approach avoids type mismatch issues by modifying the original objects directly.
|
||||||
* maintain test compatibility
|
|
||||||
*
|
*
|
||||||
* @param originalArgs The original arguments
|
* @param originalArgs The original arguments
|
||||||
* @param async Whether this is an async operation
|
* @param async Whether this is an async operation
|
||||||
@ -356,10 +287,14 @@ public class AutoJobAspect {
|
|||||||
|
|
||||||
private String getJobIdFromContext() {
|
private String getJobIdFromContext() {
|
||||||
try {
|
try {
|
||||||
return (String) request.getAttribute("jobId");
|
ServletRequestAttributes attrs = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
|
||||||
|
if (attrs != null) {
|
||||||
|
HttpServletRequest request = attrs.getRequest();
|
||||||
|
return (String) request.getAttribute("jobId");
|
||||||
|
}
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
log.debug("Could not retrieve job ID from context: {}", e.getMessage());
|
log.debug("Could not retrieve job ID from context: {}", e.getMessage());
|
||||||
return null;
|
|
||||||
}
|
}
|
||||||
|
return null;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -6,6 +6,8 @@ import java.util.Collections;
|
|||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.concurrent.CopyOnWriteArrayList;
|
import java.util.concurrent.CopyOnWriteArrayList;
|
||||||
|
|
||||||
|
import com.fasterxml.jackson.annotation.JsonIgnore;
|
||||||
|
|
||||||
import lombok.AllArgsConstructor;
|
import lombok.AllArgsConstructor;
|
||||||
import lombok.Builder;
|
import lombok.Builder;
|
||||||
import lombok.Data;
|
import lombok.Data;
|
||||||
@ -28,6 +30,7 @@ public class JobResult {
|
|||||||
private String error;
|
private String error;
|
||||||
|
|
||||||
/** List of result files for jobs that produce files */
|
/** List of result files for jobs that produce files */
|
||||||
|
@JsonIgnore
|
||||||
private List<ResultFile> resultFiles;
|
private List<ResultFile> resultFiles;
|
||||||
|
|
||||||
/** Time when the job was created */
|
/** Time when the job was created */
|
||||||
|
@ -14,6 +14,8 @@ import org.springframework.http.HttpHeaders;
|
|||||||
import org.springframework.http.MediaType;
|
import org.springframework.http.MediaType;
|
||||||
import org.springframework.http.ResponseEntity;
|
import org.springframework.http.ResponseEntity;
|
||||||
import org.springframework.stereotype.Service;
|
import org.springframework.stereotype.Service;
|
||||||
|
import org.springframework.web.context.request.RequestContextHolder;
|
||||||
|
import org.springframework.web.context.request.ServletRequestAttributes;
|
||||||
import org.springframework.web.multipart.MultipartFile;
|
import org.springframework.web.multipart.MultipartFile;
|
||||||
|
|
||||||
import jakarta.servlet.http.HttpServletRequest;
|
import jakarta.servlet.http.HttpServletRequest;
|
||||||
@ -30,7 +32,6 @@ public class JobExecutorService {
|
|||||||
|
|
||||||
private final TaskManager taskManager;
|
private final TaskManager taskManager;
|
||||||
private final FileStorage fileStorage;
|
private final FileStorage fileStorage;
|
||||||
private final HttpServletRequest request;
|
|
||||||
private final ResourceMonitor resourceMonitor;
|
private final ResourceMonitor resourceMonitor;
|
||||||
private final JobQueue jobQueue;
|
private final JobQueue jobQueue;
|
||||||
private final ExecutorService executor = ExecutorFactory.newVirtualOrCachedThreadExecutor();
|
private final ExecutorService executor = ExecutorFactory.newVirtualOrCachedThreadExecutor();
|
||||||
@ -39,14 +40,12 @@ public class JobExecutorService {
|
|||||||
public JobExecutorService(
|
public JobExecutorService(
|
||||||
TaskManager taskManager,
|
TaskManager taskManager,
|
||||||
FileStorage fileStorage,
|
FileStorage fileStorage,
|
||||||
HttpServletRequest request,
|
|
||||||
ResourceMonitor resourceMonitor,
|
ResourceMonitor resourceMonitor,
|
||||||
JobQueue jobQueue,
|
JobQueue jobQueue,
|
||||||
@Value("${spring.mvc.async.request-timeout:1200000}") long asyncRequestTimeoutMs,
|
@Value("${spring.mvc.async.request-timeout:1200000}") long asyncRequestTimeoutMs,
|
||||||
@Value("${server.servlet.session.timeout:30m}") String sessionTimeout) {
|
@Value("${server.servlet.session.timeout:30m}") String sessionTimeout) {
|
||||||
this.taskManager = taskManager;
|
this.taskManager = taskManager;
|
||||||
this.fileStorage = fileStorage;
|
this.fileStorage = fileStorage;
|
||||||
this.request = request;
|
|
||||||
this.resourceMonitor = resourceMonitor;
|
this.resourceMonitor = resourceMonitor;
|
||||||
this.jobQueue = jobQueue;
|
this.jobQueue = jobQueue;
|
||||||
|
|
||||||
@ -100,24 +99,30 @@ public class JobExecutorService {
|
|||||||
String jobId = UUID.randomUUID().toString();
|
String jobId = UUID.randomUUID().toString();
|
||||||
|
|
||||||
// Store the job ID in the request for potential use by other components
|
// Store the job ID in the request for potential use by other components
|
||||||
if (request != null) {
|
try {
|
||||||
request.setAttribute("jobId", jobId);
|
ServletRequestAttributes attrs = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
|
||||||
|
if (attrs != null) {
|
||||||
|
HttpServletRequest request = attrs.getRequest();
|
||||||
|
request.setAttribute("jobId", jobId);
|
||||||
|
|
||||||
// Also track this job ID in the user's session for authorization purposes
|
// Also track this job ID in the user's session for authorization purposes
|
||||||
// This ensures users can only cancel their own jobs
|
// This ensures users can only cancel their own jobs
|
||||||
if (request.getSession() != null) {
|
if (request.getSession() != null) {
|
||||||
@SuppressWarnings("unchecked")
|
@SuppressWarnings("unchecked")
|
||||||
java.util.Set<String> userJobIds =
|
java.util.Set<String> userJobIds =
|
||||||
(java.util.Set<String>) request.getSession().getAttribute("userJobIds");
|
(java.util.Set<String>) request.getSession().getAttribute("userJobIds");
|
||||||
|
|
||||||
if (userJobIds == null) {
|
if (userJobIds == null) {
|
||||||
userJobIds = new java.util.concurrent.ConcurrentSkipListSet<>();
|
userJobIds = new java.util.concurrent.ConcurrentSkipListSet<>();
|
||||||
request.getSession().setAttribute("userJobIds", userJobIds);
|
request.getSession().setAttribute("userJobIds", userJobIds);
|
||||||
|
}
|
||||||
|
|
||||||
|
userJobIds.add(jobId);
|
||||||
|
log.debug("Added job ID {} to user session", jobId);
|
||||||
}
|
}
|
||||||
|
|
||||||
userJobIds.add(jobId);
|
|
||||||
log.debug("Added job ID {} to user session", jobId);
|
|
||||||
}
|
}
|
||||||
|
} catch (Exception e) {
|
||||||
|
log.debug("Could not store job ID in request context: {}", e.getMessage());
|
||||||
}
|
}
|
||||||
|
|
||||||
// Determine which timeout to use
|
// Determine which timeout to use
|
||||||
|
@ -1,6 +1,5 @@
|
|||||||
package stirling.software.common.service;
|
package stirling.software.common.service;
|
||||||
|
|
||||||
import io.github.pixee.security.ZipSecurity;
|
|
||||||
import java.io.ByteArrayInputStream;
|
import java.io.ByteArrayInputStream;
|
||||||
import java.io.ByteArrayOutputStream;
|
import java.io.ByteArrayOutputStream;
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
@ -21,6 +20,8 @@ import org.springframework.http.MediaType;
|
|||||||
import org.springframework.stereotype.Service;
|
import org.springframework.stereotype.Service;
|
||||||
import org.springframework.web.multipart.MultipartFile;
|
import org.springframework.web.multipart.MultipartFile;
|
||||||
|
|
||||||
|
import io.github.pixee.security.ZipSecurity;
|
||||||
|
|
||||||
import jakarta.annotation.PreDestroy;
|
import jakarta.annotation.PreDestroy;
|
||||||
|
|
||||||
import lombok.extern.slf4j.Slf4j;
|
import lombok.extern.slf4j.Slf4j;
|
||||||
@ -361,7 +362,8 @@ public class TaskManager {
|
|||||||
MultipartFile zipFile = fileStorage.retrieveFile(zipFileId);
|
MultipartFile zipFile = fileStorage.retrieveFile(zipFileId);
|
||||||
|
|
||||||
try (ZipInputStream zipIn =
|
try (ZipInputStream zipIn =
|
||||||
ZipSecurity.createHardenedInputStream(new ByteArrayInputStream(zipFile.getBytes()))) {
|
ZipSecurity.createHardenedInputStream(
|
||||||
|
new ByteArrayInputStream(zipFile.getBytes()))) {
|
||||||
ZipEntry entry;
|
ZipEntry entry;
|
||||||
while ((entry = zipIn.getNextEntry()) != null) {
|
while ((entry = zipIn.getNextEntry()) != null) {
|
||||||
if (!entry.isDirectory()) {
|
if (!entry.isDirectory()) {
|
||||||
|
@ -29,7 +29,8 @@ public class CleanUrlInterceptor implements HandlerInterceptor {
|
|||||||
"type",
|
"type",
|
||||||
"principal",
|
"principal",
|
||||||
"startDate",
|
"startDate",
|
||||||
"endDate");
|
"endDate",
|
||||||
|
"async");
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public boolean preHandle(
|
public boolean preHandle(
|
||||||
|
Loading…
x
Reference in New Issue
Block a user