Fix missing rels content-type registration causing null MainDocumentPart#2090
Fix missing rels content-type registration causing null MainDocumentPart#2090rakeshbabuseva wants to merge 2 commits into
Conversation
Some OOXML generators omit the required `<Default Extension="rels" .../>` entry from `[Content_Types].xml`. System.IO.Packaging is strict: without that registration it cannot resolve .rels parts, so the package relationship walk never finds the officeDocument relationship and MainDocumentPart comes back null. RepairRelsContentType() in StreamPackageFeature detects this deviation before Package.Open and rebuilds the stream with the missing entry injected into [Content_Types].xml. All other ZIP entries are streamed verbatim. The repair is a no-op for well-formed packages. Gated on !NETFRAMEWORK because ZipArchive is not available on net35/net40 and the old System.IO.Packaging on those targets is already lenient. Fixes: WordprocessingDocument.Open returns null MainDocumentPart when [Content_Types].xml omits Default Extension="rels"
@dotnet-policy-service agree |
Test Results 12 files - 69 12 suites - 69 11m 15s ⏱️ - 1h 23m 33s Results for commit ece3284. ± Comparison against base commit 00967dc. This pull request removes 5 and adds 2 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
mikeebowen
left a comment
There was a problem hiding this comment.
Thanks for this submission, we should definitely handle this exception. Before final approval, please make a few changes:
- Check for
application/vnd.openxmlformats-package.relationships+xmlinstead ofExtension=\"rels\" - Throw an exception instead of adding the missing element
- Resolve build errors
| #if !NETFRAMEWORK | ||
| private static Stream RepairRelsContentType(Stream input) | ||
| { | ||
| const string Marker = "Extension=\"rels\""; |
There was a problem hiding this comment.
Technically, the extension is not required to be rels. Instead check for application/vnd.openxmlformats-package.relationships+xml because the content type won't change
| ctXml = reader.ReadToEnd(); | ||
| } | ||
|
|
||
| #if NET6_0_OR_GREATER |
There was a problem hiding this comment.
Instead of checking for the element and adding it if it's missing, check for ctXml.IndexOf(Marker, StringComparison.Ordinal) == 0 and throw an exception if it is missing. This parallels Office behavior and avoids adding preprocessor statements.
| } | ||
|
|
||
| #if NET6_0_OR_GREATER | ||
| if (ctXml.Contains(Marker, StringComparison.Ordinal)) |
There was a problem hiding this comment.
hey @rakeshbabuseva,
Thanks very much for this PR. I think this is a good catch. I do have some thoughts on this and please take a look and see what you think.
-
I think to be more correct, and although this is probably hitting corner cases, the code should be testing on
"ContentType=\"application/vnd.openxmlformats-package.relationships+xml\"/>"instead of"<Default Extension=\"rels\" " -
If it does not find any
<Default Extension=...>child withapplication/vnd.openxmlformats-package.relationships+xmlcontent type, it should throw an exception as this likely would mean the integrity of the package is not sound. -
If it does find such a
<Default Extension...>child then the package should be considered ok.
Adding "<Default Extension=\"rels\" " + "ContentType=\"application/vnd.openxmlformats-package.relationships+xml\"/>" would probably be correct most of the time but I think 29500-1 and -2 allow for using another extension and that could mean we're not helping the integrity of the package.
Word does call this out to the user and while it does offer to repair and will use "<Default Extension=\"rels\", Word also would have the extra logic to make sure the actual relationship parts use that extension or change them if they don't. We probably don't want to put all that logic in the SDK.
Finally, I think that throwing the exception would be good since it's apparent that the package integrity is not good and this would allow the Open XML producing app, library or tool to correct the situation rather than mask it.
What do you think? We can discuss further if you'd like.
Some OOXML generators omit the required
<Default Extension="rels" .../>entry from[Content_Types].xml. System.IO.Packaging is strict: without that registration it cannot resolve .rels parts, so the package relationship walk never finds the officeDocument relationship and MainDocumentPart comes back null.RepairRelsContentType() in StreamPackageFeature detects this deviation before Package.Open and rebuilds the stream with the missing entry injected into [Content_Types].xml. All other ZIP entries are streamed verbatim. The repair is a no-op for well-formed packages. Gated on !NETFRAMEWORK because ZipArchive is not available on net35/net40 and the old System.IO.Packaging on those targets is already lenient.
Fixes: WordprocessingDocument.Open returns null MainDocumentPart when [Content_Types].xml omits Default Extension="rels"