uNetworking / uWebSockets.js

μWebSockets for Node.js back-ends :metal:
Apache License 2.0
7.76k stars 563 forks source link

Strange behavior when one of query params empty #892

Closed tahonaPL closed 1 year ago

tahonaPL commented 1 year ago

Values of all query params are "undefined" when first value is empty.

version : 20.23.00

Given on request /../?test1=&test2=someValue

When
const test2 = req.getQuery("test2")

Then if (test2 != "someValue") throw new Error("Something off")

uNetworkingAB commented 1 year ago

https://github.com/uNetworking/uWebSockets/blob/master/src/QueryParser.h

Probably an edge case not implemented properly.

e3dio commented 1 year ago

Here is fast performing query parser in JS that works for all cases (for % encoded see below):

const parseQuery = s => {
   const m = new Map();
   if (!s.length) return m;
   let i = 0, i2, i3;
   do {
      i2 = s.indexOf('=', i);
      if (i2 == -1) i2 = s.length;
      i3 = s.indexOf('&', i2 + 1);
      if (i3 == -1) i3 = s.length;
      m.set(s.slice(i, i2), s.slice(i2 + 1, i3));
      i = i3 + 1;
   } while (i3 != s.length)
   return m;
};

const query = parseQuery(req.getQuery());
const val = query.get('param1');
uNetworkingAB commented 1 year ago

Don't you need to escape % hex also

e3dio commented 1 year ago

If you have % escaped in your query keys you need to update the parser function which will slow it down, or if you have % escaped in query values for specific keys you can check after parse with const decode = str => str.includes('%') ? decodeURIComponent(str) : str for better performance. So it's not fit-all solution which you would need for official library, but ideally you don't have % in your query keys, and you can specify which keys you want to check for % values after parsing

0x7d8 commented 1 year ago

Here is fast performing query parser in JS that works for all cases (for % encoded see below):

const parseQuery = s => {
   const m = new Map();
   if (!s.length) return m;
   let i = 0, i2, i3;
   do {
      i2 = s.indexOf('=', i);
      if (i2 == -1) i2 = s.length;
      i3 = s.indexOf('&', i2 + 1);
      if (i3 == -1) i3 = s.length;
      m.set(s.slice(i, i2), s.slice(i2 + 1, i3));
      i = i3 + 1;
   } while (i3 != s.length)
   return m;
};

const query = parseQuery(req.getQuery());
const val = query.get('param1');

is it intended that "q1&q2" for example ends up as "q1&q2" -> "" instead of "q1" -> "" & "q2" -> ""?

e3dio commented 1 year ago

To me that is correct, if you want { q1: '', q2: '' } the query string needs to be q1=&q2=. You need to send valid query strings

0x7d8 commented 1 year ago

Yeah, I just find it interesting because the built in querystring.parse also does "q1" -> "" & "q2" -> ""

uNetworkingAB commented 1 year ago

https://github.com/uNetworking/uWebSockets/commit/a658d1616abea0c107108d3bc60c36201bd5f807

Should be fixed in that commit, will let it run fuzzing before new release

uNetworkingAB commented 1 year ago

@tahonaPL Did you really get "undefined", didn't you get empty string? It should not be possible to get "undefined", ever. But we need to distinguish empty string from undefined, now

uNetworkingAB commented 1 year ago

https://github.com/uNetworking/uWebSockets/releases/tag/v20.40.0

uNetworkingAB commented 1 year ago

Fixed in latest release