Skip to content
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

"TypeError: Cannot convert argument to a ByteString" when giving multibyte product name in header #1209

Closed
nobu-shopify opened this issue Aug 10, 2023 · 3 comments

Comments

@nobu-shopify
Copy link

nobu-shopify commented Aug 10, 2023

What is the location of your example repository?

https://github.com/nobu-shopify/hydrogen-demo-customerapi-2023-07-28

Which package or tool is having this issue?

Hydrogen

What version of that package or tool are you using?

"@shopify/hydrogen": "^2023.7.0", "@shopify/remix-oxygen": "^1.1.1",

What version of Remix are you using?

"@remix-run/react": "1.19.1",

Steps to Reproduce

  1. Open Hydrogen demo store - https://hydrogen-demo-customerapi-2023-07-28-ff4270a546f444d678ae.o2.myshopify.dev/
  2. Click "一期一会 (ICHIGO ICHIE)" <= the beer with yellow label

Expected Behavior

The product page should be displayed, like this 一期一会 (ICHIGO ICHIE)

Actual Behavior

  1. The following error is shown:

We’ve lost this product
We couldn’t find the product you’re looking for. Try checking the URL or heading back to the home page.

  1. URL shows corrupted characters in multibyte part:

https://hydrogen-demo-customerapi-2023-07-28-ff4270a546f444d678ae.o2.myshopify.dev/products/%C3%A4%C2%B8%C2%80%C3%A6%C2%9C%C2%9F%C3%A4%C2%B8%C2%80%C3%A4%C2%BC%C2%9A-ichigo-ichie?Title=Default+Title

  1. Doing the same repro steps in my local env, the following error is observed:

TypeError: Cannot convert argument to a ByteString because the character at index 10 has a value of 20908 which is greater than 255.
at Object.webidl.converters.ByteString (/Users/nobu/dev/shopify/hydrogen/hydrogen-demo-customerapi-2023-07-28/node_modules/undici/lib/fetch/webidl.js:426:13)
at Headers.set (/Users/nobu/dev/shopify/hydrogen/hydrogen-demo-customerapi-2023-07-28/node_modules/undici/lib/fetch/headers.js:348:31)
at redirect (/Users/nobu/dev/shopify/hydrogen/hydrogen-demo-customerapi-2023-07-28/node_modules/@remix-run/router/utils.ts:1479:11)
at redirect (/Users/nobu/dev/shopify/hydrogen/hydrogen-demo-customerapi-2023-07-28/node_modules/@remix-run/server-runtime/dist/esm/responses.js:41:10)
at redirectToFirstVariant (/Users/nobu/dev/shopify/hydrogen/hydrogen-demo-customerapi-2023-07-28/app/routes/($locale).products.$productHandle.jsx:113:9)
at loader4 (/Users/nobu/dev/shopify/hydrogen/hydrogen-demo-customerapi-2023-07-28/app/routes/($locale).products.$productHandle.jsx:68:12)
at processTicksAndRejections (node:internal/process/task_queues:95:5)
at callRouteLoaderRR (/Users/nobu/dev/shopify/hydrogen/hydrogen-demo-customerapi-2023-07-28/node_modules/@remix-run/server-runtime/dist/esm/data.js:48:16)
at callLoaderOrAction (/Users/nobu/dev/shopify/hydrogen/hydrogen-demo-customerapi-2023-07-28/node_modules/@remix-run/router/router.ts:3671:16)
at async Promise.all (index 0)
GET 500 loader /products/%E5%86%AC%E3%81%AE%E6%B0%97%E3%81%BE%E3%81%90%E3%82%8C-2022-fuyu-no-kimagure-2022 [($locale).products.$productHandle] (prefetch)

Looking at nodejs/undici#1590, it appears that we are giving Unicode header to node_modules/undici/lib/fetch/headers.js

@nobu-shopify nobu-shopify changed the title "TypeError: Cannot convert argument to a ByteString" for multibyte product title page "TypeError: Cannot convert argument to a ByteString" when giving multibyte product name in header Aug 10, 2023
@wizardlyhel
Copy link
Contributor

Thank you for finding this bug

@wizardlyhel
Copy link
Contributor

wizardlyhel commented Aug 10, 2023

@nobu-shopify I am assumming you are using the demo store project as a base.

In the function redirectToFirstVariant, update the redirect url path so that it is url encoded.

function redirectToFirstVariant({
  product,
  request,
}: {
  product: ProductQuery['product'];
  request: Request;
}) {
  const searchParams = new URLSearchParams(new URL(request.url).search);
  const firstVariant = product!.variants.nodes[0];
  for (const option of firstVariant.selectedOptions) {
    searchParams.set(option.name, option.value);
  }

-  throw redirect(
-    `/products/${product!.handle}?${searchParams.toString()}`,
-    302,
-  );

+  // Use URL to avoid accidental double encoding
+  const newUrl = new URL(`/products/${product!.handle}?${searchParams.toString()}`, 'http://example.com');
+  throw redirect(newUrl.pathname + newUrl.search, 302);
}

We'll update demo store code to reflect this change as well.

@nobu-shopify
Copy link
Author

Thanks @wizardlyhel !!

Verified here that your proposed fix works on my Hydrogen demostore JS code, after removing non-null assertion in $(product!.handle) (which appears TS-only feature):

  // Fix from https://github.com/Shopify/hydrogen/issues/1209
  // Use URL to avoid accidental double encoding
  const newUrl = new URL(`/products/${product.handle}?${searchParams.toString()}`, 'http://example.com');
  throw redirect(newUrl.pathname + newUrl.search, 302);

nobu-shopify added a commit to nobu-shopify/hydrogen-demostore-auth0-2023-08-14 that referenced this issue Aug 14, 2023
nobu-shopify added a commit to nobu-shopify/hydrogen-demo-customerapi-2023-07-28 that referenced this issue Aug 14, 2023
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

No branches or pull requests

2 participants