Skip to content

Refactor OrderedParams to fix query param inconsistencies #379

@tarunKoyalwar

Description

@tarunKoyalwar

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type: MaintenanceUpdating phrasing or wording to make things clearer or removing ambiguity.

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions