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

parseOrigin in undici throws "InvalidArgumentError" for URLs with paths #3999

Open
ahmed-oues opened this issue Jan 13, 2025 · 2 comments
Open
Labels
bug Something isn't working

Comments

@ahmed-oues
Copy link

Bug Description

The parseOrigin function in the undici library throws an InvalidArgumentError when a URL contains a path, query, or fragment. This behavior makes it difficult to use the library with valid URLs like https://api.domain.com/elastic/. that's because of an if condition in the function.

This strict validation seems unnecessary for some use cases, and there should be a way to relax it.

Reproducible By

Modify parseOrigin to allow URLs with paths.
Alternatively, provide an option to bypass this validation.

navigate to \node_modules\undici\lib\core\util.js
`function parseOrigin (url) {
url = parseURL(url)

if (url.pathname !== '/' || url.search || url.hash) {
throw new InvalidArgumentError('invalid url')
}

return url
}will be function parseOrigin (url) {
url = parseURL(url)

if (url.search || url.hash) {
throw new InvalidArgumentError('invalid url')
}

return url
}`
or sothisng selse semeler to that just without the exception of a "/"

Expected Behavior

Logs & Screenshots

Node.js version: v18.20.4
Undici version: X.X.X

@ahmed-oues ahmed-oues added the bug Something isn't working label Jan 13, 2025
@mcollina
Copy link
Member

Thanks for reporting!

Can you provide steps to reproduce? We often need a reproducible example, e.g. some code that allows someone else to recreate your problem by just copying and pasting it. If it involves more than a couple of different file, create a new repository on GitHub and add a link to that.

@ahmed-oues
Copy link
Author

Thank you for your response!

Steps to Reproduce

The issue occurs when providing a connection URL to Elasticsearch that includes a path, such as https://api.domain.com/elastic/. Specifically, the problem arises due to the /elastic/ portion of the URL. The root cause is that the parseOrigin function in undici rejects URLs with paths (pathname) that are not exactly /. This results in the following error:

"path to projet"\node_modules\undici\lib\core\util.js:209 throw new InvalidArgumentError('invalid url') ^ InvalidArgumentError: invalid url at Object.parseOrigin ("path to projet"\node_modules\undici\lib\core\util.js:209:11) at new Pool (\node_modules\undici\lib\dispatcher\pool.js:70:23) at new Connection ("path to projet"\node_modules@elastic\transport\src\connection\UndiciConnection.ts:115:17) at WeightedConnectionPool.createConnection ("path to projet"\node_modules@elastic\transport\src\pool\BaseConnectionPool.ts:139:24) at WeightedConnectionPool.addConnection ("path to projet"\node_modules@elastic\transport\src\pool\BaseConnectionPool.ts:160:31) at new Client ("path to projet"\node_modules@elastic\elasticsearch\src\client.ts:268:27) at Module._compile (node:internal/modules/cjs/loader:1364:14) at Module.m._compile ("path to projet"\node_modules\ts-node\src\index.ts:1618:23) at Module._extensions..js (node:internal/modules/cjs/loader:1422:10) { code: 'UND_ERR_INVALID_ARG' }

Explanation

The error occurs because the parseOrigin function in undici strictly validates URLs and throws an InvalidArgumentError if the pathname is not /. This makes it impossible to use valid URLs with paths like /elastic/ for Elasticsearch or similar services.

Suggested Fix

I’ve created a pull request to address this issue: Relax parseOrigin validation to allow paths. The change modifies the parseOrigin function to remove the restriction on pathname while maintaining validation for search and hash. This ensures compatibility with URLs that include paths without introducing breaking changes.

Conclusion

I believe this fix will resolve the issue while keeping the library flexible for real-world use cases.

Let me know if further clarification is needed or if I can provide additional context. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants