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

normalizeURL encodes url incorrectly #165

Closed
enkot opened this issue Aug 16, 2023 · 7 comments
Closed

normalizeURL encodes url incorrectly #165

enkot opened this issue Aug 16, 2023 · 7 comments

Comments

@enkot
Copy link

enkot commented Aug 16, 2023

Environment

Node v16.20.0
Ufo v1.3.0

Reproduction

https://stackblitz.com/edit/stackblitz-starters-t7ehhr?file=index.mjs

Describe the bug

normalizeURL encodes urls incorrectly, different to encodeURI

Additional context

No response

Logs

No response

@enkot
Copy link
Author

enkot commented Aug 22, 2023

@pi0 Could you please share your thoughts on this one?
Because currently Nuxt's useFetch/$fetch has weird random symbols excluded from URL encoding, namely | for all encoding, and #, &, 1 and ^ for query parameter value encoding. This understandably breaks URL parsing in some APIs when one of these symbols is used, causing undesired effects when getting search suggestions and search results.

@pi0
Copy link
Member

pi0 commented Feb 5, 2024

Hi dear @enkot sorry for late answer on this.

I think it is a tricky situation. ufo was based on vue-router encoding utils that tried to keep characters decoded as much as possible for readability and only encode when must to do and it is unlikely to be changed for ufo v1 at least.

(see #164 for a related thread)

For ufo v2, we might migrate to native URL as parser and serializer that adds more cencoding

In your cases new URL('https://example.com/?foo=^bar').toString() returns https://example.com/?foo=^bar and new URL('https://example.com/?foo=bars').toString()returnshttps://example.com/?foo=bar`s'` too (so no encoding).

I think it would be nice to have some more context that what exact tools might be broken and need additional encoding (URL is kinda of standard nowadays)

If you still belive there are reasons to encode more characters please ping to reopen this thread but normally I would suggest that you manually use encodeURLComponent if more encoding is required before passing params to utils such as Nuxt's $fetch

@pi0 pi0 closed this as not planned Won't fix, can't repro, duplicate, stale Feb 5, 2024
@enkot
Copy link
Author

enkot commented Feb 8, 2024

Hi @pi0, thank you for the detailed answer

But I think the problem still exists. F.e. I need to send this query value - iphone |
I expect it to be encoded as iphone%20%7C but withQuery (used by ofetch) produces different result:

// Correct
console.log(encodeURIComponent('iphone |')); // result "iphone%20%7C"
// Incorrect
console.log(withQuery('/products', { q: 'iphone |' })); // result "iphone+|"

that is why I get the Malformed URI response from server because I can't send correct query with ofetch:

HTTP request parsing failed with error: "Malformed URI: /v2/search/productSearch?q=boots+|"

Workaround:

$fetch(`/products?q=${encodeURIComponent('iphone |')}`))

Repro:
Nuxt - https://stackblitz.com/edit/github-pkhe4p?file=app.vue
Node - https://stackblitz.com/edit/stackblitz-starters-8j1phn?file=index.mjs

@pi0
Copy link
Member

pi0 commented Feb 8, 2024

How (what runtime/layer) do you get `Malformed URI error from?

URL constructor in chrome at least is fine with it:

new URL('/v2/search/productSearch?q=boots+|', 'http://test.com').toString()
// => 'http://test.com/v2/search/productSearch?q=boots+|'

[...new URLSearchParams('?q=boots+|').entries()]
// => ['q', 'bots |']

@enkot
Copy link
Author

enkot commented Feb 8, 2024

I get it from the Mulesoft - https://help.mulesoft.com/s/article/java-lang-IllegalArgumentException-Malformed-URI-while-migrating-to-Mule-4
Mulesoft still supports RFC 1738 where "{", "}", "|", "", "^", "~", "[", "]", and "`" are unsafe characters.

@pi0
Copy link
Member

pi0 commented Feb 8, 2024

I see thanks for the references. I guess anyway we will switch to more encoding in the next major version (#208) but in the meantime, you have to keep using manual encodeURIComponent for compatibility.

@enkot
Copy link
Author

enkot commented Feb 8, 2024

If someone has the same issue with ofetch, here is a workaround:

const $myFetch = $fetch.create({
  onRequest(ctx) {
    if (!ctx.options.query) return

    const url = new URL(ctx.request.toString())

    Object.entries(ctx.options.query).forEach(([key, value]) =>
      url.searchParams.append(key, value)
    )

    ctx.request = url.toString()
    ctx.options.query = undefined // cleaning up the query so that ofetch doesn't process it again
  }
})

$myFetch(`https://example.com/search`, {
  query: {
    q: 'iphone ^'
  }
})
// -> https://example.com/search?q=iphone+%5E

More universal solution which works also with local url strings:

import { parseURL, parseQuery, stringifyParsedURL } from 'ufo'

const $myFetch = $fetch.create({
  onRequest(ctx) {
    if (!ctx.options.query) return

    const url = parseURL(ctx.request.toString())
    const query = new URLSearchParams()
    
    Object.entries({
      ...parseQuery(url.search),
      ...ctx.options.query
    }).forEach(([key, value]) => {
      query.append(key, value)
    })
    
    url.search = query.toString()
    ctx.request = stringifyParsedURL(url)
    ctx.options.query = undefined // cleaning up the query so that ofetch doesn't process it again
  }
})

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