fix: File format check #239
Conversation
…a into lu/develop
…a into lu/develop
WalkthroughTwo new enums for file types and file extensions were added. File type and content validation logic was introduced or strengthened in several controllers and service methods, particularly for JSON files. A utility method for validating JSON file content was implemented, and related test cases were updated to use valid JSON input. Additionally, a file type check method was simplified by changing its parameter to accept multiple allowed MIME types per extension. Some redundant file checks were removed from service implementations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller
participant SecurityFileCheckUtil
participant Service
User->>Controller: Upload file (e.g., JSON)
Controller->>SecurityFileCheckUtil: checkFileType(file, allowedTypes)
Controller->>SecurityFileCheckUtil: isValidJson(file)
SecurityFileCheckUtil-->>Controller: Validation result / Exception
Controller->>Service: Process validated file
Service->>SecurityFileCheckUtil: isValidJson(file) (if not already checked)
SecurityFileCheckUtil-->>Service: Validation result / Exception
Service-->>Controller: Process result
Controller-->>User: Response
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
🔇 Additional comments (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
base/src/main/java/com/tinyengine/it/common/enums/Enums.java (1)
957-1000: Consider JPG extension consistency.The
FileNameEnd.JPGuses ".jpeg" extension while the typical extension is ".jpg". Both are valid, but ensure this aligns with your application's file naming conventions.Otherwise, the enum is well-structured and provides a clean way to manage file extension constants.
base/src/main/java/com/tinyengine/it/common/utils/SecurityFileCheckUtil.java (1)
182-190: Consider input stream consumption implications.The JSON validation method is well-implemented, but consider that
file.getInputStream()consumes the stream. Ensure downstream code doesn't need to read the stream again, or consider usingfile.getBytes()instead.Alternative implementation to avoid stream consumption:
public static void isValidJson(MultipartFile file) { ObjectMapper objectMapper = new ObjectMapper(); try { - objectMapper.readTree(file.getInputStream()); + objectMapper.readTree(file.getBytes()); } catch (IOException e) { throw new ServiceException(ExceptionEnum.CM308.getResultCode(), ExceptionEnum.CM308.getResultMsg()); } }The current implementation is acceptable if stream reuse isn't required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
base/src/main/java/com/tinyengine/it/common/enums/Enums.java(1 hunks)base/src/main/java/com/tinyengine/it/common/utils/SecurityFileCheckUtil.java(2 hunks)base/src/main/java/com/tinyengine/it/controller/ComponentController.java(3 hunks)base/src/main/java/com/tinyengine/it/controller/I18nEntryController.java(4 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/I18nEntryServiceImpl.java(3 hunks)base/src/test/java/com/tinyengine/it/service/app/impl/I18nEntryServiceImplTest.java(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
base/src/main/java/com/tinyengine/it/controller/ComponentController.java (2)
base/src/main/java/com/tinyengine/it/common/enums/Enums.java (1)
Enums(20-1001)base/src/main/java/com/tinyengine/it/common/utils/SecurityFileCheckUtil.java (1)
SecurityFileCheckUtil(33-192)
base/src/main/java/com/tinyengine/it/controller/I18nEntryController.java (2)
base/src/main/java/com/tinyengine/it/common/enums/Enums.java (1)
Enums(20-1001)base/src/main/java/com/tinyengine/it/common/utils/SecurityFileCheckUtil.java (1)
SecurityFileCheckUtil(33-192)
base/src/main/java/com/tinyengine/it/service/app/impl/I18nEntryServiceImpl.java (2)
base/src/main/java/com/tinyengine/it/common/utils/SecurityFileCheckUtil.java (1)
SecurityFileCheckUtil(33-192)base/src/main/java/com/tinyengine/it/common/enums/Enums.java (1)
Enums(20-1001)
🔇 Additional comments (11)
base/src/main/java/com/tinyengine/it/common/enums/Enums.java (1)
912-955: LGTM! Well-structured enum for file MIME types.The
FileTypeenum provides a clean, centralized way to manage MIME type constants for file validation. All MIME types are standard and correctly defined.base/src/test/java/com/tinyengine/it/service/app/impl/I18nEntryServiceImplTest.java (2)
287-287: Good alignment of test data with JSON validation.Updating the mocked input stream to match the JSON content returned by
getBytes()ensures consistency and supports the new JSON validation logic.
302-302: Consistent test data alignment.Good consistency in updating test data to support JSON validation across multiple test methods.
base/src/main/java/com/tinyengine/it/controller/ComponentController.java (1)
16-16: Appropriate import for enum usage.The import addition supports the new file type validation logic using the centralized enums.
base/src/main/java/com/tinyengine/it/common/utils/SecurityFileCheckUtil.java (1)
15-15: Appropriate imports for JSON validation.The Jackson ObjectMapper and IOException imports correctly support the new JSON validation functionality.
Also applies to: 22-22
base/src/main/java/com/tinyengine/it/service/app/impl/I18nEntryServiceImpl.java (3)
24-24: LGTM: Import added for security validationThe import for
SecurityFileCheckUtilis correctly added to support the new JSON validation functionality.
330-331: Good: Enhanced JSON file validation with correct enum usageThe changes improve security by:
- Correcting the enum usage from
MimeTypetoFileType(line 330)- Adding JSON content validation before processing (line 331)
The
SecurityFileCheckUtil.isValidJson(file)call ensures the uploaded JSON file has valid content structure, which prevents processing malformed JSON that could cause runtime errors.
362-362: Consistent JSON validation implementationThe JSON validation is appropriately placed at the beginning of the method before any file processing occurs, ensuring early validation and consistent error handling across the codebase.
base/src/main/java/com/tinyengine/it/controller/I18nEntryController.java (3)
16-16: LGTM: Necessary imports for enhanced file validationThe imports for
EnumsandHashMapare correctly added to support the new file type validation functionality.Also applies to: 50-50
253-257: Robust file type validation implementationThe implementation creates a comprehensive validation approach by:
- Mapping file extensions to their MIME types using the new enums
- Validating both filename and content type through
SecurityFileCheckUtil.checkFileTypeThis provides defense-in-depth by checking both the file extension and the actual MIME type, preventing potential security issues from file type spoofing.
294-294: Consistent JSON-specific validationThe direct validation for JSON files using
SecurityFileCheckUtil.checkFileTypewith specific JSON parameters maintains consistency with the validation approach while being optimized for the single file type expected in this method.
base/src/main/java/com/tinyengine/it/controller/ComponentController.java
Outdated
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/controller/ComponentController.java
Outdated
Show resolved
Hide resolved
2 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit