feat: implement URL scheme policy for image fetching#917
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a configurable URL image fetching policy to the spreadsheet module, including scheme restrictions, private-network checks, redirect limits, response size limits, and image-type validation. It also adds tests around URL image conversion and bumps project/module versions to 2.0.2-incubating.
Changes:
- Added URL fetch policy types (
SchemePolicy,UrlImageFetchPolicy,CidrBlock) and integrated them intoUrlImageConverter. - Expanded image type detection in
FileTypeUtils. - Added URL image converter tests and updated module parent versions.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
pom.xml |
Bumps parent project version and revision. |
fesod-bom/pom.xml |
Updates parent version. |
fesod-common/pom.xml |
Updates parent version. |
fesod-shaded/pom.xml |
Updates parent version. |
fesod-sheet/pom.xml |
Updates parent version. |
fesod-examples/pom.xml |
Updates parent version. |
fesod-examples/fesod-sheet-examples/pom.xml |
Updates parent version. |
fesod-examples/fesod-sheet-examples/src/test/java/org/apache/fesod/sheet/temp/bug/ExcelCreat.java |
Adds the explicit HeadType import. |
fesod-sheet/src/main/java/org/apache/fesod/sheet/util/FileTypeUtils.java |
Reworks image type detection using POI file magic and DIB detection. |
fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/url/CidrBlock.java |
Adds CIDR parsing and address matching for private-network allowlists. |
fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/url/SchemePolicy.java |
Adds URL scheme policy enum. |
fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/url/UrlImageFetchPolicy.java |
Adds immutable fetch policy and builder. |
fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/url/UrlImageConverter.java |
Enforces URL policy, redirects, size limits, and image type validation. |
fesod-sheet/src/test/java/org/apache/fesod/sheet/converter/UrlImageConverterTest.java |
Adds tests for URL image fetching policy behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| FILE_TYPE_MAP.put(FileMagic.PNG, ImageData.ImageType.PICTURE_TYPE_PNG); | ||
| FILE_TYPE_MAP.put(FileMagic.WMF, ImageData.ImageType.PICTURE_TYPE_WMF); | ||
| FILE_TYPE_MAP.put(FileMagic.EMF, ImageData.ImageType.PICTURE_TYPE_EMF); | ||
| FILE_TYPE_MAP.put(FileMagic.BMP, ImageData.ImageType.PICTURE_TYPE_DIB); |
| ImageData.ImageType imageType = FileTypeUtils.getImageType(bytes); | ||
| if (imageType == null) { | ||
| throw new IOException("URL image data is not a supported image type"); | ||
| } | ||
| return new WriteCellData<>(bytes); |
| private static volatile UrlImageFetchPolicy fetchPolicy = UrlImageFetchPolicy.defaultPolicy(); | ||
|
|
||
| public static UrlImageFetchPolicy getFetchPolicy() { | ||
| return fetchPolicy; | ||
| } | ||
|
|
||
| public static void setFetchPolicy(UrlImageFetchPolicy fetchPolicy) { | ||
| if (fetchPolicy == null) { | ||
| throw new IllegalArgumentException("Fetch policy can not be null"); | ||
| } | ||
| UrlImageConverter.fetchPolicy = fetchPolicy; | ||
| } | ||
|
|
||
| public static void resetFetchPolicy() { | ||
| fetchPolicy = UrlImageFetchPolicy.defaultPolicy(); | ||
| } | ||
|
|
| InetAddress[] addresses = InetAddress.getAllByName(normalizedHost); | ||
| if (addresses.length == 0) { | ||
| throw new IOException("URL image host can not be resolved"); | ||
| } | ||
| for (InetAddress address : addresses) { |
| } | ||
|
|
||
| try { | ||
| InetAddress address = InetAddress.getByName(parts[0]); |
| if (allowedPrivateCidrs == null) { | ||
| this.allowedPrivateCidrs = Collections.emptyList(); | ||
| } else { | ||
| this.allowedPrivateCidrs = new ArrayList<>(allowedPrivateCidrs); |
| int second = bytes[1] & 0xFF; | ||
| return first == 0 | ||
| || first == 10 | ||
| || first == 127 | ||
| || (first == 100 && second >= 64 && second <= 127) | ||
| || (first == 169 && second == 254) | ||
| || (first == 172 && second >= 16 && second <= 31) | ||
| || (first == 192 && second == 168) |
| private URL startServer(int status, byte[] body, String contentType) throws IOException { | ||
| server = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); | ||
| server.createContext("/", exchange -> { |
| private URL startRedirectServer(String location) throws IOException { | ||
| server = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); | ||
| server.createContext("/", exchange -> { |
|
I'm considering whether to supplement the JavaDoc explanation: The current built-in request implementation only provides basic URL validation capabilities. If users require more comprehensive request security policies in actual scenarios (such as more granular timeouts, certificate validation, proxies, DNS, etc.), they can also choose to integrate mature third-party HttpClient implementations based on their own needs. |
I think we can simply include this in the documentation on our official website. For now, there is no need to include such extensive suggestions or information within the Javadoc comments. |
+1 |
Purpose of the pull request
What's changed?
Checklist