From ba7901822a57b7dffceccb79dd4d5758fb02808c Mon Sep 17 00:00:00 2001 From: "Louis (loco)" Date: Mon, 4 Dec 2023 10:15:03 +0000 Subject: [PATCH] [FIX] web_editor: make loadImageInfo() robust to protocol relative URLs Steps to reproduce: - Add a "Text-Image" on the website. - Replace the image by one of your own. - Save. - With the html editor, remove the `mimetype` attribute, the `data-original-src` attribute and change the `src` of the picture into the corresponding protocol-relative one. The `src` then looks like "//domain/web/image/...". - Save the modifications of the html editor. - Enter in edit mode. - Click on the image. -> It is now impossible to change options such as `Filter`, `Width` and `Quality`. Because the `data-original-src` attribute was removed from the image, the system tries to add it back thanks to the `loadImageInfo()` logic. Because the `src` attribute of the image is now a protocol relative url, `new URL(src)` will raise an error and the logic will use this url as argument for the `/web_editor/get_image_info` route. Because the url given in argument of the rpc call is not the relative one, the system fails to find the original attachment. The `mimetype` attribute is therefore not added back on the image, leading to the impossibility to change some options. To solve the problem, this commit modifies a bit [this commit]. In order to be robust to absolute, relative and protocol relative URLs, an URL object is first created from the image src. The relative URL (`.pathname`) of the URL object is then used to retrieve the original attachment linked to the image. Let's synthesize the different `relativeSrc` obtained with different image src. In the following examples, "https://test.com/blog/travel-1" will be used as `img.ownerDocument.defaultView.location.href`. (the complete URL of the document in which the image is located). - `src` is an absolute URL (e.g. "https://test.com/web/image/697-d0f2aaf8/shoes.jpg"). In this case, `relativeSrc` = "/web/image/697-d0f2aaf8/shoes.jpg". - `src` is a relative URL that begins with a slash (e.g. "/web/image/697-d0f2aaf8/shoes.jpg"). This URL represents an absolute path starting from the root of the domain. In this case, `relativeSrc` = "/web/image/697-d0f2aaf8/shoes.jpg". - `src` is a relative URL that does not begin with a slash (e.g. "web/image/697-d0f2aaf8/shoes.jpg"). The interpretation of this URL depends on the current location. In this case, `relativeSrc` = "/blog/web/image/697-d0f2aaf8/shoes.jpg". - `src` is a protocol relative URL (e.g. "//test.com/web/image/697-d0f2aaf8/shoes.jpg"); there is only the protocol missing. In this case, `relativeSrc` = "/web/image/697-d0f2aaf8/shoes.jpg". This solution takes the advantage of the second argument of the `URL()` constructor which is used if the first parameter is a relative or protocol relative URL and which is ignored if the first parameter is an absolute URL. This commit does not only modify [this commit] to handle more types of URLs but also: - To avoid having to use `.split()`. Indeed, `.pathname` does not include query parameters. - To avoid having to consider an error raised by `new URL()` as a normal flow. Indeed, in [this commit], an error would be intercepted by the `catch` if `src` was a relative URL. This was a legitimate flow. The problem was that other unwanted types of src (for example protocol relative URL) were also raising errors but were silently ignored (as intercepted in the `catch`). [this commit]: https://github.com/odoo/odoo/commit/89c14783846288a2de53f6258a93440e02550b13 task-3623731 closes odoo/odoo#146238 X-original-commit: 96324fe8443647a078756c98f9629af274ff38a5 Signed-off-by: Quentin Smetz (qsm) Signed-off-by: Colin Louis (loco) --- .../static/src/js/editor/image_processing.js | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/addons/web_editor/static/src/js/editor/image_processing.js b/addons/web_editor/static/src/js/editor/image_processing.js index 0aa452236f4..9bf3dc0aa94 100644 --- a/addons/web_editor/static/src/js/editor/image_processing.js +++ b/addons/web_editor/static/src/js/editor/image_processing.js @@ -448,18 +448,16 @@ export async function loadImageInfo(img, rpc, attachmentSrc = '') { if ((img.dataset.originalSrc && img.dataset.mimetypeBeforeConversion) || !src) { return; } + // In order to be robust to absolute, relative and protocol relative URLs, + // the src of the img is first converted to an URL object. To do so, the URL + // of the document in which the img is located is used as a base to build + // the URL object if the src of the img is a relative or protocol relative + // URL. The original attachment linked to the img is then retrieved thanks + // to the path of the built URL object. + const srcUrl = new URL(src, img.ownerDocument.defaultView.location.href); + const relativeSrc = srcUrl.pathname; - // Only consider the "relative" part of the URL. Needed because some - // relative URLs were wrongly converted to absolute URLs at some point and - // user domains could have been changed meanwhile. - let relativeSrc; - try { - const srcUrl = new URL(src); - relativeSrc = srcUrl.pathname; - } catch { - relativeSrc = src; - } - const {original} = await rpc('/web_editor/get_image_info', {src: relativeSrc.split(/[?#]/)[0]}); + const {original} = await rpc('/web_editor/get_image_info', {src: relativeSrc}); // If src was an absolute "external" URL, we consider unlikely that its // relative part matches something from the DB and even if it does, nothing // bad happens, besides using this random image as the original when using