Skip to content

Commit

Permalink
fix: report ZIP checks after the 'Validating…' message
Browse files Browse the repository at this point in the history 
Internal changes:

- move the zip header checks from the `EpubCheck` class to the
  `OCFChecker` class
- run the checks _after_ the "Validating using EPUB version…" message
- make sure the checks are run in any case even when the validation
  aborts before the "Validating using EPUB version…" message.

Test changes:

- add a CLI test case for the message ordering
- update the invalid mimetype test files to be based on the minimal EPUB
- remove a duplicate test case (`lorem-mimetype-2`)

Fix #1025
  • Loading branch information
@rdeltour
rdeltour committed Mar 27, 2019
1 parent b9ed8f6 commit 73b0ee8
Show file tree
Hide file tree
Showing 19 changed files with 187 additions and 358 deletions.
73 changes: 4 additions & 69 deletions src/main/java/com/adobe/epubcheck/api/EpubCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
package com.adobe.epubcheck.api;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
Expand All @@ -41,7 +40,6 @@
import com.adobe.epubcheck.opf.DocumentValidator;
import com.adobe.epubcheck.opf.OPFData;
import com.adobe.epubcheck.opf.ValidationContext.ValidationContextBuilder;
import com.adobe.epubcheck.util.CheckUtil;
import com.adobe.epubcheck.util.DefaultReportImpl;
import com.adobe.epubcheck.util.EPUBVersion;
import com.adobe.epubcheck.util.ResourceUtil;
Expand Down Expand Up @@ -210,7 +208,6 @@ public boolean validate()
public int doValidate()
{
ZipFile zip = null;
FileInputStream epubIn = null;
try
{
if (!epubFile.exists())
Expand All @@ -219,13 +216,11 @@ public int doValidate()
return 2;
}

epubIn = new FileInputStream(epubFile);
checkEpubHeader(epubIn);
zip = new ZipFile(epubFile);

OCFPackage ocf = new OCFZipPackage(zip);
OCFChecker checker = new OCFChecker(new ValidationContextBuilder().ocf(ocf).report(report)
.profile(profile).build());
OCFChecker checker = new OCFChecker(new ValidationContextBuilder()
.path(epubFile.getAbsolutePath()).ocf(ocf).report(report).profile(profile).build());
checker.runChecks();

String extension = ResourceUtil.getExtension(epubFile.getName());
Expand All @@ -236,16 +231,14 @@ public int doValidate()
c.checkPackage();
} catch (IOException e)
{
// run ZIP header checks in any case before returning
OCFChecker.checkZipHeader(epubFile, report);
report.message(MessageId.PKG_008, EPUBLocation.create(epubFile.getName(), ""),
e.getMessage());
} finally
{
try
{
if (epubIn != null)
{
epubIn.close();
}
if (zip != null)
{
zip.close();
Expand Down Expand Up @@ -285,62 +278,4 @@ void checkExtension(OCFPackage ocf, String extension)
}
}

void checkEpubHeader(FileInputStream epubIn)
throws IOException
{
byte[] header = new byte[58];

int readCount = epubIn.read(header);
if (readCount != -1)
{
while (readCount < header.length)
{
int read = epubIn.read(header, readCount, header.length - readCount);
// break on eof
if (read == -1)
{
break;
}
readCount += read;
}
}

if (readCount != header.length)
{
report.message(MessageId.PKG_003, EPUBLocation.create(epubFile.getName(), ""));
}
else
{
int fnsize = getIntFromBytes(header, 26);
int extsize = getIntFromBytes(header, 28);

if (header[0] != 'P' && header[1] != 'K')
{
report.message(MessageId.PKG_004, EPUBLocation.create(epubFile.getName()));
}
else if (fnsize != 8)
{
report.message(MessageId.PKG_006, EPUBLocation.create(epubFile.getName()));
}
else if (extsize != 0)
{
report.message(MessageId.PKG_005, EPUBLocation.create(epubFile.getName()), extsize);
}
else if (!CheckUtil.checkString(header, 30, "mimetype"))
{
report.message(MessageId.PKG_006, EPUBLocation.create(epubFile.getName()));
}
else if (!CheckUtil.checkString(header, 38, "application/epub+zip"))
{
report.message(MessageId.PKG_007, EPUBLocation.create("mimetype"));
}
}
}

private int getIntFromBytes(byte[] bytes, int offset)
{
int hi = 0xFF & bytes[offset + 1];
int lo = 0xFF & bytes[offset];
return hi << 8 | lo;
}
}
107 changes: 104 additions & 3 deletions src/main/java/com/adobe/epubcheck/ocf/OCFChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@

