Internal server error on collection COPY with Depth: infinity #9

New issue

Closed

opened 2024-11-20 10:37:16 -08:00 by thebartekbanach · 4 comments

thebartekbanach commented

2024-11-20 10:37:16 -08:00

(Migrated from github.com)

Hello!

First of all, I wanna say THANK YOU for this great library, it saved me a lot of work. If you configure Sponsor button I can give you a tip 😄

I encountered a problem with COPY method when trying to use it on collection with Depth: infinity header. I get 500 Internal server error, caused because incorrect destination URL being created.

The exact error message that I extracted with debugger is:

TypeError: Invalid URL
    at new URL (node:internal/url:797:36)
    at <anonymous> (/app/node_modules/nephele/src/Methods/COPY.ts:315:40)
    at async <anonymous> (/app/node_modules/nephele/src/catchErrors.ts:39:14)
    at async recursivelyCopy (/app/node_modules/nephele/src/Methods/COPY.ts:352:14)
    at async COPY.run (/app/node_modules/nephele/src/Methods/COPY.ts:355:21)
    at async <anonymous> (/app/node_modules/nephele/src/catchErrors.ts:39:14) {code: 'ERR_INVALID_URL', input: 'http://localhost/_/api/v1/resources/kopia-pierwsza/skopiowany/skopiowany/kolejny/', base: 'http:://localhost', stack: 'TypeError: Invalid URL
    at new URL (node:i…ode_modules/nephele/src/catchErrors.ts:39:14)', message: 'Invalid URL'}

With the most important thing noticible here:

code: 'ERR_INVALID_URL',
input: 'http://localhost/_/api/v1/resources/kopia-pierwsza/skopiowany/skopiowany/kolejny/', 
base: 'http:://localhost'

Look at the base - protocol includes double :: character, which causes an error.

I did not dive deep into this, but if we take a look at this piece of code, we can see the cause:

const destinationUrl = new URL(
    destination.toString().replace(/\/?$/, () => '/') +
        encodeURIComponent(name) +
        ((await child.isCollection()) ? '/' : ''),
    `${destination.protocol}://${destination.host}`, // <-- unnecesary colon
);

Destination variable is of URL type. According to this docs on MDN the protocol property returns protocol name with a colon behind it, for example http:.

So the result is, as you can see in the error message provided above, baseUrl with value "http:://localhost" provided into URL constructor.

I can create a PR with a fix, but first I need a green light from you. It seems like a small and easy fix, but it may break some functionalities that I do not even realize that exist.

The change proposed by me is to remove additional colon:

const destinationUrl = new URL(
    destination.toString().replace(/\/?$/, () => '/') +
        encodeURIComponent(name) +
        ((await child.isCollection()) ? '/' : ''),
    `${destination.protocol}//${destination.host}`, // NO COLON, COMPARE WITH CODE ABOVE
);

Thank you for taking time to look at this issue.

Have a good day!

Hello! First of all, I wanna say THANK YOU for this great library, it saved me a lot of work. If you configure *Sponsor* button I can give you a tip 😄 I encountered a problem with COPY method when trying to use it on collection with `Depth: infinity` header. I get `500 Internal server error`, caused because incorrect destination URL being created. The exact error message that I extracted with debugger is: ```text TypeError: Invalid URL at new URL (node:internal/url:797:36) at <anonymous> (/app/node_modules/nephele/src/Methods/COPY.ts:315:40) at async <anonymous> (/app/node_modules/nephele/src/catchErrors.ts:39:14) at async recursivelyCopy (/app/node_modules/nephele/src/Methods/COPY.ts:352:14) at async COPY.run (/app/node_modules/nephele/src/Methods/COPY.ts:355:21) at async <anonymous> (/app/node_modules/nephele/src/catchErrors.ts:39:14) {code: 'ERR_INVALID_URL', input: 'http://localhost/_/api/v1/resources/kopia-pierwsza/skopiowany/skopiowany/kolejny/', base: 'http:://localhost', stack: 'TypeError: Invalid URL at new URL (node:i…ode_modules/nephele/src/catchErrors.ts:39:14)', message: 'Invalid URL'} ``` With the most important thing noticible here: ``` code: 'ERR_INVALID_URL', input: 'http://localhost/_/api/v1/resources/kopia-pierwsza/skopiowany/skopiowany/kolejny/', base: 'http:://localhost' ``` Look at the `base` - protocol includes double **`::`** character, which causes an error. I did not dive deep into this, but if we take a look at [this piece of code](https://github.com/sciactive/nephele/blob/87b72cdcf220949124966f27d160df471ab18fd8/packages/nephele/src/Methods/COPY.ts#L319), we can see the cause: ```ts const destinationUrl = new URL( destination.toString().replace(/\/?$/, () => '/') + encodeURIComponent(name) + ((await child.isCollection()) ? '/' : ''), `${destination.protocol}://${destination.host}`, // <-- unnecesary colon ); ``` **Destination variable is of `URL` type**. According to [this docs on MDN](https://developer.mozilla.org/en-US/docs/Web/API/URL/protocol) the `protocol` property returns **protocol name with a colon behind it**, for example `http:`. So the result is, as you can see in the error message provided above, `baseUrl` with value `"http:://localhost"` provided into URL constructor. I can create a PR with a fix, but first I need a green light from you. It seems like a small and easy fix, but it may break some functionalities that I do not even realize that exist. The change proposed by me is to remove additional colon: ```ts const destinationUrl = new URL( destination.toString().replace(/\/?$/, () => '/') + encodeURIComponent(name) + ((await child.isCollection()) ? '/' : ''), `${destination.protocol}//${destination.host}`, // NO COLON, COMPARE WITH CODE ABOVE ); ``` Thank you for taking time to look at this issue. Have a good day!

thebartekbanach commented

2024-12-17 08:07:43 -08:00

(Migrated from github.com)

Following script fixes the issue by replacing problematic line with fixed one:

// this script should be placed inside some directory fe. `nephele_copy_fix` next to your `node_modules` directory

import fs from "fs/promises";

console.log("Loading source file contents");

const sourceFileContents = await fs.readFile(
    "../node_modules/nephele/dist/Methods/COPY.js",
    "utf-8",
);

console.log("Source file contents loaded, replacing the problematic code");

const fixedSourceFileContents = sourceFileContents.replace(
    "`${destination.protocol}://${destination.host}`", // colon exists
    "`${destination.protocol}//${destination.host}`", // colon removed
);

console.log("Overwriting the source file with the fixed code");

await fs.writeFile(
    "../node_modules/nephele/dist/Methods/COPY.js",
    fixedSourceFileContents,
);

console.log("Temporary fix completed");

We are using it for a month and everything works as expected :)

Following script fixes the issue by replacing problematic line with fixed one: ```js // this script should be placed inside some directory fe. `nephele_copy_fix` next to your `node_modules` directory import fs from "fs/promises"; console.log("Loading source file contents"); const sourceFileContents = await fs.readFile( "../node_modules/nephele/dist/Methods/COPY.js", "utf-8", ); console.log("Source file contents loaded, replacing the problematic code"); const fixedSourceFileContents = sourceFileContents.replace( "`${destination.protocol}://${destination.host}`", // colon exists "`${destination.protocol}//${destination.host}`", // colon removed ); console.log("Overwriting the source file with the fixed code"); await fs.writeFile( "../node_modules/nephele/dist/Methods/COPY.js", fixedSourceFileContents, ); console.log("Temporary fix completed"); ``` We are using it for a month and everything works as expected :)

hperrin commented

2024-12-19 11:03:08 -08:00

(Migrated from github.com)

Wow! Thank you for the detailed report and the fix. I'll go ahead and merge this and make a new release. And thank you for the compliments. I'll set up a sponsor button. :)

hperrin commented

2024-12-19 11:04:46 -08:00

(Migrated from github.com)

This project has been on the back burner for a month, since I've been updating all my other projects to the new version of Svelte, but I'll be focusing on it again soon.

hperrin commented

2024-12-23 17:53:08 -08:00

(Migrated from github.com)

This is now fixed in release 1.0.0-alpha.62.

❤️ 1