Skip to content

Commit

Permalink
Fail early, in modern GENERIC builds, if certain required browser f…
Browse files Browse the repository at this point in the history 
…unctionality is missing (issue 11762)

With two kind of builds now being produced, with/without translation/polyfills, it's unfortunately somewhat easy for users to accidentally pick the wrong one.

In the case where a user would attempt to use a modern build of PDF.js in an older browser, such as e.g. IE11, the failure would be immediate when the code is loaded (given the use of unsupported ECMAScript features).
However in some browsers/environments, in particular Node.js, a modern PDF.js build may load correctly and thus *appear* to function, only to fail for e.g. certain API calls. To hopefully lessen the support burden, and to try and improve things overall, this patch adds checks to ensure that a modern build of PDF.js cannot be used in browsers/environments which lack native support for critical functionality (such as e.g. `ReadableStream`). Hence we'll fail early, with an error message telling users to pick an ES5-compatible build instead.

To ensure that we actually test things better especially w.r.t. usage of the PDF.js library in Node.js environments, the `gulp npm-test` task as used by Node.js/Travis was changed (back) to test an ES5-compatible build.
(Since the bots still test the code as-is, without transpilation/polyfills, this shouldn't really be a problem as far as I can tell.)
As part of these changes there's now both `gulp lib` and `gulp lib-es5` build targets, similar to e.g. the generic builds, which thanks to some re-factoring only required adding a small amount of code.

*Please note:* While it's probably too early to tell if this will be a widespread issue, it's possible that this is the sort of patch that *may* warrant being `git cherry-pick`ed onto the current beta version (v2.4.456).
  • Loading branch information
@Snuffleupagus
Snuffleupagus committed Apr 1, 2020
1 parent ce17276 commit 7107045
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 88 deletions.
4 changes: 2 additions & 2 deletions external/dist/lib/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ pre-built library as found in e.g. the `/build`, `/web`, and `/image_decoders`
folders in the root of this repository.

Please note that the "lib" build target exists mostly to enable unit-testing in
Node.js/Travis, and that you'll need to handle e.g. any Node.js dependencies
yourself if using the files in this folder.
Node.js/Travis, and that you'll need to handle e.g. any necessary polyfills
and/or Node.js dependencies yourself if using the files in this folder.
175 changes: 94 additions & 81 deletions gulpfile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1150,92 +1150,105 @@ gulp.task("jsdoc", function(done) {
});
});

