[sitecore-jss] [mediaApi] updateImageUrl fix for invalid hash errors on Sitecore

docs/data/routes/release-notes/en.md

@@ -22,7 +22,7 @@ There are [migration instructions](/upgrade-guides/18.0) from JSS 16-based appli
 	* [React-Native sample] Sitecore logo in header is touchable and will navigate to Home page when click on it.
 	* [React-Native sample] Removed usage of `dataConversation`.
 * [PR #638](https://github.com/Sitecore/jss/pull/638) [samples/nextjs] The sitemap fetcher in `getStaticPaths` now uses pagination to fetch all pages, and the default `pageSize` value can be customized.
-* [PR #645](https://github.com/Sitecore/jss/pull/645) Debug logging for SDKs (focused on Next.js) using the [debug](https://www.npmjs.com/package/debug) module.
+* [PR #645](https://github.com/Sitecore/jss/pull/645) [Debug logging](/docs/fundamentals/troubleshooting/debug-logging) for SDKs (focused on Next.js) using the [debug](https://www.npmjs.com/package/debug) module.
 
 ### Bug Fixes
 
@@ -37,6 +37,7 @@ There are [migration instructions](/upgrade-guides/18.0) from JSS 16-based appli
 	* [React-Native sample] Fix connected tunnel mode for secure (https) Sitecore endpoints.
 * [PR #638](https://github.com/Sitecore/jss/pull/638) [sitecore-jss-nextjs] [samples/nextjs] Fix issue with `getStaticPaths`only pre-rendering the first 10 pages.
 * [PR #677](https://github.com/Sitecore/jss/pull/677) [sitecore-jss-forms] Fix issue where pre-filled (default) form data isn't removed for multi-valued fields when user de-selects values.
+* [PR #681](https://github.com/Sitecore/jss/pull/681) [sitecore-jss] [mediaApi] `updateImageUrl` now *only* switches to JSS media handler prefix if `imageParams` are sent. Otherwise, the original media URL is returned. Fixes hash errors ("ERROR MediaRequestProtection: An invalid/missing hash value was encountered.") from appearing in Sitecore logs.
 
 ## Sitecore JSS 16.0 for Sitecore 10.1
 

packages/sitecore-jss-react-native/src/components/__snapshots__/Image.test.tsx.snap

@@ -6,7 +6,7 @@ exports[`<Image /> with network images with direct image object, no value/editab
   resizeMode="cover"
   source={
     Object {
-      "uri": "https://jssapp/-/jssmedia/myfile.ashx&w=180&h=360",
+      "uri": "https://jssapp/-/media/myfile.ashx&w=180&h=360",
     }
   }
   style={
@@ -24,7 +24,7 @@ exports[`<Image /> with network images with style prop with width or height spec
   resizeMode="cover"
   source={
     Object {
-      "uri": "https://jssapp/-/jssmedia/myfile.ashx&w=180&h=360",
+      "uri": "https://jssapp/-/media/myfile.ashx&w=180&h=360",
     }
   }
   style={
@@ -47,7 +47,7 @@ exports[`<Image /> with network images with value/editable image object should r
   resizeMode="cover"
   source={
     Object {
-      "uri": "https://jssapp/-/jssmedia/myfile.ashx&w=180&h=360",
+      "uri": "https://jssapp/-/media/myfile.ashx&w=180&h=360",
     }
   }
   style={

packages/sitecore-jss-react/.vscode/launch.json

@@ -3,19 +3,19 @@
   "configurations": [
     {
       "name": "Run mocha",
-      "type": "node2",
+      "type": "node",
       "request": "launch",
       "program": "${workspaceRoot}/node_modules/mocha/bin/_mocha",
       "stopOnEntry": false,
       "args": [
-        "--compilers",
-        "js:babel-core/register",
         "--require",
-        "./src/tests/jsdom-setup.js",
-        "./src/tests/enzyme-setup.js",
-        "./src/**/*.test.js",
+        "ts-node/register",
+        "./src/tests/jsdom-setup.ts",
+        "./src/tests/enzyme-setup.ts",
         "--no-timeouts",
-        "./dist/test"
+        "--colors",
+        "./src/**/*.test.ts",
+        "./src/**/*.test.tsx"
       ],
       "cwd": "${workspaceRoot}",
       "runtimeExecutable": null

packages/sitecore-jss-react/src/components/Image.test.tsx

@@ -181,18 +181,19 @@ describe('<Image />', () => {
         id: 'some-id',
         style: { width: '100%' },
         className: 'the-dude-abides',
+        imageParams: { foo: 'bar' },
         mediaUrlPrefix: /\/([-~]{1})assets\//i,
       };
       const rendered = mount(<Image {...props} />);
 
-      expect(rendered.find('img').prop('src')).to.equal('/~/jssmedia/img/test0.png');
+      expect(rendered.find('img').prop('src')).to.equal('/~/jssmedia/img/test0.png?foo=bar');
 
       rendered.setProps({
         ...props,
         media: { value: { src: '/-assets/img/test0.png', alt: 'my image' } },
       });
 
-      expect(rendered.find('img').prop('src')).to.equal('/-/jssmedia/img/test0.png');
+      expect(rendered.find('img').prop('src')).to.equal('/-/jssmedia/img/test0.png?foo=bar');
     });
 
     it('should transform url with direct image object, no value/editable', () => {
@@ -207,11 +208,12 @@ describe('<Image />', () => {
           width: '100%',
         },
         className: 'the-dude-abides',
+        imageParams: { foo: 'bar' },
         mediaUrlPrefix: /\/([-~]{1})assets\//i,
       };
       const rendered = mount(<Image {...props} />);
 
-      expect(rendered.find('img').prop('src')).to.equal('/~/jssmedia/img/test0.png');
+      expect(rendered.find('img').prop('src')).to.equal('/~/jssmedia/img/test0.png?foo=bar');
 
       rendered.setProps({
         ...props,
@@ -222,7 +224,7 @@ describe('<Image />', () => {
         },
       });
 
-      expect(rendered.find('img').prop('src')).to.equal('/-/jssmedia/img/test0.png');
+      expect(rendered.find('img').prop('src')).to.equal('/-/jssmedia/img/test0.png?foo=bar');
     });
 
     it('should transform url with responsive image object', () => {
@@ -239,7 +241,10 @@ describe('<Image />', () => {
 
       const rendered = mount(<Image {...props} />);
 
-      expect(rendered.find('img').prop('src')).to.equal('/~/jssmedia/img/test0.png');
+      expect(rendered.find('img').prop('src')).to.equal('/~assets/img/test0.png');
+      expect(rendered.find('img').prop('srcSet')).to.equal(
+        '/~/jssmedia/img/test0.png?mw=100 100w, /~/jssmedia/img/test0.png?mw=300 300w'
+      );
 
       rendered.setProps({
         ...props,
@@ -248,9 +253,13 @@ describe('<Image />', () => {
           width: 8,
           height: 10,
         },
+        imageParams: { foo: 'bar' },
       });
 
-      expect(rendered.find('img').prop('src')).to.equal('/-/jssmedia/img/test0.png');
+      expect(rendered.find('img').prop('src')).to.equal('/-/jssmedia/img/test0.png?foo=bar');
+      expect(rendered.find('img').prop('srcSet')).to.equal(
+        '/-/jssmedia/img/test0.png?foo=bar&mw=100 100w, /-/jssmedia/img/test0.png?foo=bar&mw=300 300w'
+      );
     });
   });
 

packages/sitecore-jss/src/media-api.test.ts

@@ -64,26 +64,57 @@ describe('getRequiredParams', () => {
 });
 
 describe('updateImageUrl', () => {
+  it('should return original if no params', () => {
+    const original =
+      'http://sitecore/-/media/lorem/ipsum.jpg?h=1&w=2&hash=CC5043DC03C6C27F40EDB08CF84AB8670C05D63D';
+    const updated = updateImageUrl(original);
+    expect(updated).to.equal(original);
+  });
+
   it('should override parameters with those provided', () => {
+    const original = 'http://sitecore/-/media/lorem.png?mh=3&mw=4';
+    const updated = updateImageUrl(original, { mh: '5', mw: '6' });
+    const url = URL(updated, true);
+    expect(url.query.mh).to.equal('5');
+    expect(url.query.mw).to.equal('6');
+  });
+
+  it('should remove non-required query parameters not provided', () => {
     const original =
-      'http://sitecore/-/media/lorem.png?h=1&w=2&mh=3&mw=4hash=CC5043DC03C6C27F40EDB08CF84AB8670C05D63D';
+      'http://sitecore/-/media/lorem.png?h=1&w=2&mh=3&mw=4&hash=CC5043DC03C6C27F40EDB08CF84AB8670C05D63D';
     const updated = updateImageUrl(original, { mh: '5', mw: '6' });
     const url = URL(updated, true);
     expect(url.query.mh).to.equal('5');
     expect(url.query.mw).to.equal('6');
-    expect(url.query.hash).to.be.undefined; // eslint-disable-line no-unused-expressions
+    expect(url.query.h).to.be.undefined;
+    expect(url.query.w).to.be.undefined;
+    expect(url.query.hash).to.be.undefined;
+  });
+
+  it('should preserve required query parameters', () => {
+    const original =
+      'http://sitecore/-/media/lorem.png?rev=100&db=master&la=en&vs=200&ts=foo&mh=3&mw=4';
+    const updated = updateImageUrl(original, { mh: '5', mw: '6' });
+    const url = URL(updated, true);
+    expect(url.query.mh).to.equal('5');
+    expect(url.query.mw).to.equal('6');
+    expect(url.query.rev).to.equal('100');
+    expect(url.query.db).to.equal('master');
+    expect(url.query.la).to.equal('en');
+    expect(url.query.vs).to.equal('200');
+    expect(url.query.ts).to.equal('foo');
   });
 
   it('should replace /-/media/ with /-/jssmedia/', () => {
     const original = 'http://sitecore/-/media/lorem/ipsum.jpg';
-    const updated = updateImageUrl(original);
+    const updated = updateImageUrl(original, { foo: 'bar' });
     const url = URL(updated);
     expect(url.pathname).to.startsWith('/-/jssmedia/');
   });
 
   it('should replace /~/media/ with /~/jssmedia/', () => {
     const original = 'http://sitecore/~/media/lorem/ipsum.jpg';
-    const updated = updateImageUrl(original);
+    const updated = updateImageUrl(original, { foo: 'bar' });
     const url = URL(updated);
     expect(url.pathname).to.startsWith('/~/jssmedia/');
   });
@@ -92,31 +123,31 @@ describe('updateImageUrl', () => {
     it('should replace /-assets/ with /-/jssmedia', () => {
       const original = 'http://sitecore/-assets/lorem/ipsum.jpg';
       const mediaUrlPrefix = /\/([-~]{1})assets\//i;
-      const updated = updateImageUrl(original, undefined, mediaUrlPrefix);
+      const updated = updateImageUrl(original, { foo: 'bar' }, mediaUrlPrefix);
       const url = URL(updated);
       expect(url.pathname).to.startsWith('/-/jssmedia/');
     });
 
     it('should replace /~assets/ with /~/jssmedia', () => {
       const original = 'http://sitecore/~assets/lorem/ipsum.jpg';
       const mediaUrlPrefix = /\/([-~]{1})assets\//i;
-      const updated = updateImageUrl(original, undefined, mediaUrlPrefix);
+      const updated = updateImageUrl(original, { foo: 'bar' }, mediaUrlPrefix);
       const url = URL(updated);
       expect(url.pathname).to.startsWith('/~/jssmedia/');
     });
 
     it('should replace /-/assets/ with /-/jssmedia/', () => {
       const original = 'http://sitecore/-/assets/lorem/ipsum.jpg';
       const mediaUrlPrefix = /\/([-~]{1})\/assets\//i;
-      const updated = updateImageUrl(original, undefined, mediaUrlPrefix);
+      const updated = updateImageUrl(original, { foo: 'bar' }, mediaUrlPrefix);
       const url = URL(updated);
       expect(url.pathname).to.startsWith('/-/jssmedia/');
     });
 
     it('should replace /~/assets/ with /~/jssmedia/', () => {
       const original = 'http://sitecore/~/assets/lorem/ipsum.jpg';
       const mediaUrlPrefix = /\/([-~]{1})\/assets\//i;
-      const updated = updateImageUrl(original, undefined, mediaUrlPrefix);
+      const updated = updateImageUrl(original, { foo: 'bar' }, mediaUrlPrefix);
       const url = URL(updated);
       expect(url.pathname).to.startsWith('/~/jssmedia/');
     });

packages/sitecore-jss/src/media-api.ts

@@ -49,10 +49,11 @@ export const getRequiredParams = (qs: { [key: string]: string | undefined }) =>
 };
 
 /**
- * Receives a Sitecore media URL and replaces `/~/media` or `/-/media` with `/~/jssmedia` or `/-/jssmedia`, respectively.
- * Can use `mediaUrlPrefix` in order to use custom checker.
- * This replacement allows the JSS media handler to be used for JSS app assets.
- * Also, any provided `params` are used as the querystring parameters for the media URL.
+ * Prepares a Sitecore media URL with `params` for use by the JSS media handler.
+ * This is done by replacing `/~/media` or `/-/media` with `/~/jssmedia` or `/-/jssmedia`, respectively.
+ * Provided `params` are used as the querystring parameters for the media URL.
+ * Can use `mediaUrlPrefix` in order to use a custom prefix.
+ * If no `params` are sent, the original media URL is returned.
  * @param {string} url
  * @param {Object} [params]
  * @param {RegExp} [mediaUrlPrefix=mediaUrlPrefixRegex]
@@ -63,6 +64,10 @@ export const updateImageUrl = (
   params?: { [key: string]: string | number | undefined } | null,
   mediaUrlPrefix: RegExp = mediaUrlPrefixRegex
 ) => {
+  if (!params || Object.keys(params).length === 0) {
+    // if params aren't supplied, no need to run it through JSS media handler
+    return url;
+  }
   // polyfill node `global` in browser to workaround https://github.com/unshiftio/url-parse/issues/150
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   if (typeof window !== 'undefined' && !(window as any).global) {
@@ -71,18 +76,14 @@ export const updateImageUrl = (
   }
   const parsed = URL(url, {}, true);
 
-  const query = { ...(params || parsed.query) };
-
-  // In case if imageParams provided
-  if (params) {
-    const requiredParams = getRequiredParams(parsed.query);
+  const requiredParams = getRequiredParams(parsed.query);
 
-    Object.entries(requiredParams).forEach(([key, param]) => {
-      if (param) {
-        query[key] = param;
-      }
-    });
-  }
+  const query = { ...params };
+  Object.entries(requiredParams).forEach(([key, param]) => {
+    if (param) {
+      query[key] = param;
+    }
+  });
 
   parsed.set('query', query);