search

samscott89 / serde_qs

Serde support for querystring-style strings
Apache License 2.0
193 stars 68 forks source link

Maximum query string length #64

Open elpiel opened 2 years ago

elpiel commented 2 years ago

Working on a web server I've realized that hyper nor serde_qs have an option for a maximum query string length. It would be nice to add such an additional option to the Config of serde_qs rather than implementing custom validation for each place that uses serde_qs to deserialize query strings.

What is your opinion on this?

samscott89 commented 2 years ago

Hey @elpiel.

That sounds like a mostly reasonable idea. For example, we already support having a "depth" limit, which controls how deeply we'll attempt to construct maps.

Out of curiosity, what's the use case? I could maybe see that as a simple protection against denial of service attacks (I'm imagining someone submitting q[][][][][][][]....<many many more>[][][][][]=1 for example.

On the other hand, this feels like it would be growing the scope of serde_qs beyond what I'd reasonable want to support. I think it would be easier to write that as a middleware in whatever web framework you are using.

elpiel commented 2 years ago

Out of curiosity, what's the use case? I could maybe see that as a simple protection against denial of service attacks...

Yes, that's actually our use case. Since we don't use a framework (just hyper) I don't want to always check the string length before deserializing.

There's already a Config with a few options and I thought it's a good place to add the maximum string length too.

Ps: although there is no limit in the RFC, browsers and servers usually have a set limit according to: https://stackoverflow.com/a/812962/2509411