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

Refactor OrderedParams to fix query param inconsistencies #379

Open
tarunKoyalwar opened this issue Mar 29, 2024 · 0 comments
Open

Refactor OrderedParams to fix query param inconsistencies #379

tarunKoyalwar opened this issue Mar 29, 2024 · 0 comments

Comments

@tarunKoyalwar
Copy link
Member

Proposed Changes

  • context Query Param = (Equals) inconsistency + issue nuclei#4963
  • we are storing query params as Key/Value pairs in orderedmap and maintaining its order and finally assigning it to url.RawQuery to use it unchanged there by forcing unsafe query params etc , but this issue requires the use of seperators to be dynamic & expects to be used as it is without any change ( sometimes with = and sometimes without = )
  • A Switch for updating this is not expected behaviour because this cannot handle the case where there is combination of both cases

I propose we internally discard orderedMap implementation of params i.e OrderedParams ( even params ) if possible and instead of map / orderedMap to maintain state we use string in the same way go std lib does this

go std lib (url)

https://github.com/projectdiscovery/nuclei/blob/03d6f7c98b5eab060bcde4f64bf56189dec26791/pkg/protocols/http/raw/raw.go#L99

aka , the source of true is Query String which remains unchanged . and all helpers like Set() , Add() , Iterate() etc are implemented in following way

Parse -> Do operation (Set,Add,Iterate) -> Encode to String 

this fixes the need for having a seperate orderedparams vs params and the value provided in template never changes regardless because string remains source of truth instead of map or our custom datastructure

This may be slightly expensive because we will be parsing & encoding but negligible and memory usage wouldn't be issue because string concatentation is cheaper since strings are immutable

cc: @dogancanbakir @Mzack9999

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

1 participant