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

[bug]: query parameters are escaped implicitly #2829

Open
1 task done
liudonghua123 opened this issue Oct 27, 2022 · 17 comments
Open
1 task done

[bug]: query parameters are escaped implicitly #2829

liudonghua123 opened this issue Oct 27, 2022 · 17 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@liudonghua123
Copy link

liudonghua123 commented Oct 27, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

When I send a Get request with the following parameters(in bulk edit).

token: token_part1 token_part2
attrs: password%3Dabc

The actual request sent is url?token=token_part1+token_part2&attrs=password%253Dabc which is wrong for my server.

If I send the same Get request using postman, the actual request sent is url?token=token_part1%20token_part2&attrs=password%3Dabc which is correct for my server.

I found the problem code is around the following code.

const url = new URL(reqClone.url ?? "")
for (const [key, value] of reqClone.params.entries()) {
url.searchParams.append(key, value)
}
reqClone.url = url.toString()

url=new URL("http://localhost")
url.searchParams.append('token','token_part1 token_part2')
url.searchParams.append('attrs','password%3Dabc')
console.info(url.toString()) // http://localhost/?token=token_part1+token_part2&attrs=password%253abc

url=new URL('http://localhost/?token=token_part1+token_part2&attrs=password%253abc')
console.info(url.searchParams.get('token')) // token_part1 token_part2
console.info(url.searchParams.get('attrs')) // password%3Dabc
console.info(decodeURI('http://localhost/?token=token_part1+token_part2&attrs=password%253abc')) // http://localhost/?token=token_part1+token_part2&attrs=password%3Dabc

See also https://developer.mozilla.org/en-US/docs/Web/API/URL/toString, https://javascript.info/url, https://url.spec.whatwg.org/#URL-stringification-behavior.

Steps to reproduce

  1. Go to Parameters tab, click Bulk edit.
  2. Fill with the following contents
token: token_part1 token_part2
attrs: password%3Dabc
  1. Click Send button to make the request.

Environment

Release

Version

Cloud

@liudonghua123 liudonghua123 added bug Something isn't working need testing Needs to be tested before merging onto production labels Oct 27, 2022
@liudonghua123 liudonghua123 changed the title [bug]: query parameters are escaped [bug]: for query parameters, some of them are escaped, some of them are not Oct 27, 2022
@liudonghua123
Copy link
Author

If I send the request with the following parameters.

token: token_part1 token_part2
attrs: password%3Dabc

Then the request is ok. It seems the space in url query string can be encode/escape as + or %20. Both of them work.

So maybe the request works in postman is because it checks whether some spaces are in the parameters or not, encodeURIComponent is called if some spaces are included. It will NOT encode/escape the parameters.

A config setting toggles the encode/escape functionality would be nice for test purposes.

@liudonghua123 liudonghua123 changed the title [bug]: for query parameters, some of them are escaped, some of them are not [bug]: for query parameters are escaped Oct 27, 2022
@liudonghua123
Copy link
Author

The space character is the only character that has two acceptable URL-encoded representations. Instead of encoding a space as “%20,” you can use the plus sign reserved character to represent a space.

https://www.fullhost.com/blog/what-does-20-mean-in-a-web-address/

url1=new URL('http://localhost/?token=token_part1+token_part2&attrs=password%253abc')
url2=new URL('http://localhost/?token=token_part1%20token_part2&attrs=password%253abc')
console.info(url1.searchParams.get('token')) // token_part1 token_part2
console.info(url2.searchParams.get('token')) // token_part1 token_part2
console.info(url1.searchParams.get('token') === url2.searchParams.get('token')) // true

@liudonghua123 liudonghua123 changed the title [bug]: for query parameters are escaped [bug]: query parameters are escaped default explicitly Oct 27, 2022
@liudonghua123 liudonghua123 changed the title [bug]: query parameters are escaped default explicitly [bug]: query parameters are escaped implicitly Oct 27, 2022
@AndrewBastin
Copy link
Member

Since the current implementation is correct according to the spec, we are prioritizing this as a low priority issue.

PR contributions for implementing the toggle for considering both ways are welcome in the meantime :)

@AndrewBastin AndrewBastin added good first issue Good for newcomers and removed need testing Needs to be tested before merging onto production labels Oct 27, 2022
@Rathan-Naik
Copy link

Rathan-Naik commented Jan 1, 2023

Just getting started on contributions @AndrewBastin I can work on implementing the toggle.

@AndrewBastin
Copy link
Member

Cool @Rathan-Naik, please do let me know if you need any help/guidance.

Assigning the task to you.

@Ahishekoza
Copy link

Hello @AndrewBastin,
I would like to contribute to this issue

@JManan
Copy link

JManan commented Jun 24, 2023

hey @AndrewBastin i would like to contribute to this issue. Please let me do it.

@AndrewBastin
Copy link
Member

@JManan sure.

@Pheonix075
Copy link

hey @AndrewBastin I would like to contribute to this issue.give me a chance

@VinayakSingh2001
Copy link

Hey @tejakummarikuntla I would like to work on this, can you please assign this to me !!

@tejakummarikuntla
Copy link

Sorry @VinayakSingh2001 , I don't think I can help you here. Did you mean to tag @AndrewBastin ?

@yashnirmal
Copy link

Hey @tejakummarikuntla is the issue still open?

@liudonghua123
Copy link
Author

No PR currently?

I suggest to add a configuration named Encode(Encode the query params, three options provided, enable means always encode the params, disable means do not encode the params and auto means encode the params which contain some common special characters need to encode).

And only two group of settings available now, one for Theme, another for Interceptor. Maybe a new group named Other is a good idea for this new Encode setting.

@meetdhanani17
Copy link

meetdhanani17 commented Oct 2, 2023

it is need to add a toggle like space as %20 or + or i change to direct convert space as %20?

@maneeshms
Copy link

@tejakummarikuntla If this issue is still open and no PR raised yet, i would like to give a try.

@liyasthomas
Copy link
Member

Assigning to @maneeshms.

@liyasthomas liyasthomas assigned maneeshms and unassigned Rathan-Naik Dec 28, 2023
@TheDynamicPunk
Copy link

TheDynamicPunk commented May 16, 2024

Hey @liyasthomas it's been a while since there has been some activity on this thread. If you've still not received any PR's for this, maybe I can take a look with some assistance from you?

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

No branches or pull requests