RFD: A plan to untangle ZipFile.Source.initCEN
Hi, *Summary* Relieve ZipFile.Source.initCEN from non-parsing related responsibilities, such that it can become a pure function for parsing and validating CEN directories. Make it return Source, such that instance fields of this class may become final, immutable and accessible without synchronization. *Motivation* The method ZipFile.Source.initCEN has been the cause of a good amount of head-scratching over the years. It was brought over from the native implementation when this was transitioned to Java in Java 9. Risk assessments at that time probably constrained the code to closely align with the native code, which together with interpreter-friendliness in early startup have affected the current shape of this code. It is hard to summarize the responsibilities of this method, but I'll try: o Call findEND() to read metadata from the (Zip64) END headers in the ZIP file, validate fields and reject invalid values o Determine the position of the first LOC header o Read the CEN byte array from the ZIP file o Construct hash table data structures o Assign these to instance fields in Source o Parse and validate the CEN structure and populate hash table structures o Detect manifest file(s), signature related files and multi-release JAR versions o Detect when a ZIP has more CEN entries than declared by the END header and restart parsing by invoking itself recursively after counting the actual number of CEN entries o Detect when it is being invoked recursively to avoid reading the END and CEN headers twice It's been obvious for a while that this method performs an excessive number of unrelated tasks, knows too much about the state of the object being constructed and handles a disproportionate amount of funky control flow. It calls out to a number of helper methods to perform parsing and validation, some of them static, some relying on instance state. The amount of synchronization in ZipFile today is probably excessive for the current implementation. It made more sense back when ZipFile was backed by native code and resources. In particular, lookup misses pay a high relative cost to synchronization which is probably not needed. Making Source fields final moves us in a direction which may allow us to eventually remove synchronization for operations that only touch a parsed, immutable CEN. *Ideas and plan for improvements* I think we can improve the current situation by: o Moving END header parsing and validation out of initCEN o Moving CEN byte array reading out of initCEN o Making all helper methods static (it's a mix today) At this point, initCEN will almost, but not quite be a pure function for parsing the CEN byte array. So the next natural step is to actually make it a pure function by: o Making it static o Having it return a Source instance instead of assigning to the instance being constructed o Making Source (mostly) immutable by marking parsed instance fields as 'final' Rather than a big bang change, I see this effort as a step-by-step refactoring, probably over several PRs, each focusing on a smaller aspect of refactoring, hopefully making them easier to review. This is not an all-or-nothing effort, most of these changes will be incremental improvements on their own merit. If we don't get all the way, that's also okay. *Feedback?* Input to this idea/plan is welcome. If someone sees reasons this may not be a good idea, it's better to raise such concern here and now. Eirik.
Hi Eirik, While I think there may be some merit/benefit to what your suggesting, I think we should take a step back and also look at ZipFS to see what we might be able to share as part of a refactoring as there is a somewhat similar flow to ZipFS initCEN The benefit would be reducing the need to have to maintain somewhat duplicate code across both APIs. Best, Lance On Feb 10, 2026, at 6:38 AM, Eirik Bjørsnøs <eirbjo@gmail.com> wrote: Hi, Summary Relieve ZipFile.Source.initCEN from non-parsing related responsibilities, such that it can become a pure function for parsing and validating CEN directories. Make it return Source, such that instance fields of this class may become final, immutable and accessible without synchronization. Motivation The method ZipFile.Source.initCEN has been the cause of a good amount of head-scratching over the years. It was brought over from the native implementation when this was transitioned to Java in Java 9. Risk assessments at that time probably constrained the code to closely align with the native code, which together with interpreter-friendliness in early startup have affected the current shape of this code. It is hard to summarize the responsibilities of this method, but I'll try: o Call findEND() to read metadata from the (Zip64) END headers in the ZIP file, validate fields and reject invalid values o Determine the position of the first LOC header o Read the CEN byte array from the ZIP file o Construct hash table data structures o Assign these to instance fields in Source o Parse and validate the CEN structure and populate hash table structures o Detect manifest file(s), signature related files and multi-release JAR versions o Detect when a ZIP has more CEN entries than declared by the END header and restart parsing by invoking itself recursively after counting the actual number of CEN entries o Detect when it is being invoked recursively to avoid reading the END and CEN headers twice It's been obvious for a while that this method performs an excessive number of unrelated tasks, knows too much about the state of the object being constructed and handles a disproportionate amount of funky control flow. It calls out to a number of helper methods to perform parsing and validation, some of them static, some relying on instance state. The amount of synchronization in ZipFile today is probably excessive for the current implementation. It made more sense back when ZipFile was backed by native code and resources. In particular, lookup misses pay a high relative cost to synchronization which is probably not needed. Making Source fields final moves us in a direction which may allow us to eventually remove synchronization for operations that only touch a parsed, immutable CEN. Ideas and plan for improvements I think we can improve the current situation by: o Moving END header parsing and validation out of initCEN o Moving CEN byte array reading out of initCEN o Making all helper methods static (it's a mix today) At this point, initCEN will almost, but not quite be a pure function for parsing the CEN byte array. So the next natural step is to actually make it a pure function by: o Making it static o Having it return a Source instance instead of assigning to the instance being constructed o Making Source (mostly) immutable by marking parsed instance fields as 'final' Rather than a big bang change, I see this effort as a step-by-step refactoring, probably over several PRs, each focusing on a smaller aspect of refactoring, hopefully making them easier to review. This is not an all-or-nothing effort, most of these changes will be incremental improvements on their own merit. If we don't get all the way, that's also okay. Feedback? Input to this idea/plan is welcome. If someone sees reasons this may not be a good idea, it's better to raise such concern here and now. Eirik. [oracle_sig_logo.gif] Lance Andersen | Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 Lance.Andersen@oracle.com
On Tue, Feb 10, 2026 at 7:24 PM Lance Andersen <lance.andersen@oracle.com> wrote:
While I think there may be some merit/benefit to what your suggesting, I think we should take a step back and also look at ZipFS to see what we might be able to share as part of a refactoring as there is a somewhat similar flow to ZipFS initCEN
The benefit would be reducing the need to have to maintain somewhat duplicate code across both APIs.
I have less intimate experience with the ZipFS implementation. But ZipFile's initCEN is so convoluted with various responsibilities, state management and control flow that every time I take a step back from it I immediately lose track of whatever understanding I just had about how this method actually works :) If anything, peeling apart the various layers of this very big onion should make it easier to see which parts are reusable, and which parts are better maintained in each implementation. Perhaps we could eventually extract the CEN parsing and validation into a little utility library, reusable from ZipFS? ZipFs includes write support, needs to fit within the FileSystem API, supports Posix attributes etc. But sure, there may be some synergies. I don't see how the refactorings and enhancements in code organization proposed here would make such synergies less achievable. On the contrary, I think peeling the onion may be a necessary precondition. Or did you have more specific ideas in mind when suggesting we "take a step back"? Eirik.
On Tue, Feb 10, 2026 at 7:24 PM Lance Andersen <lance.andersen@oracle.com> wrote:
While I think there may be some merit/benefit to what your suggesting, I think we should take a step back and also look at ZipFS to see what we might be able to share as part of a refactoring as there is a somewhat similar flow to ZipFS initCEN
The benefit would be reducing the need to have to maintain somewhat duplicate code across both APIs.
Lance, I think discussing a reusable CEN parsing library at this point may be a bit premature. But we certainly don't want refactorings discussed here to make such a future harder to get to. If your concern is that the refactorings suggested here may make ZipFile/ZipFS initCENs diverge further, then perhaps we can add as a goal that these two should be refactored together? We could certainly do that. Either in lockstep with each refactoring being applied across APIs in the same PRs (where applicable), or we could clean up the ZipFile first, then have a follow up where we realign ZipFS at the end. Whatever we think makes most sense. Wdyt? Eirik.
Hi Eirik, Yes one of the goals should be to share as much code as possible, regardless of where the initial changes are to ZipFile. initCEN is similar in both instances WRT flow and would best to keep changes in mind so that they can easily be retrofitted to ZipFS with the hopes of having shared code making it easier to address issues that often apply to both due to CEN processing Another area which you had indicated looking at is ZipCoder which there are 2 similar implementations between ZipFS and ZipFile et al. I believe part of that is due to ZipFS starting as a demo but would be good to streamline and ideally get to 1 implementation if possible. I have asked Sherman, who is the original author of ZipFS and worked extensively on Zip to chime in on your ZipCoder suggestion. This has been on the todo list to look at but has not risen to the top of the list so appreciate your time in giving some thought to it. Hope that answers your question. Best, Lance On Feb 13, 2026, at 9:58 AM, Eirik Bjørsnøs <eirbjo@gmail.com> wrote: On Tue, Feb 10, 2026 at 7:24 PM Lance Andersen <lance.andersen@oracle.com<mailto:lance.andersen@oracle.com>> wrote: While I think there may be some merit/benefit to what your suggesting, I think we should take a step back and also look at ZipFS to see what we might be able to share as part of a refactoring as there is a somewhat similar flow to ZipFS initCEN The benefit would be reducing the need to have to maintain somewhat duplicate code across both APIs. Lance, I think discussing a reusable CEN parsing library at this point may be a bit premature. But we certainly don't want refactorings discussed here to make such a future harder to get to. If your concern is that the refactorings suggested here may make ZipFile/ZipFS initCENs diverge further, then perhaps we can add as a goal that these two should be refactored together? We could certainly do that. Either in lockstep with each refactoring being applied across APIs in the same PRs (where applicable), or we could clean up the ZipFile first, then have a follow up where we realign ZipFS at the end. Whatever we think makes most sense. Wdyt? Eirik. [oracle_sig_logo.gif] Lance Andersen | Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 Lance.Andersen@oracle.com
On Fri, Feb 13, 2026 at 8:43 PM Lance Andersen <lance.andersen@oracle.com> wrote:
Yes one of the goals should be to share as much code as possible, regardless of where the initial changes are to ZipFile. initCEN is similar in both instances WRT flow and would best to keep changes in mind so that they can easily be retrofitted to ZipFS with the hopes of having shared code making it easier to address issues that often apply to both due to CEN processing
Good, it seems our goals here are reasonably aligned: To pick apart initCEN and make parts reusable across implementations. Reduce maintenance costs caused by duplicated and difficult-to-understand code paths.
Another area which you had indicated looking at is ZipCoder which there are 2 similar implementations between ZipFS and ZipFile et al. I believe part of that is due to ZipFS starting as a demo but would be good to streamline and ideally get to 1 implementation if possible. I have asked Sherman, who is the original author of ZipFS and worked extensively on Zip to chime in on your ZipCoder suggestion.
Yes, ZipCoder also have a lot in common but have diverged over the years. There may be slight differences in client needs, but I think most of it can be aligned across implementations. I have done a quick review of differences between ZipFile/ZipFileSystem implementations and so far I have filed: https://bugs.openjdk.org/browse/JDK-8377983 to make ZipFileSystem not read the END header when reading the CEN https://bugs.openjdk.org/browse/JDK-8377992 to align ZipFileSystem END header validation with ZipFile. https://bugs.openjdk.org/browse/JDK-8377978 to add a test for a ZipFile.Source.findEND edge-case. Eirik.
participants (2)
-
Eirik Bjørsnøs
-
Lance Andersen