Include the raw query string for the custom parser

efbenson commented 3 years ago

Support plan

is this issue currently blocking your project? (yes/no): no
is this issue affecting a production system? (yes/no): no

Context

node version: 14
module version: 19
environment (e.g. node, browser, native): node
used with (e.g. hapi application, another framework, standalone, ...): hapi
any other relevant information: using qs for query string parsing

What problem are you trying to solve?

I would like to send complex data in the query string. This includes nulls. The current preprocessing of the querystring will turn null objects properties into empty strings. More details and examples can be found here: https://github.com/ljharb/qs#handling-of-null-values

Currently the work around is to dig into the raw request in the handler and pull out the url and parse it myself. It would be nice if there was a second parameter on the custom parser to pass in the raw query string with no preprocessing done to it.

Do you have a new or modified API suggestion to solve the problem?

Test and working if you would like I can PR it. https://github.com/efbenson/hapi/commit/5903cd510bc2dd47d3289e2089f2838fb9a9df27

Nargonath commented 3 years ago

hapi already gives you the ability to override the query parser used by your server: https://hapi.dev/api/?v=20.0.2#-serveroptionsqueryparser. I think it solves your problem so I'll close this issue but feel free to reopen if I misunderstood you.

devinivy commented 3 years ago

I believe that this is a new feature augmenting the existing query parser functionality: https://github.com/efbenson/hapi/commit/5903cd510bc2dd47d3289e2089f2838fb9a9df27#diff-f6078640b332d9e3a509ffc552a2fb79cb93f7f0c1fb99bc6d23ccb195fd22a9R793-R801

Seems like a valid feature request, has also been mentioned in the past here: https://github.com/hapijs/hapi/issues/4058. For now you should be able to workaround this limitation of the parser option using request.setUrl() in an onRequest extension. The fact that you can work around it in this way is also probably the main argument against adding this feature. Either way, I think it's up for discussion!

Nargonath commented 3 years ago

Alright, my bad then. I misunderstood the request at hand.

Nargonath commented 3 years ago

After another fresh look, I think this feature would be a nice addition. I understand there is a workaround but I don't feel this feature is big enough that it would clutter the codebase. Plus the addition of the raw querystring parameter is generic enough to allow other use cases and not just specific to this feature request.

I'm in favor of it. What do you want to do @devinivy? Should we wait for other TSC members to weigh-in ? Are you in favor of this feature request as well?

hapijs / hapi