1
0
Fork 0
mirror of https://github.com/mozilla/pdf.js.git synced 2025-04-26 10:08:06 +02:00

Re-factor the handling of *empty* Name-instances (PR 13612 follow-up)

When working on PR 13612, I mostly prioritized a simple solution that didn't require touching a lot of code. However, while working on PR 13735 I started to realize that the static `Name.empty` construction really wasn't a good idea.

In particular, having a special `Name`-instance where the `name`-property isn't actually a String is confusing (to put it mildly) and can easily lead to issues elsewhere. The only reason for not simply allowing the `name`-property to be an *empty* string, in PR 13612, was to avoid having to touch a lot of existing code. However, it turns out that this is only limited to a few methods in the `PartialEvaluator` and a few of the `BaseLocalCache`-implementations, all of which can be easily re-factored to handle *empty* `Name`-instances.

All-in-all, I think that this patch is even an *overall* improvement since we're now validating (what should always be) `Name`-data better in the `PartialEvaluator`.
This is what I ought to have done from the start, sorry about the code churn here!
This commit is contained in:
Jonas Jenwald 2021-07-14 21:38:19 +02:00
parent 64f86de5cb
commit 3838c4e27c
5 changed files with 39 additions and 21 deletions

View file

@ -1540,7 +1540,7 @@ class PartialEvaluator {
timeSlotManager.reset();
const operation = {};
let stop, i, ii, cs, name;
let stop, i, ii, cs, name, isValidName;
while (!(stop = timeSlotManager.check())) {
// The arguments parsed by read() are used beyond this loop, so we
// cannot reuse the same array on each iteration. Therefore we pass
@ -1556,8 +1556,10 @@ class PartialEvaluator {
switch (fn | 0) {
case OPS.paintXObject:
// eagerly compile XForm objects
isValidName = args[0] instanceof Name;
name = args[0].name;
if (name) {
if (isValidName) {
const localImage = localImageCache.getByName(name);
if (localImage) {
operatorList.addOp(localImage.fn, localImage.args);
@ -1568,7 +1570,7 @@ class PartialEvaluator {
next(
new Promise(function (resolveXObject, rejectXObject) {
if (!name) {
if (!isValidName) {
throw new FormatError("XObject must be referred to by name.");
}
@ -1922,8 +1924,10 @@ class PartialEvaluator {
fn = OPS.shadingFill;
break;
case OPS.setGState:
isValidName = args[0] instanceof Name;
name = args[0].name;
if (name) {
if (isValidName) {
const localGStateObj = localGStateCache.getByName(name);
if (localGStateObj) {
if (localGStateObj.length > 0) {
@ -1936,7 +1940,7 @@ class PartialEvaluator {
next(
new Promise(function (resolveGState, rejectGState) {
if (!name) {
if (!isValidName) {
throw new FormatError("GState must be referred to by name.");
}
@ -2823,14 +2827,16 @@ class PartialEvaluator {
xobjs = resources.get("XObject") || Dict.empty;
}
var isValidName = args[0] instanceof Name;
var name = args[0].name;
if (name && emptyXObjectCache.getByName(name)) {
if (isValidName && emptyXObjectCache.getByName(name)) {
break;
}
next(
new Promise(function (resolveXObject, rejectXObject) {
if (!name) {
if (!isValidName) {
throw new FormatError("XObject must be referred to by name.");
}
@ -2935,14 +2941,16 @@ class PartialEvaluator {
);
return;
case OPS.setGState:
isValidName = args[0] instanceof Name;
name = args[0].name;
if (name && emptyGStateCache.getByName(name)) {
if (isValidName && emptyGStateCache.getByName(name)) {
break;
}
next(
new Promise(function (resolveGState, rejectGState) {
if (!name) {
if (!isValidName) {
throw new FormatError("GState must be referred to by name.");
}