Skip to content

break tags inside html #440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Conversation

mariotaku
Copy link

@mariotaku mariotaku commented May 21, 2022

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

When I tried to embed following HTML:

<script>
function foo() {
}

foo();
</script>

into JS, then inline that JS into HTML, I ran into a problem.

The HTML was introduced as a string like below.

var code = "<script>function foo(){} foo();</script>";

When it was inlined into HTML, it became like this:

<html>
  <script>
  var code = "<script>function foo(){} foo();</script>";
  </script>
</html>

As you can see, HTML can't properly escape </script> inside JS code, thus syntax error occured.

This change is to break "<script>" or "</script>" to "<" + "script>" or "<" + "/script>", like this:

<html>
  <script>
  var code = "<" + "script>function foo(){} foo();<" + "/script>";
  </script>
</html>

Breaking Changes

As this was not provided as an option, it may look strange to developers.

Additional Info

I tried to find solutions to add such thing without modifying the code, but finally decided to make this a PR. If you think this should be better changed to an optional feature, please let me know.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 21, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: mariotaku / name: Mariotaku (77deb26)

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, looks like bug in a parse...

@mariotaku
Copy link
Author

This is how HTML processes code inside <script> tag. https://stackoverflow.com/questions/20162699/using-script-tags-within-script-break-the-script

Also I noticed this can be done in plugin that inline JavaScript, too. So If you think it's best to implement such feature elsewhere, I'll close this PR.

@alexander-akait
Copy link
Member

Can you provide example of html with </script> tag in string?

@mariotaku
Copy link
Author

mariotaku commented May 24, 2022

webpack.config.js:

const path = require('path');

const HtmlWebpackPlugin = require('html-webpack-plugin');
const InlineChunkHtmlPlugin = require('inline-chunk-html-plugin');

module.exports = (env) => [{
    target: 'web',
    mode: env.production ? 'production' : 'development',
    entry: {
        demo: './demo.js',
    },
    resolve: {
        extensions: ['.js'],
    },
    module: {
        rules: [
            {
                test: /\.html$/i,
                loader: "html-loader"
            },
        ],
    },
    output: {
        filename: 'js/[name]-[contenthash].js',
        path: path.resolve(__dirname, 'dist'),
        publicPath: '/',
        clean: true,
    },
    plugins: [
        new HtmlWebpackPlugin({
            filename: 'demo.html',
            chunks: ['demo'],
        }),
        new InlineChunkHtmlPlugin(HtmlWebpackPlugin, ['demo']),
    ],
}];

embed.html:

<script>
function foo() {
}

foo();
</script>

demo.js

const embed = require('./embed.html');

console.log(embed);

Output:

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <title>Webpack App</title>
  <meta name="viewport" content="width=device-width, initial-scale=1"><script>/******/ (() => { // webpackBootstrap
/******/ 	var __webpack_modules__ = ({

/***/ "./app/embed.html":
/*!************************!*\
  !*** ./app/embed.html ***!
  \************************/
/***/ ((__unused_webpack_module, __webpack_exports__, __webpack_require__) => {

"use strict";
__webpack_require__.r(__webpack_exports__);
/* harmony export */ __webpack_require__.d(__webpack_exports__, {
/* harmony export */   "default": () => (__WEBPACK_DEFAULT_EXPORT__)
/* harmony export */ });
// Module
var code = "<script>\r\n    function foo() {\r\n    }\r\n\r\n    foo();\r\n</script>";
// Exports
/* harmony default export */ const __WEBPACK_DEFAULT_EXPORT__ = (code);

/***/ })

/******/ 	});
/************************************************************************/
/******/ 	// The module cache
/******/ 	var __webpack_module_cache__ = {};
/******/ 	
/******/ 	// The require function
/******/ 	function __webpack_require__(moduleId) {
/******/ 		// Check if module is in cache
/******/ 		var cachedModule = __webpack_module_cache__[moduleId];
/******/ 		if (cachedModule !== undefined) {
/******/ 			return cachedModule.exports;
/******/ 		}
/******/ 		// Create a new module (and put it into the cache)
/******/ 		var module = __webpack_module_cache__[moduleId] = {
/******/ 			// no module.id needed
/******/ 			// no module.loaded needed
/******/ 			exports: {}
/******/ 		};
/******/ 	
/******/ 		// Execute the module function
/******/ 		__webpack_modules__[moduleId](module, module.exports, __webpack_require__);
/******/ 	
/******/ 		// Return the exports of the module
/******/ 		return module.exports;
/******/ 	}
/******/ 	
/************************************************************************/
/******/ 	/* webpack/runtime/define property getters */
/******/ 	(() => {
/******/ 		// define getter functions for harmony exports
/******/ 		__webpack_require__.d = (exports, definition) => {
/******/ 			for(var key in definition) {
/******/ 				if(__webpack_require__.o(definition, key) && !__webpack_require__.o(exports, key)) {
/******/ 					Object.defineProperty(exports, key, { enumerable: true, get: definition[key] });
/******/ 				}
/******/ 			}
/******/ 		};
/******/ 	})();
/******/ 	
/******/ 	/* webpack/runtime/hasOwnProperty shorthand */
/******/ 	(() => {
/******/ 		__webpack_require__.o = (obj, prop) => (Object.prototype.hasOwnProperty.call(obj, prop))
/******/ 	})();
/******/ 	
/******/ 	/* webpack/runtime/make namespace object */
/******/ 	(() => {
/******/ 		// define __esModule on exports
/******/ 		__webpack_require__.r = (exports) => {
/******/ 			if(typeof Symbol !== 'undefined' && Symbol.toStringTag) {
/******/ 				Object.defineProperty(exports, Symbol.toStringTag, { value: 'Module' });
/******/ 			}
/******/ 			Object.defineProperty(exports, '__esModule', { value: true });
/******/ 		};
/******/ 	})();
/******/ 	
/************************************************************************/
var __webpack_exports__ = {};
// This entry need to be wrapped in an IIFE because it need to be isolated against other modules in the chunk.
(() => {
/*!*********************!*\
  !*** ./app/demo.js ***!
  \*********************/
var html = __webpack_require__(/*! ./embed.html */ "./app/embed.html");

console.log(html);
})();

/******/ })()
;</script></head>
  <body>
  </body>
</html>

I will close the PR for now as I think this feature should be done in another plugin (which I did here . Sorry & thanks for your time!

@mariotaku mariotaku closed this May 24, 2022
@alexander-akait
Copy link
Member

@mariotaku I see, look we need to fix it here, sounds like a bug, you can't control <script> in your sources

@alexander-akait
Copy link
Member

Let's add test case

@mariotaku
Copy link
Author

@alexander-akait Could you allow the CI to run so we can see if the changes are good?

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for delay, can you add test case?

@mariotaku
Copy link
Author

OK. I'll write a test case to assert script tags are properly escaped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants