fix: report fatalError when doctype is inside elements (xmldom#550)

which is a regression compared to 0.8.x, most likely introduced as part
of xmldom#498

- add check to `parseDoctypeCommentOrCData`
- drop redundant and broken `Element.appendChild` implementation
- `hasInsertableNodeType` now checks for `CharacterData` nodes instead
of only text nodes
- align ParseError and DOMException in how they are extending Error
- wrap `DOMException`s in `ParseError` in sax parser
- move custom errors to own module
  - and allow current and modern constructor arguments for DOMException

lib/conventions.js

@@ -391,22 +391,6 @@ var NAMESPACE = freeze({
 	XMLNS: 'http://www.w3.org/2000/xmlns/',
 });
 
-/**
- * Creates an error that will not be caught by XMLReader aka the SAX parser.
- *
- * @class
- * @param {string} message
- * @param {any} [locator]
- * Optional, can provide details about the location in the source.
- */
-function ParseError(message, locator) {
-	this.message = message;
-	this.locator = locator;
-	if (Error.captureStackTrace) Error.captureStackTrace(this, ParseError);
-}
-ParseError.prototype = new Error();
-ParseError.prototype.name = ParseError.name;
-
 exports.assign = assign;
 exports.find = find;
 exports.freeze = freeze;
@@ -422,4 +406,3 @@ exports.isHTMLVoidElement = isHTMLVoidElement;
 exports.isValidMimeType = isValidMimeType;
 exports.MIME_TYPE = MIME_TYPE;
 exports.NAMESPACE = NAMESPACE;
-exports.ParseError = ParseError;

lib/dom-parser.js

@@ -2,6 +2,7 @@
 
 var conventions = require('./conventions');
 var dom = require('./dom');
+var errors = require('./errors');
 var entities = require('./entities');
 var sax = require('./sax');
 
@@ -12,7 +13,7 @@ var isHTMLMimeType = conventions.isHTMLMimeType;
 var isValidMimeType = conventions.isValidMimeType;
 var MIME_TYPE = conventions.MIME_TYPE;
 var NAMESPACE = conventions.NAMESPACE;
-var ParseError = conventions.ParseError;
+var ParseError = errors.ParseError;
 
 var XMLReader = sax.XMLReader;
 

lib/errors.js

@@ -0,0 +1,196 @@
+'use strict';
+
+var conventions = require('./conventions');
+
+function extendError(constructor, writableName) {
+	constructor.prototype = Object.create(Error.prototype, {
+		constructor: { value: constructor },
+		name: { value: constructor.name, enumerable: true, writable: writableName },
+	});
+}
+
+var DOMExceptionName = conventions.freeze({
+	/**
+	 * the default value as defined by the spec
+	 */
+	Error: 'Error',
+	/**
+	 * @deprecated
+	 * Use RangeError instead.
+	 */
+	IndexSizeError: 'IndexSizeError',
+	/**
+	 * @deprecated
+	 * Just to match the related static code, not part of the spec.
+	 */
+	DomstringSizeError: 'DomstringSizeError',
+	HierarchyRequestError: 'HierarchyRequestError',
+	WrongDocumentError: 'WrongDocumentError',
+	InvalidCharacterError: 'InvalidCharacterError',
+	/**
+	 * @deprecated
+	 * Just to match the related static code, not part of the spec.
+	 */
+	NoDataAllowedError: 'NoDataAllowedError',
+	NoModificationAllowedError: 'NoModificationAllowedError',
+	NotFoundError: 'NotFoundError',
+	NotSupportedError: 'NotSupportedError',
+	InUseAttributeError: 'InUseAttributeError',
+	InvalidStateError: 'InvalidStateError',
+	SyntaxError: 'SyntaxError',
+	InvalidModificationError: 'InvalidModificationError',
+	NamespaceError: 'NamespaceError',
+	/**
+	 * @deprecated
+	 * Use TypeError for invalid arguments,
+	 * "NotSupportedError" DOMException for unsupported operations,
+	 * and "NotAllowedError" DOMException for denied requests instead.
+	 */
+	InvalidAccessError: 'InvalidAccessError',
+	/**
+	 * @deprecated
+	 * Just to match the related static code, not part of the spec.
+	 */
+	ValidationError: 'ValidationError',
+	/**
+	 * @deprecated
+	 * Use TypeError instead.
+	 */
+	TypeMismatchError: 'TypeMismatchError',
+	SecurityError: 'SecurityError',
+	NetworkError: 'NetworkError',
+	AbortError: 'AbortError',
+	/**
+	 * @deprecated
+	 * Just to match the related static code, not part of the spec.
+	 */
+	URLMismatchError: 'URLMismatchError',
+	QuotaExceededError: 'QuotaExceededError',
+	TimeoutError: 'TimeoutError',
+	InvalidNodeTypeError: 'InvalidNodeTypeError',
+	DataCloneError: 'DataCloneError',
+	EncodingError: 'EncodingError',
+	NotReadableError: 'NotReadableError',
+	UnknownError: 'UnknownError',
+	ConstraintError: 'ConstraintError',
+	DataError: 'DataError',
+	TransactionInactiveError: 'TransactionInactiveError',
+	ReadOnlyError: 'ReadOnlyError',
+	VersionError: 'VersionError',
+	OperationError: 'OperationError',
+	NotAllowedError: 'NotAllowedError',
+	OptOutError: 'OptOutError',
+});
+var DOMExceptionNames = Object.keys(DOMExceptionName);
+
+function isValidDomExceptionCode(value) {
+	return typeof value === 'number' && value >= 1 && value <= 25;
+}
+function endsWithError(value) {
+	return typeof value === 'string' && value.substring(value.length - DOMExceptionName.Error.length) === DOMExceptionName.Error;
+}
+/**
+ * DOM operations only raise exceptions in "exceptional" circumstances, i.e., when an operation
+ * is impossible to perform (either for logical reasons, because data is lost, or because the
+ * implementation has become unstable). In general, DOM methods return specific error values in
+ * ordinary processing situations, such as out-of-bound errors when using NodeList.
+ *
+ * Implementations should raise other exceptions under other circumstances. For example,
+ * implementations should raise an implementation-dependent exception if a null argument is
+ * passed when null was not expected.
+ *
+ * This implementation supports the following usages:
+ * 1. according to the living standard (both arguments are mandatory):
+ * ```
+ * new DOMException("message (can be empty)", DOMExceptionNames.HierarchyRequestError)
+ * ```
+ * 2. according to previous xmldom implementation (only the first argument is required):
+ * ```
+ * new DOMException(DOMException.HIERARCHY_REQUEST_ERR, "optional message")
+ * ```
+ * both result in the proper name being set.
+ *
+ * @class DOMException
+ * @param {number | string} messageOrCode
+ * The reason why an operation is not acceptable.
+ * If it is a number, it is used to determine the `name`, see
+ * {@link https://www.w3.org/TR/DOM-Level-3-Core/core.html#ID-258A00AF ExceptionCode}
+ * @param {string | keyof typeof DOMExceptionName | Error} [nameOrMessage]
+ * The `name` to use for the error.
+ * If `messageOrCode` is a number, this arguments is used as the `message` instead.
+ * @augments Error
+ * @see https://webidl.spec.whatwg.org/#idl-DOMException
+ * @see https://webidl.spec.whatwg.org/#dfn-error-names-table
+ * @see https://www.w3.org/TR/DOM-Level-3-Core/core.html#ID-17189187
+ * @see http://www.w3.org/TR/2000/REC-DOM-Level-2-Core-20001113/ecma-script-binding.html
+ * @see http://www.w3.org/TR/REC-DOM-Level-1/ecma-script-language-binding.html
+ */
+function DOMException(messageOrCode, nameOrMessage) {
+	// support old way of passing arguments: first argument is a valid number
+	if (isValidDomExceptionCode(messageOrCode)) {
+		this.name = DOMExceptionNames[messageOrCode];
+		this.message = nameOrMessage || '';
+	} else {
+		this.message = messageOrCode;
+		this.name = endsWithError(nameOrMessage) ? nameOrMessage : DOMExceptionName.Error;
+	}
+	if (Error.captureStackTrace) Error.captureStackTrace(this, DOMException);
+}
+extendError(DOMException, true);
+Object.defineProperties(DOMException.prototype, {
+	code: {
+		enumerable: true,
+		get: function () {
+			var code = DOMExceptionNames.indexOf(this.name);
+			if (isValidDomExceptionCode(code)) return code;
+			return 0;
+		},
+	},
+});
+
+DOMException.INDEX_SIZE_ERR = 1;
+DOMException.DOMSTRING_SIZE_ERR = 2;
+DOMException.HIERARCHY_REQUEST_ERR = 3;
+DOMException.WRONG_DOCUMENT_ERR = 4;
+DOMException.INVALID_CHARACTER_ERR = 5;
+DOMException.NO_DATA_ALLOWED_ERR = 6;
+DOMException.NO_MODIFICATION_ALLOWED_ERR = 7;
+DOMException.NOT_FOUND_ERR = 8;
+DOMException.NOT_SUPPORTED_ERR = 9;
+DOMException.INUSE_ATTRIBUTE_ERR = 10;
+DOMException.INVALID_STATE_ERR = 11;
+DOMException.SYNTAX_ERR = 12;
+DOMException.INVALID_MODIFICATION_ERR = 13;
+DOMException.NAMESPACE_ERR = 14;
+DOMException.INVALID_ACCESS_ERR = 15;
+DOMException.VALIDATION_ERR = 16;
+DOMException.TYPE_MISMATCH_ERR = 17;
+DOMException.SECURITY_ERR = 18;
+DOMException.NETWORK_ERR = 19;
+DOMException.ABORT_ERR = 20;
+DOMException.URL_MISMATCH_ERR = 21;
+DOMException.QUOTA_EXCEEDED_ERR = 22;
+DOMException.TIMEOUT_ERR = 23;
+DOMException.INVALID_NODE_TYPE_ERR = 24;
+DOMException.DATA_CLONE_ERR = 25;
+
+/**
+ * Creates an error that will not be caught by XMLReader aka the SAX parser.
+ *
+ * @class
+ * @param {string} message
+ * @param {any} [locator]
+ * @param {Error} [cause]
+ * Optional, can provide details about the location in the source.
+ */
+function ParseError(message, locator, cause) {
+	this.message = message;
+	this.locator = locator;
+	this.cause = cause;
+	if (Error.captureStackTrace) Error.captureStackTrace(this, ParseError);
+}
+extendError(ParseError);
+
+exports.DOMException = DOMException;
+exports.DOMExceptionName = DOMExceptionName;
+exports.ParseError = ParseError;

lib/index.js

@@ -1,16 +1,17 @@
 'use strict';
-
 var conventions = require('./conventions');
 exports.assign = conventions.assign;
 exports.hasDefaultHTMLNamespace = conventions.hasDefaultHTMLNamespace;
 exports.isHTMLMimeType = conventions.isHTMLMimeType;
 exports.isValidMimeType = conventions.isValidMimeType;
 exports.MIME_TYPE = conventions.MIME_TYPE;
 exports.NAMESPACE = conventions.NAMESPACE;
-exports.ParseError = conventions.ParseError;
+
+var errors = require('./errors');
+exports.ParseError = errors.ParseError;
+exports.DOMException = errors.DOMException;
 
 var dom = require('./dom');
-exports.DOMException = dom.DOMException;
 exports.DOMImplementation = dom.DOMImplementation;
 exports.XMLSerializer = dom.XMLSerializer;
 

lib/sax.js

@@ -2,12 +2,14 @@
 
 var conventions = require('./conventions');
 var g = require('./grammar');
+var errors = require('./errors');
 
 var isHTMLEscapableRawTextElement = conventions.isHTMLEscapableRawTextElement;
 var isHTMLMimeType = conventions.isHTMLMimeType;
 var isHTMLRawTextElement = conventions.isHTMLRawTextElement;
 var NAMESPACE = conventions.NAMESPACE;
-var ParseError = conventions.ParseError;
+var ParseError = errors.ParseError;
+var DOMException = errors.DOMException;
 
 //var handlers = 'resolveEntity,getExternalSubset,characters,endDocument,endElement,endPrefixMapping,ignorableWhitespace,processingInstruction,setDocumentLocator,skippedEntity,startDocument,startElement,startPrefixMapping,notationDecl,unparsedEntityDecl,error,fatalError,warning,attributeDecl,elementDecl,externalEntityDecl,internalEntityDecl,comment,endCDATA,endDTD,endEntity,startCDATA,startDTD,startEntity'.split(',')
 
@@ -231,6 +233,8 @@ function parse(source, defaultNSMapCopy, entityMap, domBuilder, errorHandler) {
 		} catch (e) {
 			if (e instanceof ParseError) {
 				throw e;
+			} else if (e instanceof DOMException) {
+				throw new ParseError(e.name + ': ' + e.message, domBuilder.locator, e);
 			}
 			errorHandler.error('element parse error: ' + e);
 			end = -1;
@@ -451,7 +455,6 @@ function parseElementStartPart(source, start, el, currentNSMap, entityReplacer,
 					}
 				}
 		} //end outer switch
