mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-03 05:10:16 +03:00
refactor(api): replace regex literals with compiled patterns for improved performance and readability (#6511)
# Description of Changes This pull request refactors several utility classes and controllers to replace inline regular expression usage with precompiled `Pattern` constants. This change improves performance, consistency, and maintainability by ensuring that regex patterns are compiled only once and reused throughout the codebase. Additionally, it enhances code clarity and security in filename and SQL content sanitization. <!-- Please provide a summary of the changes, including: - What was changed - Why the change was made - Any challenges encountered Closes #(issue_number) --> --- ## Checklist ### General - [X] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [X] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [X] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [X] I have performed a self-review of my own code - [X] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [X] I have run `task check` to verify linters, typechecks, and tests pass - [X] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details. --------- Co-authored-by: Anthony Stirling <77850077+Frooodle@users.noreply.github.com>
This commit is contained in:
co-authored by
Anthony Stirling
parent
a4ffdc7831
commit
e2ea720fc8
@@ -5,6 +5,7 @@ import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Locale;
|
||||
import java.util.Optional;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.apache.pdfbox.pdmodel.PDDocument;
|
||||
import org.apache.pdfbox.pdmodel.common.PDRectangle;
|
||||
@@ -28,6 +29,8 @@ import lombok.extern.slf4j.Slf4j;
|
||||
@Component
|
||||
public class PdfTextLocator {
|
||||
|
||||
private static final Pattern NON_ALPHANUMERIC_PATTERN = Pattern.compile("[^A-Za-z0-9]");
|
||||
|
||||
/** One found line of text with its user-space bounding box. */
|
||||
public record MatchedBox(float x, float y, float width, float height) {}
|
||||
|
||||
@@ -82,7 +85,7 @@ public class PdfTextLocator {
|
||||
|
||||
/** Strip everything non-alphanumeric and lowercase for tolerant matching. */
|
||||
private static String normalize(String s) {
|
||||
return s.replaceAll("[^A-Za-z0-9]", "").toLowerCase(Locale.ROOT);
|
||||
return NON_ALPHANUMERIC_PATTERN.matcher(s).replaceAll("").toLowerCase(Locale.ROOT);
|
||||
}
|
||||
|
||||
private static final class CapturedLine {
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
package stirling.software.common.util;
|
||||
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
public class RequestUriUtils {
|
||||
|
||||
private static final Pattern SHARE_LINK_PATTERN = Pattern.compile("^/share/[^/]+/?$");
|
||||
|
||||
public static boolean isStaticResource(String requestURI) {
|
||||
return isStaticResource("", requestURI);
|
||||
}
|
||||
@@ -202,7 +206,7 @@ public class RequestUriUtils {
|
||||
// Workflow participant endpoints - access controlled by share tokens, not login
|
||||
|| trimmedUri.startsWith("/api/v1/workflow/participant/")
|
||||
// Share-link SPA bootstrap; data APIs remain protected
|
||||
|| trimmedUri.matches("^/share/[^/]+/?$");
|
||||
|| SHARE_LINK_PATTERN.matcher(trimmedUri).matches();
|
||||
}
|
||||
|
||||
private static String stripContextPath(String contextPath, String requestURI) {
|
||||
|
||||
+5
-2
@@ -2,6 +2,7 @@ package stirling.software.proprietary.controller.api;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.math.BigDecimal;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.MediaType;
|
||||
@@ -46,6 +47,7 @@ import stirling.software.proprietary.service.MathAuditorOrchestrator;
|
||||
@Tag(name = "AI Tools", description = "Dispatchable AI-backed tools.")
|
||||
public class MathAuditorAgentController {
|
||||
|
||||
private static final Pattern NEWLINE_PATTERN = Pattern.compile("[\\r\\n]");
|
||||
private final MathAuditorOrchestrator orchestrator;
|
||||
|
||||
@PostMapping(value = "/math-auditor-agent", consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
|
||||
@@ -83,9 +85,10 @@ public class MathAuditorAgentController {
|
||||
return ResponseEntity.badRequest().build();
|
||||
}
|
||||
|
||||
String originalFilename = fileInput.getOriginalFilename();
|
||||
String safeName =
|
||||
fileInput.getOriginalFilename() != null
|
||||
? fileInput.getOriginalFilename().replaceAll("[\\r\\n]", "_")
|
||||
originalFilename != null
|
||||
? NEWLINE_PATTERN.matcher(originalFilename).replaceAll("_")
|
||||
: "<unnamed>";
|
||||
log.info("[math-auditor-agent] request file={} tolerance={}", safeName, tolerance);
|
||||
|
||||
|
||||
+5
-2
@@ -1,6 +1,7 @@
|
||||
package stirling.software.proprietary.controller.api;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.springframework.core.io.ByteArrayResource;
|
||||
import org.springframework.core.io.Resource;
|
||||
@@ -45,6 +46,7 @@ import tools.jackson.databind.node.ObjectNode;
|
||||
@Tag(name = "AI Tools", description = "Dispatchable AI-backed tools.")
|
||||
public class PdfCommentAgentController {
|
||||
|
||||
private static final Pattern NEWLINE_PATTERN = Pattern.compile("[\\r\\n]");
|
||||
private final PdfCommentAgentOrchestrator orchestrator;
|
||||
private final ObjectMapper objectMapper;
|
||||
|
||||
@@ -78,9 +80,10 @@ public class PdfCommentAgentController {
|
||||
String prompt)
|
||||
throws IOException {
|
||||
|
||||
String originalFilename = fileInput.getOriginalFilename();
|
||||
String safeName =
|
||||
fileInput.getOriginalFilename() != null
|
||||
? fileInput.getOriginalFilename().replaceAll("[\\r\\n]", "_")
|
||||
originalFilename != null
|
||||
? NEWLINE_PATTERN.matcher(originalFilename).replaceAll("_")
|
||||
: "<unnamed>";
|
||||
log.info(
|
||||
"[pdf-comment-agent] request file={} promptLen={}",
|
||||
|
||||
+14
-9
@@ -46,6 +46,9 @@ public class DatabaseService implements DatabaseServiceInterface {
|
||||
|
||||
public static final String BACKUP_PREFIX = "backup_";
|
||||
public static final String SQL_SUFFIX = ".sql";
|
||||
private static final Pattern WHITESPACE_PATTERN = Pattern.compile("\\s+");
|
||||
private static final Pattern LINE_COMMENT_PATTERN = Pattern.compile("--[^\r\n]*");
|
||||
private static final Pattern BLOCK_COMMENT_PATTERN = Pattern.compile("/\\*[\\s\\S]*?\\*/");
|
||||
private final Path BACKUP_DIR;
|
||||
|
||||
// Whitelist of allowed SQL patterns for H2 database backups (generated by SCRIPT command)
|
||||
@@ -520,7 +523,7 @@ public class DatabaseService implements DatabaseServiceInterface {
|
||||
private void validateSqlContent(Path scriptPath) {
|
||||
try {
|
||||
String content = Files.readString(scriptPath);
|
||||
String normalizedContent = normalizeSqlContent(content);
|
||||
String normalizedContent = sanitizeSql(content);
|
||||
|
||||
String codeOnly = stripStringLiterals(normalizedContent);
|
||||
for (Pattern deniedPattern : DENIED_PATTERNS) {
|
||||
@@ -568,19 +571,21 @@ public class DatabaseService implements DatabaseServiceInterface {
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalizes SQL content by removing comments to prevent bypass attacks.
|
||||
* Sanitize SQL content by removing comments to prevent bypass attacks.
|
||||
*
|
||||
* @param sql the SQL content to normalize
|
||||
* @return normalized SQL without comments
|
||||
* @param sql the SQL content to sanitize
|
||||
* @return sanitize SQL without comments
|
||||
*/
|
||||
private String normalizeSqlContent(String sql) {
|
||||
private String sanitizeSql(String sql) {
|
||||
// Remove block comments (/* ... */)
|
||||
sql = sql.replaceAll("/\\*[\\s\\S]*?\\*/", " ");
|
||||
// TODO: I feel like this should re-evaluated.
|
||||
// Passing around SQL like this, smells a bit when we have Hibernate/Critaria API.
|
||||
String intermediateSql = BLOCK_COMMENT_PATTERN.matcher(sql).replaceAll(" ");
|
||||
// Remove line comments (--....)
|
||||
sql = sql.replaceAll("--[^\r\n]*", " ");
|
||||
intermediateSql = LINE_COMMENT_PATTERN.matcher(intermediateSql).replaceAll(" ");
|
||||
// Collapse multiple whitespaces
|
||||
sql = sql.replaceAll("\\s+", " ");
|
||||
return sql.trim();
|
||||
intermediateSql = WHITESPACE_PATTERN.matcher(intermediateSql).replaceAll(" ");
|
||||
return intermediateSql.trim();
|
||||
}
|
||||
|
||||
private String stripStringLiterals(String sql) {
|
||||
|
||||
+4
-1
@@ -7,6 +7,7 @@ import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.UUID;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.apache.commons.io.FilenameUtils;
|
||||
import org.apache.pdfbox.pdmodel.PDDocument;
|
||||
@@ -62,6 +63,8 @@ public class PdfCommentAgentOrchestrator {
|
||||
/** Filename used when the uploaded PDF has no usable original filename. */
|
||||
private static final String FALLBACK_OUTPUT_NAME = "document-commented.pdf";
|
||||
|
||||
private static final Pattern NEWLINE_PATTERN = Pattern.compile("[\\r\\n]");
|
||||
|
||||
/**
|
||||
* Small value record returned to the controller: the annotated PDF bytes, the suggested
|
||||
* download filename (used in the {@code Content-Disposition} header), and metadata the
|
||||
@@ -263,7 +266,7 @@ public class PdfCommentAgentOrchestrator {
|
||||
|
||||
private static String safeName(String originalFilename) {
|
||||
return originalFilename != null
|
||||
? originalFilename.replaceAll("[\\r\\n]", "_")
|
||||
? NEWLINE_PATTERN.matcher(originalFilename).replaceAll("_")
|
||||
: "<unnamed>";
|
||||
}
|
||||
}
|
||||
|
||||
+7
-1
@@ -4,9 +4,11 @@ import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.nio.file.Paths;
|
||||
import java.nio.file.StandardCopyOption;
|
||||
import java.util.Optional;
|
||||
import java.util.UUID;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.springframework.core.io.FileSystemResource;
|
||||
import org.springframework.core.io.Resource;
|
||||
@@ -19,6 +21,7 @@ import stirling.software.proprietary.security.model.User;
|
||||
@RequiredArgsConstructor
|
||||
public class LocalStorageProvider implements StorageProvider {
|
||||
|
||||
private static final Pattern CONTROL_CHARACTER_PATTERN = Pattern.compile("\\p{Cntrl}");
|
||||
private final Path basePath;
|
||||
|
||||
@Override
|
||||
@@ -79,7 +82,10 @@ public class LocalStorageProvider implements StorageProvider {
|
||||
if (filename == null || filename.isBlank()) {
|
||||
return "file";
|
||||
}
|
||||
String stripped = Path.of(filename).getFileName().toString().replaceAll("\\p{Cntrl}", "");
|
||||
String stripped =
|
||||
CONTROL_CHARACTER_PATTERN
|
||||
.matcher(Paths.get(filename).getFileName().toString())
|
||||
.replaceAll("");
|
||||
return stripped.isBlank() ? "file" : stripped;
|
||||
}
|
||||
}
|
||||
|
||||
+8
-3
@@ -4,10 +4,11 @@ import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.net.URI;
|
||||
import java.net.URISyntaxException;
|
||||
import java.nio.file.Path;
|
||||
import java.nio.file.Paths;
|
||||
import java.time.Duration;
|
||||
import java.util.Optional;
|
||||
import java.util.UUID;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.springframework.core.io.InputStreamResource;
|
||||
import org.springframework.core.io.Resource;
|
||||
@@ -34,6 +35,7 @@ import software.amazon.awssdk.services.s3.presigner.model.PresignedGetObjectRequ
|
||||
@Slf4j
|
||||
public class S3StorageProvider implements StorageProvider, AutoCloseable {
|
||||
|
||||
private static final Pattern CONTROL_CHARACTER_PATTERN = Pattern.compile("\\p{Cntrl}");
|
||||
private final S3Client s3Client;
|
||||
private final S3Presigner s3Presigner;
|
||||
private final String bucket;
|
||||
@@ -152,7 +154,7 @@ public class S3StorageProvider implements StorageProvider, AutoCloseable {
|
||||
}
|
||||
// Strip CR/LF and other control chars before path parsing (Path.of throws on them on
|
||||
// Windows, and they defeat header parsers).
|
||||
String stripped = originalFilename.replaceAll("\\p{Cntrl}", "");
|
||||
String stripped = CONTROL_CHARACTER_PATTERN.matcher(originalFilename).replaceAll("");
|
||||
// Use only the basename to avoid leaking directory structure into the header.
|
||||
int lastSeparator = Math.max(stripped.lastIndexOf('/'), stripped.lastIndexOf('\\'));
|
||||
if (lastSeparator >= 0) {
|
||||
@@ -184,7 +186,10 @@ public class S3StorageProvider implements StorageProvider, AutoCloseable {
|
||||
if (filename == null || filename.isBlank()) {
|
||||
return "file";
|
||||
}
|
||||
String stripped = Path.of(filename).getFileName().toString().replaceAll("\\p{Cntrl}", "");
|
||||
String stripped =
|
||||
CONTROL_CHARACTER_PATTERN
|
||||
.matcher(Paths.get(filename).getFileName().toString())
|
||||
.replaceAll("");
|
||||
return stripped.isBlank() ? "file" : stripped;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user