From 99de25d6ccd2392eeeba6ed7d794d47107c0ffd4 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Tue, 11 Sep 2018 15:16:07 +0200 Subject: [PATCH 1/2] Implement unit tests for the `isSameOrigin` and `createValidAbsoluteUrl` utility functions Moreover, mark the `isValidProtocol` function as private since it's only used in the utilities file and is not (meant to be) exported. --- src/shared/util.js | 7 +++-- test/unit/util_spec.js | 67 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/shared/util.js b/src/shared/util.js index b823e6bdc..d557c5d64 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -332,7 +332,7 @@ function isSameOrigin(baseUrl, otherUrl) { } // Checks if URLs use one of the whitelisted protocols, e.g. to avoid XSS. -function isValidProtocol(url) { +function _isValidProtocol(url) { if (!url) { return false; } @@ -349,7 +349,8 @@ function isValidProtocol(url) { } /** - * Attempts to create a valid absolute URL (utilizing `isValidProtocol`). + * Attempts to create a valid absolute URL. + * * @param {URL|string} url - An absolute, or relative, URL. * @param {URL|string} baseUrl - An absolute URL. * @returns Either a valid {URL}, or `null` otherwise. @@ -360,7 +361,7 @@ function createValidAbsoluteUrl(url, baseUrl) { } try { var absoluteUrl = baseUrl ? new URL(url, baseUrl) : new URL(url); - if (isValidProtocol(absoluteUrl)) { + if (_isValidProtocol(absoluteUrl)) { return absoluteUrl; } } catch (ex) { /* `new URL()` will throw on incorrect data. */ } diff --git a/test/unit/util_spec.js b/test/unit/util_spec.js index 87059f89e..f61a835dd 100644 --- a/test/unit/util_spec.js +++ b/test/unit/util_spec.js @@ -14,9 +14,9 @@ */ import { - bytesToString, getInheritableProperty, isArrayBuffer, isBool, isEmptyObj, - isNum, isSpace, isString, log2, ReadableStream, removeNullCharacters, - stringToBytes, stringToPDFString, URL + bytesToString, createValidAbsoluteUrl, getInheritableProperty, isArrayBuffer, + isBool, isEmptyObj, isNum, isSameOrigin, isSpace, isString, log2, + ReadableStream, removeNullCharacters, stringToBytes, stringToPDFString, URL } from '../../src/shared/util'; import { Dict, Ref } from '../../src/core/primitives'; import { XRefMock } from './test_utils'; @@ -323,4 +323,65 @@ describe('util', function() { expect(typeof url.href).toEqual('string'); }); }); + + describe('isSameOrigin', function() { + it('handles invalid base URLs', function() { + // The base URL is not valid. + expect(isSameOrigin('/foo', '/bar')).toEqual(false); + + // The base URL has no origin. + expect(isSameOrigin('blob:foo', '/bar')).toEqual(false); + }); + + it('correctly checks if the origin of both URLs matches', function() { + expect(isSameOrigin('https://www.mozilla.org/foo', + 'https://www.mozilla.org/bar')).toEqual(true); + expect(isSameOrigin('https://www.mozilla.org/foo', + 'https://www.example.com/bar')).toEqual(false); + }); + }); + + describe('createValidAbsoluteUrl', function() { + it('handles invalid URLs', function() { + expect(createValidAbsoluteUrl(undefined, undefined)).toEqual(null); + expect(createValidAbsoluteUrl(null, null)).toEqual(null); + expect(createValidAbsoluteUrl('/foo', '/bar')).toEqual(null); + }); + + it('handles URLs that do not use a whitelisted protocol', function() { + expect(createValidAbsoluteUrl('magnet:?foo', null)).toEqual(null); + }); + + it('correctly creates a valid URL for whitelisted protocols', function() { + // `http` protocol + expect(createValidAbsoluteUrl('http://www.mozilla.org/foo', null)) + .toEqual(new URL('http://www.mozilla.org/foo')); + expect(createValidAbsoluteUrl('/foo', 'http://www.mozilla.org')) + .toEqual(new URL('http://www.mozilla.org/foo')); + + // `https` protocol + expect(createValidAbsoluteUrl('https://www.mozilla.org/foo', null)) + .toEqual(new URL('https://www.mozilla.org/foo')); + expect(createValidAbsoluteUrl('/foo', 'https://www.mozilla.org')) + .toEqual(new URL('https://www.mozilla.org/foo')); + + // `ftp` protocol + expect(createValidAbsoluteUrl('ftp://www.mozilla.org/foo', null)) + .toEqual(new URL('ftp://www.mozilla.org/foo')); + expect(createValidAbsoluteUrl('/foo', 'ftp://www.mozilla.org')) + .toEqual(new URL('ftp://www.mozilla.org/foo')); + + // `mailto` protocol (base URLs have no meaning and should yield `null`) + expect(createValidAbsoluteUrl('mailto:foo@bar.baz', null)) + .toEqual(new URL('mailto:foo@bar.baz')); + expect(createValidAbsoluteUrl('/foo', 'mailto:foo@bar.baz')) + .toEqual(null); + + // `tel` protocol (base URLs have no meaning and should yield `null`) + expect(createValidAbsoluteUrl('tel:+0123456789', null)) + .toEqual(new URL('tel:+0123456789')); + expect(createValidAbsoluteUrl('/foo', 'tel:0123456789')) + .toEqual(null); + }); + }); }); From bf13c8a50bee52ac74cb65daa0f787a18ddbdc08 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Tue, 11 Sep 2018 15:27:15 +0200 Subject: [PATCH 2/2] Use the `const` keyword for constants in `src/shared/util.js` Moreover, move general constants to the top of the file, i.e., those that are not closely tied to a function in the file. --- src/shared/util.js | 55 +++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/src/shared/util.js b/src/shared/util.js index d557c5d64..48ed88be7 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -17,7 +17,8 @@ import './compatibility'; import { ReadableStream } from './streams_polyfill'; import { URL } from './url_polyfill'; -var FONT_IDENTITY_MATRIX = [0.001, 0, 0, 0.001, 0, 0]; +const IDENTITY_MATRIX = [1, 0, 0, 1, 0, 0]; +const FONT_IDENTITY_MATRIX = [0.001, 0, 0, 0.001, 0, 0]; const NativeImageDecoding = { NONE: 'none', @@ -37,7 +38,7 @@ const PermissionFlag = { PRINT_HIGH_QUALITY: 0x800, }; -var TextRenderingMode = { +const TextRenderingMode = { FILL: 0, STROKE: 1, FILL_STROKE: 2, @@ -50,13 +51,13 @@ var TextRenderingMode = { ADD_TO_PATH_FLAG: 4, }; -var ImageKind = { +const ImageKind = { GRAYSCALE_1BPP: 1, RGB_24BPP: 2, RGBA_32BPP: 3, }; -var AnnotationType = { +const AnnotationType = { TEXT: 1, LINK: 2, FREETEXT: 3, @@ -85,7 +86,7 @@ var AnnotationType = { REDACT: 26, }; -var AnnotationFlag = { +const AnnotationFlag = { INVISIBLE: 0x01, HIDDEN: 0x02, PRINT: 0x04, @@ -98,7 +99,7 @@ var AnnotationFlag = { LOCKEDCONTENTS: 0x200, }; -var AnnotationFieldFlag = { +const AnnotationFieldFlag = { READONLY: 0x0000001, REQUIRED: 0x0000002, NOEXPORT: 0x0000004, @@ -120,7 +121,7 @@ var AnnotationFieldFlag = { COMMITONSELCHANGE: 0x4000000, }; -var AnnotationBorderStyleType = { +const AnnotationBorderStyleType = { SOLID: 1, DASHED: 2, BEVELED: 3, @@ -128,7 +129,7 @@ var AnnotationBorderStyleType = { UNDERLINE: 5, }; -var StreamType = { +const StreamType = { UNKNOWN: 0, FLATE: 1, LZW: 2, @@ -141,7 +142,7 @@ var StreamType = { RL: 9, }; -var FontType = { +const FontType = { UNKNOWN: 0, TYPE1: 1, TYPE1C: 2, @@ -161,14 +162,14 @@ const VerbosityLevel = { INFOS: 5, }; -var CMapCompressionType = { +const CMapCompressionType = { NONE: 0, BINARY: 1, STREAM: 2, }; // All the possible operations for an operator list. -var OPS = { +const OPS = { // Intentionally start from 1 so it is easy to spot bad operators that will be // 0's. dependency: 1, @@ -264,6 +265,20 @@ var OPS = { constructPath: 91, }; +const UNSUPPORTED_FEATURES = { + unknown: 'unknown', + forms: 'forms', + javaScript: 'javaScript', + smask: 'smask', + shadingPattern: 'shadingPattern', + font: 'font', +}; + +const PasswordResponses = { + NEED_PASSWORD: 1, + INCORRECT_PASSWORD: 2, +}; + let verbosity = VerbosityLevel.WARNINGS; function setVerbosityLevel(level) { @@ -307,15 +322,6 @@ function assert(cond, msg) { } } -var UNSUPPORTED_FEATURES = { - unknown: 'unknown', - forms: 'forms', - javaScript: 'javaScript', - smask: 'smask', - shadingPattern: 'shadingPattern', - font: 'font', -}; - // Checks if URLs have the same origin. For non-HTTP based URLs, returns false. function isSameOrigin(baseUrl, otherUrl) { try { @@ -388,11 +394,6 @@ function getLookupTableFactory(initializer) { }; } -var PasswordResponses = { - NEED_PASSWORD: 1, - INCORRECT_PASSWORD: 2, -}; - var PasswordException = (function PasswordExceptionClosure() { function PasswordException(msg, code) { this.name = 'PasswordException'; @@ -693,8 +694,6 @@ function getInheritableProperty({ dict, key, getArray = false, return values; } -var IDENTITY_MATRIX = [1, 0, 0, 1, 0, 0]; - var Util = (function UtilClosure() { function Util() {} @@ -892,7 +891,7 @@ function toRomanNumerals(number, lowerCase = false) { return (lowerCase ? romanStr.toLowerCase() : romanStr); } -var PDFStringTranslateTable = [ +const PDFStringTranslateTable = [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x2D8, 0x2C7, 0x2C6, 0x2D9, 0x2DD, 0x2DB, 0x2DA, 0x2DC, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,