Replace runtime panics with the matching Java exceptions - #174
Conversation
An unwrap audit found panics reachable from ordinary Java code: - File.length() returns 0 for a missing file; isDirectory/isFile lose their guard-then-unwrap shape - FileImpl (native runtime) maps open/read/write/seek failures to IOError instead of panicking, so FileInputStream and RandomAccessFile guards actually produce FileNotFoundException; FileOutputStream gains the same guard - File I/O operations (read/write/seek/available/length/setLength) throw java.io.IOException on failure via a shared helper - Class.forName resolves the class and throws ClassNotFoundException (new runtime class) instead of panicking on any not-yet-loaded name - StringBuffer.append(char)/append(char[]) keep exact UTF-16 units so unpaired surrogates no longer panic and pairs built char by char survive; String.valueOf(char) builds through [C for the same reason - PrintStream.println(char) replaces an unpaired surrogate with '?' like the JDK charset encoder - ZipFile validates the archive in its constructor and throws java.util.zip.ZipException (new runtime class) for a malformed archive; getInputStream returns null for a missing entry Expected outputs for the new fixtures are generated by a real JVM. Remaining unwraps are invariants (interpreter stack discipline, thread attach), guarded lookups, or documented gaps (lenient calendar normalization, ClassFormatError plumbing).
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #174 +/- ##
==========================================
+ Coverage 83.71% 83.89% +0.18%
==========================================
Files 171 173 +2
Lines 12867 12939 +72
==========================================
+ Hits 10771 10855 +84
+ Misses 2096 2084 -12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR hardens the RustJava runtime by removing panic paths reachable from ordinary Java code and mapping failures to the Java exceptions/behaviors that the JDK produces, with new E2E fixtures whose expected outputs were generated from a reference JVM.
Changes:
- Replace
unwrap()-based runtime panics in file I/O, class loading, surrogate handling, and zip handling with JDK-aligned exceptions/return values. - Add runtime classes
java.lang.ClassNotFoundExceptionandjava.util.zip.ZipExceptionand wire them into the runtime loader. - Add new Java fixtures and golden outputs covering missing files, I/O errors,
Class.forName, surrogate chars, and corrupt zip behavior.
Reviewed changes
Copilot reviewed 28 out of 33 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test_data/ZipCorrupt.txt | Golden output for corrupt/non-zip ZipFile constructor behavior. |
| test_data/SurrogateChars.txt | Golden output for surrogate-pair and lone-surrogate behaviors. |
| test_data/src/ZipCorrupt.java | Fixture exercising ZipFile constructor throwing ZipException. |
| test_data/src/SurrogateChars.java | Fixture exercising surrogate pair construction and lone surrogate printing. |
| test_data/src/ForName.java | Fixture exercising Class.forName success + ClassNotFoundException. |
| test_data/src/FileLength.java | Fixture for File.exists() + File.length() on missing file. |
| test_data/src/FileErrors.java | Fixture for FileNotFoundException from common file I/O constructors. |
| test_data/ForName.txt | Golden output for Class.forName fixture. |
| test_data/FileLength.txt | Golden output for missing-file File.length() returning 0. |
| test_data/FileErrors.txt | Golden output for missing-file/dir constructor failures. |
| src/runtime/io.rs | Replace several I/O unwrap()s with IOError propagation in the native runtime file impls. |
| src/runtime.rs | Change runtime open() to return an IOError instead of panicking on open failure. |
| java_runtime/src/runtime/io.rs | Add IOError::Io variant for non-NotFound I/O failures. |
| java_runtime/src/loader.rs | Register new runtime classes (ClassNotFoundException, ZipException). |
| java_runtime/src/classes/java/util/zip/zip_file.rs | Convert zip parsing/entry access panics into ZipException and JDK-like null returns. |
| java_runtime/src/classes/java/util/zip/zip_exception.rs | Add java.util.zip.ZipException runtime class (extends IOException). |
| java_runtime/src/classes/java/util/zip.rs | Export ZipException module/type from java.util.zip. |
| java_runtime/src/classes/java/lang/string.rs | Preserve unpaired surrogates in String.valueOf(char) by constructing via [C. |
| java_runtime/src/classes/java/lang/string_buffer.rs | Switch internal appends to UTF-16-based append to preserve unpaired surrogates. |
| java_runtime/src/classes/java/lang/class.rs | Make Class.forName resolve/load and throw ClassNotFoundException instead of panicking. |
| java_runtime/src/classes/java/lang/class_not_found_exception.rs | Add java.lang.ClassNotFoundException runtime class (extends Exception). |
| java_runtime/src/classes/java/lang.rs | Export ClassNotFoundException from java.lang module. |
| java_runtime/src/classes/java/io/random_access_file.rs | Route file op failures through shared checked() helper instead of unwrap(). |
| java_runtime/src/classes/java/io/print_stream.rs | Avoid panicking on surrogate char printing; replace with ?. |
| java_runtime/src/classes/java/io/input_stream_reader.rs | Propagate array-buffer read failures instead of unwrapping. |
| java_runtime/src/classes/java/io/file.rs | Make File.isDirectory/isFile/length avoid unwraps and match JDK missing-file behavior. |
| java_runtime/src/classes/java/io/file_output_stream.rs | Add open guard for FileOutputStream and propagate write failures as IOException. |
| java_runtime/src/classes/java/io/file_input_stream.rs | Propagate read/available failures as IOException via checked(). |
| java_runtime/src/classes/java/io.rs | Add shared checked() helper to convert runtime I/O failures into java.io.IOException. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbba0fe4e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Ok(class.into()) | ||
| match jvm.resolve_class(&qualified_name).await { | ||
| Ok(class) => Ok(class.java_class().into()), | ||
| Err(_) => Err(jvm.exception("java/lang/ClassNotFoundException", &rust_name).await), |
There was a problem hiding this comment.
Preserve linkage errors from Class.forName
When the requested class file is present but resolving one of its superclasses or interfaces fails, resolve_class propagates that Java error (for example NoClassDefFoundError from jvm/src/jvm.rs when a dependency is missing). This blanket Err(_) converts those linkage failures into ClassNotFoundException, so Java code can incorrectly catch them as CNFE instead of seeing the linkage error that Class.forName should propagate. Only the actual “requested class not found” case should be remapped here; other resolve_class errors should be returned unchanged.
Useful? React with 👍 / 👎.
Judged the two remaining remote branches on the fork: - dependabot/cargo/tracing-attributes-0.1.31: deleted. PR #4 (fa92ef9) removed the tracing-attributes direct dependency outright, so the branch patches a Cargo.toml line that no longer exists. - wie-ktf-hardening: preserved. 8 of its 12 commits are already in upstream/main via squash merges (dlunch#174 dlunch#175 dlunch#176 dlunch#177 dlunch#180 dlunch#182); git cherry missed this because origin/main trails upstream/main by 20 commits. 4 commits carry residual value. No code changes. Co-authored-by: jun0 <junyoung.choi.a@miraeasset.com> Co-authored-by: Claude <noreply@anthropic.com>
…am-sync-s1-tracing-cut-1f356ae] * Bump bytemuck from 1.25.0 to 1.25.1 (dlunch#173) Bumps [bytemuck](https://github.com/Lokathor/bytemuck) from 1.25.0 to 1.25.1. - [Changelog](https://github.com/Lokathor/bytemuck/blob/main/changelog.md) - [Commits](Lokathor/bytemuck@v1.25.0...v1.25.1) --- updated-dependencies: - dependency-name: bytemuck dependency-version: 1.25.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Replace runtime panics with the matching Java exceptions (dlunch#174) * Replace runtime panics with the matching Java exceptions An unwrap audit found panics reachable from ordinary Java code: - File.length() returns 0 for a missing file; isDirectory/isFile lose their guard-then-unwrap shape - FileImpl (native runtime) maps open/read/write/seek failures to IOError instead of panicking, so FileInputStream and RandomAccessFile guards actually produce FileNotFoundException; FileOutputStream gains the same guard - File I/O operations (read/write/seek/available/length/setLength) throw java.io.IOException on failure via a shared helper - Class.forName resolves the class and throws ClassNotFoundException (new runtime class) instead of panicking on any not-yet-loaded name - StringBuffer.append(char)/append(char[]) keep exact UTF-16 units so unpaired surrogates no longer panic and pairs built char by char survive; String.valueOf(char) builds through [C for the same reason - PrintStream.println(char) replaces an unpaired surrogate with '?' like the JDK charset encoder - ZipFile validates the archive in its constructor and throws java.util.zip.ZipException (new runtime class) for a malformed archive; getInputStream returns null for a missing entry Expected outputs for the new fixtures are generated by a real JVM. Remaining unwraps are invariants (interpreter stack discipline, thread attach), guarded lookups, or documented gaps (lenient calendar normalization, ClassFormatError plumbing). * Inline the IOException conversion at each I/O call site * Return the same Thread object from Thread.currentThread() (dlunch#175) Every attached thread now owns its java/lang/Thread instance: attach takes the instance for threads started via Thread.start (so currentThread() inside run() is the started Thread object) and creates one otherwise (bootstrap, external attachers). currentThread() returns the stored instance, and the GC roots it per thread. Also parse unrecognized classfile attributes as an opaque Unknown variant instead of failing — JVMS 4.7.1 requires silently ignoring them, and the anonymous-class fixture carries EnclosingMethod and Signature attributes the parser rejected. Expected output for the fixture is generated by a real JVM. * Add Java primitive wrapper classes (dlunch#176) * Add Java primitive wrapper classes * Use Character digit semantics for numeric parsing * Bump tokio from 1.52.3 to 1.52.4 (dlunch#179) Bumps [tokio](https://github.com/tokio-rs/tokio) from 1.52.3 to 1.52.4. - [Release notes](https://github.com/tokio-rs/tokio/releases) - [Commits](tokio-rs/tokio@tokio-1.52.3...tokio-1.52.4) --- updated-dependencies: - dependency-name: tokio dependency-version: 1.52.4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * [rustjava-upstream-sync-s1-tracing-cut-1f356ae] docs: record S1 landing (conflicts 2, setProperty descriptor breakage) --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Inseok Lee <git@dlun.ch> Co-authored-by: jun0 <junyoung.choi.a@miraeasset.com>
Summary
An audit of every
unwrap()in the runtime found panics reachable from ordinary Java code — starting fromFile.length()crashing on a file that simply doesn't exist. Each is replaced with the exception (or return value) the JDK produces; expected outputs for all new fixtures are generated by running them on a real JVM.Fixes
File / streams
File.length()returns0for a missing file (spec) instead of panicking;isDirectory/isFiledrop their guard-then-unwrap shape.FileImplin the native runtime panicked on any failingopen, so the existingFileNotFoundExceptionguards inFileInputStream/RandomAccessFilewere unreachable. It now maps open/read/write/seek failures toIOError(newIovariant),FileOutputStreamgains the same guard, and all file I/O operations (read,write,seek,available,length,setLength,getFilePointer) throwjava.io.IOExceptionon failure via a shared helper.Class.forName
ClassNotFoundException(new runtime class, extendsExceptionper the 1.2 hierarchy) when resolution fails.Unpaired surrogates
StringBuffer.append(char)/append(char[],int,int)panicked on an unpaired surrogate. The internal append is now UTF-16-based, so building a surrogate pair char by char produces the correct string;String.valueOf(char)constructs through[Cfor the same reason;PrintStream.println(char)replaces an unpaired surrogate with?matching the JDK charset encoder.Zip
new ZipFile(...)on a non-zip file panicked inZipArchive::new. The constructor now validates the archive (matching the JDK's timing) and throwsjava.util.zip.ZipException(new runtime class, extendsIOException);getInputStreamreturnsnullfor an entry not in the archive.Not converted (audited, intentionally left)
Thread— error path of the error path, inside a spawn closure.GregorianCalendaron out-of-range fields — the correct behavior is lenient normalization (a feature), not an exception.ClassInfo::parseon a malformed classfile — needsClassFormatErrorplumbing through jvm-less sync code (existing TODO).Test plan
E2E fixtures with real-JVM-generated expected output, each verified to fail before its fix:
FileLength(missing-file length/exists),FileErrors(FileNotFoundExceptionfrom FileInputStream/FileOutputStream/RandomAccessFile),ForName(loadsjava.util.Vector,ClassNotFoundExceptionfor an unknown name),SurrogateChars(pair building, lone-surrogate length/println),ZipCorrupt(ZipExceptionfrom the constructor).cargo test --workspacegreen, fmt clean, clippy no new warnings.