import static com.adobe.epubcheck.opf.ValidationContext.ValidationContextPredicates.*;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.text.Normalizer;
Expand Down Expand Up @@ -64,7 +66,6 @@
public class OCFChecker
{

@SuppressWarnings("unchecked")
private static final ValidatorMap validatorMap = ValidatorMap.builder()
.put(Predicates.and(path(OCFData.containerEntry), version(EPUBVersion.VERSION_2)),
XMLValidators.CONTAINER_20_RNG)
Expand Down Expand Up @@ -114,7 +115,11 @@ public void runChecks()
ocf.setReport(report);
if (!ocf.hasEntry(OCFData.containerEntry))
{
report.message(MessageId.RSC_002, EPUBLocation.create(ocf.getName()));
checkZipHeader(); // run ZIP header checks in any case before returning
// do not report the missing container entry if a fatal error was already reported
if (report.getFatalErrorCount() == 0) {
report.message(MessageId.RSC_002, EPUBLocation.create(ocf.getName()));
}
return;
}
long l = ocf.getTimeEntry(OCFData.containerEntry);
Expand All @@ -130,6 +135,7 @@ public void runChecks()
List<String> opfPaths = containerData.getEntries(OPFData.OPF_MIME_TYPE);
if (opfPaths == null || opfPaths.isEmpty())
{
checkZipHeader(); // run ZIP header checks in any case before returning
report.message(MessageId.RSC_003, EPUBLocation.create(OCFData.containerEntry));
return;
}
Expand Down Expand Up @@ -157,12 +163,14 @@ else if (opfPath.isEmpty())
}
else if (!ocf.hasEntry(opfPath))
{
checkZipHeader(); // run ZIP header checks in any case before returning
report.message(MessageId.OPF_002, EPUBLocation.create(OCFData.containerEntry), opfPath);
return;
}
}
if (rootfileErrorCounter == opfPaths.size())
{
checkZipHeader(); // run ZIP header checks in any case before returning
// end validation at this point when @full-path attribute is missing in
// container.xml
// otherwise, tons of errors would be thrown
Expand All @@ -179,7 +187,10 @@ else if (!ocf.hasEntry(opfPath))
EPUBVersion detectedVersion = null;
final EPUBVersion validationVersion;
OPFData opfData = ocf.getOpfData().get(opfPaths.get(0));
if (opfData == null) return;// The error must have been reported during
if (opfData == null) {
checkZipHeader(); // run ZIP header checks in any case before returning
return;// The error must have been reported during
}
// parsing
detectedVersion = opfData.getVersion();
report.info(null, FeatureEnum.FORMAT_VERSION, detectedVersion.toString());
Expand All @@ -197,6 +208,11 @@ else if (!ocf.hasEntry(opfPath))
validationVersion = detectedVersion;
}
newContextBuilder.version(validationVersion);

//
// Check the EPUB file header
// ------------------------------
checkZipHeader();

//
// Compute the validation profile
Expand Down Expand Up @@ -428,5 +444,90 @@ private void validateRenditionMapping(ValidationContext context)
}
parser.process();
}

private void checkZipHeader()
{
checkZipHeader(new File(context.path), report);
}