-		//console.log('p++',p)
 		p++;
 	}
 }
@@ -780,13 +783,9 @@ function parseDoctypeCommentOrCData(source, start, domBuilder, errorHandler, isH
 			}
 		case 'D': {
 			// should be DOCTYPE
-			var doctype = {
-				name: undefined,
-				publicId: undefined,
-				systemId: undefined,
-				internalSubset: undefined,
-			};
-
+			if (domBuilder.doc && domBuilder.doc.documentElement) {
+				return errorHandler.fatalError('Doctype not allowed inside or after documentElement at position ' + p.getIndex());
+			}
 			if (!p.substringStartsWith(g.DOCTYPE_DECL_START)) {
 				return errorHandler.fatalError('Expected ' + g.DOCTYPE_DECL_START + ' at position ' + p.getIndex());
 			}
@@ -795,6 +794,12 @@ function parseDoctypeCommentOrCData(source, start, domBuilder, errorHandler, isH
 				return errorHandler.fatalError('Expected whitespace after ' + g.DOCTYPE_DECL_START + ' at position ' + p.getIndex());
 			}
 
+			var doctype = {
+				name: undefined,
+				publicId: undefined,
+				systemId: undefined,
+				internalSubset: undefined,
+			};
 			// Parse the DOCTYPE name
 			doctype.name = p.getMatch(g.Name);
 			if (!doctype.name)

test/dom-parser.test.js

@@ -2,8 +2,9 @@
 
 const { describe, expect, test } = require('@jest/globals');
 const { DOMParser } = require('../lib');
-const { assign, MIME_TYPE, NAMESPACE, ParseError } = require('../lib/conventions');
+const { assign, MIME_TYPE, NAMESPACE } = require('../lib/conventions');
 const { __DOMHandler, onErrorStopParsing, onWarningStopParsing } = require('../lib/dom-parser');
+const { ParseError } = require('../lib/errors');
 
 const NS_CUSTOM = 'custom-default-ns';
 
@@ -285,6 +286,17 @@ describe('DOMParser', () => {
 			expect(() => new DOMParser({ onError }).parseFromString('<!-- only comment -->', MIME_TYPE.XML_TEXT)).toThrow(ParseError);
 			expect(onError).toHaveBeenCalledWith('fatalError', expect.stringContaining('root'), expect.any(__DOMHandler));
 		});
+		test('should report fatalError when doctype is inside element', () => {
+			const onError = jest.fn();
+			expect(() =>
+				new DOMParser({ onError }).parseFromString('<root><!DOCTYPE root PUBLIC "pubId" "systemId"></root>', MIME_TYPE.XML_TEXT)
+			).toThrow(ParseError);
+			expect(onError).toHaveBeenCalledWith(
+				'fatalError',
+				expect.stringContaining('Doctype not allowed'),
+				expect.any(__DOMHandler)
+			);
+		});
 	});
 });