Add initial Java conversion engine - #114
Merged
Merged
Conversation
|
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit details: You’ve used all 2 included reviews currently available. This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (27)
Comment |
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness/security issues in the ZIP normalization/limit logic and PDF stream serialization that should be fixed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an initial Java 17 implementation of MiniPdf, introducing a Maven reactor with a core library and CLI that can detect OOXML containers, extract basic DOCX/XLSX text, and emit deterministic PDF 1.4 output with security-focused ZIP/XML handling.
Changes:
- Introduces
minipdf-javaMaven reactor withminipdf(library) andminipdf-cli(shaded CLI). - Implements initial OOXML detection + secure ZIP/XML parsing, plus basic DOCX/XLSX text extraction and PDF writing.
- Adds JUnit tests covering PDF structure invariants, OOXML security guards, classic fixtures, and CLI behavior.
File summaries
| File | Description |
|---|---|
| minipdf-java/pom.xml | Adds Java 17 parent reactor with dependency/plugin management (JUnit, picocli, surefire). |
| minipdf-java/minipdf/pom.xml | Defines the Java library module and test dependencies. |
| minipdf-java/minipdf-cli/pom.xml | Defines the CLI module and shaded JAR packaging. |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/MiniPdf.java | Public Java API for format detection, conversion, and font registration. |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/MiniPdfException.java | Introduces categorized exceptions for IO/ZIP/XML/format/input errors. |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/OfficeFormat.java | Adds Office format enum used by detection/conversion routing. |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/ConversionOptions.java | Adds conversion options (currently page size override). |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/PageSize.java | Adds standard/custom page sizes with input validation. |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/PdfColor.java | Adds PDF RGB color model with component validation. |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/PdfPage.java | Adds page abstraction collecting text drawing operations. |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/PdfDocument.java | Implements deterministic PDF 1.4 writing (objects/xref/streams). |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/RegisteredFont.java | Adds immutable font registration record with defensive copying. |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/internal/OoxmlPackage.java | Secure ZIP loading with entry/path validation and size limits. |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/internal/OfficePackageDetector.java | Detects DOCX/XLSX/PPTX based on OOXML package entries. |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/internal/SecureXml.java | Provides hardened StAX parsing configuration (no DTD/XXE). |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/internal/SimplePdfTextRenderer.java | Renders extracted text lines into basic PDF pages with wrapping. |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/internal/docx/DocxConverter.java | Extracts paragraph text from DOCX word/document.xml. |
| minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/internal/xlsx/XlsxConverter.java | Extracts shared strings + worksheet rows into printable lines. |
| minipdf-java/minipdf-cli/src/main/java/io/github/minisoftware/minipdf/cli/MiniPdfCommand.java | Adds picocli-based CLI supporting direct and convert subcommand syntax. |
| minipdf-java/minipdf/src/test/java/io/github/minisoftware/minipdf/PdfDocumentTest.java | Tests PDF header/footer, stream length accounting, escaping, xref offsets, and validation. |
| minipdf-java/minipdf/src/test/java/io/github/minisoftware/minipdf/PageSizeTest.java | Tests standard sizes, input validation, and options behavior. |
| minipdf-java/minipdf/src/test/java/io/github/minisoftware/minipdf/OoxmlSecurityTest.java | Tests traversal/XXE protections in OOXML processing. |
| minipdf-java/minipdf/src/test/java/io/github/minisoftware/minipdf/OfficeFormatDetectionTest.java | Tests format detection and unsupported PPTX behavior. |
| minipdf-java/minipdf/src/test/java/io/github/minisoftware/minipdf/FontRegistrationTest.java | Tests defensive copying and input validation for font registration. |
| minipdf-java/minipdf/src/test/java/io/github/minisoftware/minipdf/ClassicFixtureSmokeTest.java | Smoke-tests converting classic XLSX/DOCX fixtures from the repo. |
| minipdf-java/minipdf/src/test/java/io/github/minisoftware/minipdf/BasicOfficeConversionTest.java | Tests minimal DOCX/XLSX conversions, page override, and XML error classification. |
| minipdf-java/minipdf-cli/src/test/java/io/github/minisoftware/minipdf/cli/MiniPdfCommandTest.java | Tests CLI direct/subcommand flows and option validation. |
Review details
Suppressed comments (1)
minipdf-java/minipdf/src/main/java/io/github/minisoftware/minipdf/internal/OoxmlPackage.java:96
normalizeEntryNamenormalizes path separators but not case.OfficePackageDetectordetects formats case-insensitively, but converters later look up exact lower-case paths (e.g.,word/document.xml,xl/sharedStrings.xml). This can lead to successful format detection followed by conversion failure for ZIPs that use upper-case entry names (and the tests already expect case normalization during detection).
String normalized = entryName.replace('\\', '/');
- Files reviewed: 27/27 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+62
to
+64
| if (totalSize > MAX_TOTAL_SIZE | ||
| || (input.length > 0 && totalSize / input.length > MAX_EXPANSION_RATIO)) { | ||
| throw invalidPackage("ZIP package expands beyond the configured limit"); |
Comment on lines
+116
to
+120
| ByteArrayOutputStream object = new ByteArrayOutputStream(); | ||
| write(object, "<< /Length " + stream.length + " >>\nstream\n"); | ||
| object.writeBytes(stream); | ||
| write(object, "endstream"); | ||
| return object.toByteArray(); |
Comment on lines
+39
to
+43
| int streamStart = matcher.end(); | ||
| int streamEnd = text.indexOf("endstream", streamStart); | ||
| assertEquals(Integer.parseInt(matcher.group(1)), streamEnd - streamStart); | ||
| streams++; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
minipdfand executableminipdf-climodulesPPTX is detected but intentionally remains unsupported in this initial Java implementation.
Validation
mvn -B -ntp verifyclassic01_basic_table_with_headers.xlsxto a valid PDF