public static void checkZipHeader(File epubFile, Report report) {

FileInputStream epubIn = null;
try
{

epubIn = new FileInputStream(epubFile);

byte[] header = new byte[58];

int readCount = epubIn.read(header);
if (readCount != -1)
{
while (readCount < header.length)
{
int read = epubIn.read(header, readCount, header.length - readCount);
// break on eof
if (read == -1)
{
break;
}
readCount += read;
}
}

if (readCount != header.length)
{
report.message(MessageId.PKG_003, EPUBLocation.create(epubFile.getName(), ""));
}
else
{
int fnsize = getIntFromBytes(header, 26);
int extsize = getIntFromBytes(header, 28);

if (header[0] != 'P' && header[1] != 'K')
{
report.message(MessageId.PKG_004, EPUBLocation.create(epubFile.getName()));
}
else if (fnsize != 8)
{
report.message(MessageId.PKG_006, EPUBLocation.create(epubFile.getName()));
}
else if (extsize != 0)
{
report.message(MessageId.PKG_005, EPUBLocation.create(epubFile.getName()), extsize);
}
else if (!CheckUtil.checkString(header, 30, "mimetype"))
{
report.message(MessageId.PKG_006, EPUBLocation.create(epubFile.getName()));
}
else if (!CheckUtil.checkString(header, 38, "application/epub+zip"))
{
report.message(MessageId.PKG_007, EPUBLocation.create("mimetype"));
}
}
} catch (IOException e)
{
report.message(MessageId.PKG_008, EPUBLocation.create(epubFile.getName(), ""),
e.getMessage());
} finally
{
try
{
if (epubIn != null)
{
epubIn.close();
}
} catch (IOException ignored)
{
}
}
}

private static int getIntFromBytes(byte[] bytes, int offset)
{
int hi = 0xFF & bytes[offset + 1];
int lo = 0xFF & bytes[offset];
return hi << 8 | lo;
}

}
2 changes: 1 addition & 1 deletion src/test/java/com/adobe/epubcheck/api/Epub20CheckTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public void testValidateEPUBPBadOpfNamespace()
@Test
public void testValidateEPUB_mimetypeAndVersion()
{
Collections.addAll(expectedErrors, MessageId.PKG_006, MessageId.OPF_001);
Collections.addAll(expectedErrors, MessageId.OPF_001, MessageId.PKG_006);
Collections.addAll(expectedFatals, MessageId.OPF_019);
testValidateDocument("/invalid/mimetypeAndVersion.epub");
}
Expand Down
9 changes: 1 addition & 8 deletions src/test/java/com/adobe/epubcheck/api/Epub30CheckExpandedTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,7 @@ public void testValidateEPUBPLoremBasicMathml()
public void testValidateEPUBPLoremMimetype()
{
Collections.addAll(expectedErrors, MessageId.PKG_007);
testValidateDocument("invalid/lorem-mimetype");
}

@Test
public void testValidateEPUBPLoremMimetype2()
{
Collections.addAll(expectedErrors, MessageId.PKG_007);
testValidateDocument("invalid/lorem-mimetype-2");
testValidateDocument("invalid/mimetype-file-incorrect-value");
}

@Test
Expand Down
25 changes: 25 additions & 0 deletions src/test/java/com/adobe/epubcheck/cli/CLITest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,31 @@ public void testLocalizationWithNoLocale()
}));
}


@Test
public void testVersionMessageComesFirst()
{
PrintStream outOrig = System.out;
PrintStream errOrig = System.err;
CountingOutStream out = new CountingOutStream();
System.setOut(new PrintStream(out));
System.setErr(new PrintStream(out));
EpubChecker epubChecker = new EpubChecker();
epubChecker.run(
new String[] { getAbsoluteBasedir(expPath + "invalid/mimetype-file-incorrect-value"), "-mode", "exp", "--locale", "en" });
System.setOut(outOrig);
System.setErr(errOrig);
int versionMessageIndex = out.getValue().indexOf("Validating using");
int mimetypeErrorMessageIndex = out.getValue().indexOf("mimetype");
assertTrue("Version message should be in the output.",
versionMessageIndex >= 0);
assertTrue("PKG-007 should be in the output.",
mimetypeErrorMessageIndex >= 0);
assertTrue("Version message should be printed before PKG-007",
versionMessageIndex < mimetypeErrorMessageIndex);
}


private int run(String[] args, boolean verbose)
{
PrintStream outOrig = System.out;
Expand Down
6 changes: 0 additions & 6 deletions src/test/resources/30/expanded/invalid/lorem-mimetype-2/EPUB/lorem.css
View file

This file was deleted.

17 changes: 0 additions & 17 deletions src/test/resources/30/expanded/invalid/lorem-mimetype-2/EPUB/lorem.opf
View file

This file was deleted.

Loading

0 comments on commit 73b0ee8

Please sign in to comment.