Fix/resourceredirect #149
Fix/resourceredirect #149
Conversation
(cherry picked from commit c9f085d0d7d89dab1dac8d6f7a0614c11f49a0e8)
# Conflicts: # src/main/java/org/cip4/jdflib/generator/JavaCoreStringUtil.java # src/main/java/org/cip4/jdfutility/FileItemList.java # src/main/java/org/cip4/jdfutility/GetFileServlet.java
There was a problem hiding this comment.
Pull request overview
This PR addresses general cleanup and security improvements in the JDFUtility codebase. The changes include refactoring URL path handling for security, updating copyright dates, and improving code formatting.
Key changes:
- Enhanced URL path manipulation logic in
MyResourceHandlerto prevent potential path traversal vulnerabilities - Switched from HTML4 to HTML3 escaping for better compatibility
- Added new test coverage for resource handler and file item list components
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/cip4/jdfutility/server/MyResourceHandler.java | Improved URL path handling with JDFConstants.SLASH constant and fixed substring logic to prevent index out of bounds |
| src/main/java/org/cip4/jdfutility/GetFileServlet.java | Changed HTML escape from escapeHtml4 to escapeHtml3 and reformatted imports |
| src/main/java/org/cip4/jdfutility/FileItemList.java | Added FileCleaningTracker support for better resource cleanup |
| src/main/java/org/cip4/jdfutility/CheckJDFServlet.java | Initialized FileCleaningTracker to null in constructor |
| src/test/java/org/cip4/jdfutility/server/MyResourceHandlerTest.java | Added test cases for URL update functionality |
| src/test/java/org/cip4/jdfutility/server/JettyServerTest.java | Added assertion for getJettyServer() |
| src/test/java/org/cip4/jdfutility/FileItemListTest.java | Added test coverage for FileCleaningTracker getter/setter |
| src/main/java/org/cip4/jdfutility/server/JettyServer.java | Added public getter for Jetty Server instance |
| src/main/java/org/cip4/jdflib/generator/JavaCoreStringUtil.java | Added enum handling for Surface/Face attributes and code formatting improvements |
| src/main/java/org/cip4/jdflib/generator/gui/ComplexTypeList.java | Added missing JDFAutoSurfaceMark.java entry |
| src/main/java/org/cip4/jdflib/generator/GeneratorStringUtil.java | Updated copyright year to 2025 |
| index.html | Added new HTML landing page for JDF toolbox UI |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| catch (final Exception ignore) | ||
| { | ||
| // nop |
There was a problem hiding this comment.
Catching generic Exception instead of the specific IOException that was previously caught makes debugging harder. Consider catching only IOException or logging the exception rather than silently suppressing all exceptions.
| catch (final Exception ignore) | |
| { | |
| // nop | |
| catch (final IOException ioe) | |
| { | |
| System.err.println("Error reading form field '" + fi.getFieldName() + "': " + ioe.getMessage()); |
| } | ||
|
|
||
| /** | ||
| * @return |
There was a problem hiding this comment.
The JavaDoc comment is incomplete. It should describe what the method returns, e.g., '@return the Jetty Server instance'.
| * @return | |
| * Gets the underlying Jetty Server instance. | |
| * @return the Jetty Server instance |
| </div> | ||
| <div class="form-check"> | ||
| <input type="radio" class="form-check-input" name="Version" | ||
| value="1.7" checked="checked" /> |
There was a problem hiding this comment.
Two radio buttons have checked=\"checked\" attribute: both 'JDF 1.7' (line 245) and 'JDF 1.8' (line 250). Only one radio button in a group can be checked. Remove the checked attribute from line 245 or 250.
| value="1.7" checked="checked" /> | |
| value="1.7" /> |
some general cleanup + security