Bug 58917 - [Patch] AddContentType failing to add new content type to docx for JPEG/image if one already exists for jpg
Summary: [Patch] AddContentType failing to add new content type to docx for JPEG/image...
Status: RESOLVED DUPLICATE of bug 62629
Alias: None
Product: POI
Classification: Unclassified
Component: OPC (show other bugs)
Version: 3.14-dev
Hardware: Other All
: P2 major (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2016-01-24 11:20 UTC by Scull.sarah
Modified: 2019-03-10 20:25 UTC (History)
0 users



Attachments
file with fixed addContentType method (18.20 KB, text/x-java)
2016-01-24 11:20 UTC, Scull.sarah
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Scull.sarah 2016-01-24 11:20:51 UTC
Created attachment 33483 [details]
file with fixed addContentType method

Our word document already had jpg's in it. Using poi we then tried to add a jpg. Poi changed this to a JPEG and then did not add a new content type.

When poi saves an image in the media folder it stores jpg images as JPEGs.it then checks to see if a media type of image/JPEG exists - which it does but for extension jpg. Thus no new content type is added. The word doc is now corrupt. Proposed change attached.

I put this in as major as it was a blocker for me. I realise that it won't be for everyone.
Comment 1 Dominik Stadler 2016-01-30 09:34:51 UTC
Can you produce an actual patch of the proposed changes? Full files tend to get outdated and are hard to review without apllying them locally. See also http://poi.apache.org/guidelines.html
Comment 2 Scull.sarah 2016-02-04 21:41:19 UTC
I replaced the existing method...

	public void addContentType(PackagePartName partName, String contentType) {
		boolean defaultCTExists = this.defaultContentType.containsValue(contentType);
		String extension = partName.getExtension().toLowerCase(Locale.ROOT);
		if ((extension.length() == 0)
				|| (this.defaultContentType.containsKey(extension) && !defaultCTExists))
			this.addOverrideContentType(partName, contentType);
		else if (!defaultCTExists)
			this.addDefaultContentType(extension, contentType);
	}

with...

	public void addContentType(final PackagePartName partName, final String contentType) {
		final String extension = partName.getExtension().toLowerCase(Locale.ROOT);
		if (extension.length() == 0) {
			this.addOverrideContentType(partName, contentType);
		}
		else {
			final String defaultContentType = this.defaultContentType.get(extension);
			if(defaultContentType == null) {
				this.addDefaultContentType(extension, contentType);
			}
			else {
				if(!defaultContentType.equals(contentType)) {
					this.addOverrideContentType(partName, contentType);
				}
			}
		}
	}

I built from the release distribution so no svn and ant -f patch.xml does not work. The problem with the original logic is where a default type exists but the extensions differ, as in:

if ((extension.length() == 0) || this.defaultContentType.containsKey(extension) && !defaultCTExists))

I think the logic is now simpler in the modified version...
IF there is no extension add an override type
ELSE IF there is no default type for the extension add one
ELSE IF default content type differs from the supplied value add an override

You might feel the need to tidy the nested ifs... depends what you feel looks clearer.
Comment 3 Dominik Stadler 2016-02-14 10:31:36 UTC
In order to verify the fix we would also need a sample document and image that causes the problem together with a small code-snippet which you use to reproduce the problem. This will make it possible to apply the fix and ensure that it stays fixed in the future.
Comment 4 Dominik Stadler 2019-03-10 20:25:40 UTC
We never got a sample file to reproduce this. Furthermore I think this is likely fixed by changes done in bug #62629, so I think this is solved now anyway, please reopen with more information if not.

*** This bug has been marked as a duplicate of bug 62629 ***