function buildLib(defines, dir) {
// When we create a bundle, webpack is run on the source and it will replace
// require with __webpack_require__. When we want to use the real require,
// __non_webpack_require__ has to be used.
// In this target, we don't create a bundle, so we have to replace the
// occurences of __non_webpack_require__ ourselves.
function babelPluginReplaceNonWebPackRequire(babel) {
return {
visitor: {
Identifier(curPath, state) {
if (curPath.node.name === "__non_webpack_require__") {
curPath.replaceWith(babel.types.identifier("require"));
}
},
},
};
}
function preprocess(content) {
var skipBabel =
bundleDefines.SKIP_BABEL || /\/\*\s*no-babel-preset\s*\*\//.test(content);
content = preprocessor2.preprocessPDFJSCode(ctx, content);
content = babel.transform(content, {
sourceType: "module",
presets: skipBabel ? undefined : ["@babel/preset-env"],
plugins: [
"@babel/plugin-transform-modules-commonjs",
[
"@babel/plugin-transform-runtime",
{
helpers: false,
regenerator: true,
},
],
babelPluginReplaceNonWebPackRequire,
],
}).code;
var removeCjsSrc = /^(var\s+\w+\s*=\s*(_interopRequireDefault\()?require\(".*?)(?:\/src)(\/[^"]*"\)\)?;)$/gm;
content = content.replace(removeCjsSrc, (all, prefix, interop, suffix) => {
return prefix + suffix;
});
return licenseHeaderLibre + content;
}
var babel = require("@babel/core");
var versionInfo = getVersionJSON();
var bundleDefines = builder.merge(defines, {
BUNDLE_VERSION: versionInfo.version,
BUNDLE_BUILD: versionInfo.commit,
TESTING: process.env["TESTING"] === "true",
});
var ctx = {
rootPath: __dirname,
saveComments: false,
defines: bundleDefines,
map: {
"pdfjs-lib": "../pdf",
},
};
var licenseHeaderLibre = fs
.readFileSync("./src/license_header_libre.js")
.toString();
var preprocessor2 = require("./external/builder/preprocessor2.js");
return merge([
gulp.src(
[
"src/{core,display,shared}/*.js",
"!src/shared/{cffStandardStrings,fonts_utils}.js",
"src/{pdf,pdf.worker}.js",
],
{ base: "src/" }
),
gulp.src(
["examples/node/domstubs.js", "web/*.js", "!web/{pdfjs,viewer}.js"],
{ base: "." }
),
gulp.src("test/unit/*.js", { base: "." }),
])
.pipe(transform("utf8", preprocess))
.pipe(gulp.dest(dir));
}

gulp.task(
"lib",
gulp.series("buildnumber", "default_preferences", function() {
// When we create a bundle, webpack is run on the source and it will replace
// require with __webpack_require__. When we want to use the real require,
// __non_webpack_require__ has to be used.
// In this target, we don't create a bundle, so we have to replace the
// occurences of __non_webpack_require__ ourselves.
function babelPluginReplaceNonWebPackRequire(babel) {
return {
visitor: {
Identifier(curPath, state) {
if (curPath.node.name === "__non_webpack_require__") {
curPath.replaceWith(babel.types.identifier("require"));
}
},
},
};
}
function preprocess(content) {
var skipBabel =
bundleDefines.SKIP_BABEL ||
/\/\*\s*no-babel-preset\s*\*\//.test(content);
content = preprocessor2.preprocessPDFJSCode(ctx, content);
content = babel.transform(content, {
sourceType: "module",
presets: skipBabel ? undefined : ["@babel/preset-env"],
plugins: [
"@babel/plugin-transform-modules-commonjs",
[
"@babel/plugin-transform-runtime",
{
helpers: false,
regenerator: true,
},
],
babelPluginReplaceNonWebPackRequire,
],
}).code;
var removeCjsSrc = /^(var\s+\w+\s*=\s*(_interopRequireDefault\()?require\(".*?)(?:\/src)(\/[^"]*"\)\)?;)$/gm;
content = content.replace(
removeCjsSrc,
(all, prefix, interop, suffix) => {
return prefix + suffix;
}
);
return licenseHeaderLibre + content;
}
var babel = require("@babel/core");
var versionInfo = getVersionJSON();
var bundleDefines = builder.merge(DEFINES, {
var defines = builder.merge(DEFINES, { GENERIC: true, LIB: true });

return buildLib(defines, "build/lib/");
})
);

gulp.task(
"lib-es5",
gulp.series("buildnumber", "default_preferences", function() {
var defines = builder.merge(DEFINES, {
GENERIC: true,
LIB: true,
BUNDLE_VERSION: versionInfo.version,
BUNDLE_BUILD: versionInfo.commit,
TESTING: process.env["TESTING"] === "true",
SKIP_BABEL: false,
});
var ctx = {
rootPath: __dirname,
saveComments: false,
defines: bundleDefines,
map: {
"pdfjs-lib": "../pdf",
},
};
var licenseHeaderLibre = fs
.readFileSync("./src/license_header_libre.js")
.toString();
var preprocessor2 = require("./external/builder/preprocessor2.js");
return merge([
gulp.src(
[
"src/{core,display,shared}/*.js",
"!src/shared/{cffStandardStrings,fonts_utils}.js",
"src/{pdf,pdf.worker}.js",
],
{ base: "src/" }
),
gulp.src(
["examples/node/domstubs.js", "web/*.js", "!web/{pdfjs,viewer}.js"],
{ base: "." }
),
gulp.src("test/unit/*.js", { base: "." }),
])
.pipe(transform("utf8", preprocess))
.pipe(gulp.dest("build/lib/"));

return buildLib(defines, "build/lib-es5/");
})
);

Expand Down Expand Up @@ -1371,7 +1384,7 @@ gulp.task("baseline", function(done) {

gulp.task(
"unittestcli",
gulp.series("testing-pre", "lib", function(done) {
gulp.series("testing-pre", "lib-es5", function(done) {
var options = [
"node_modules/jasmine/bin/jasmine",
"JASMINE_CONFIG_PATH=test/unit/clitests.json",
Expand Down
15 changes: 15 additions & 0 deletions src/core/worker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,21 @@ var WorkerMessageHandler = {
"; thus breaking e.g. `for...in` iteration of `Array`s."
);
}

// Ensure that (primarily) Node.js users won't accidentally attempt to use
// a non-translated/non-polyfilled build of the library, since that would
// quickly fail anyway because of missing functionality (such as e.g.
// `ReadableStream).
if (
(typeof PDFJSDev === "undefined" || PDFJSDev.test("SKIP_BABEL")) &&
typeof ReadableStream === "undefined"
) {
throw new Error(
"The browser/environment lacks native support for critical " +
"functionality used by the PDF.js library (e.g. `ReadableStream`); " +
"please use an ES5-compatible build instead."
);
}
}

var docId = docParams.docId;
Expand Down
6 changes: 2 additions & 4 deletions src/shared/compatibility.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,9 @@
*/
/* eslint no-var: error */

// Skip compatibility checks for modern builds (unless we're running the
// unit-tests in Node.js/Travis) and if we already ran the module.
// Skip compatibility checks for modern builds and if we already ran the module.
if (
(typeof PDFJSDev === "undefined" ||
PDFJSDev.test("!SKIP_BABEL || (LIB && TESTING)")) &&
(typeof PDFJSDev === "undefined" || !PDFJSDev.test("SKIP_BABEL")) &&
(typeof globalThis === "undefined" || !globalThis._pdfjsCompatibilityChecked)
) {
// Provides support for globalThis in legacy browsers.
Expand Down
2 changes: 1 addition & 1 deletion test/unit/clitests.json
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"spec_dir": "build/lib/test/unit",
"spec_dir": "build/lib-es5/test/unit",

"helpers": [
"clitests_helper.js"
Expand Down

0 comments on commit 7107045

Please sign in to comment.