Support all RFC3986 § 2.2 reserved characters

[shaunco]

and § 2.1 percent-encoding uppercase and lowercase

Resolves: #73
Change-type: patch

[shaunco]

@Page- / @thgreasi - please review when you have a moment. Please note that all 11,541 tests pass for me locally (including the 25 new ones I just added), but flowzone doesn't seem to automatically run unit tests in this repo.

Apologies for the tags, but none of these repos allow for me to request reviewers on PRs.

[Page-]

#75 should let the CI run on this, and for requesting reviewers it's a github thing that requires write access so nothing I can do there I'm afraid

[Page-]

My worry on this is that I think it could have a large impact on parsing performance due to the number of branches it introduces that all need to be explored. I do wonder if using a native function to normalize the uri before running through the odata parsing part that only needs to handle a normalized form (as talked about in https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#sec_URLSyntax) would be more efficient/correct

[shaunco]

My worry on this is that I think it could have a large impact on parsing performance due to the number of branches it introduces that all need to be explored. I do wonder if using a native function to normalize the uri before running through the odata parsing part that only needs to handle a normalized form (as talked about in https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#sec_URLSyntax) would be more efficient/correct

That is where I had originally started in balena-io/pinejs#693 , but moved to here as I wasn't sure if there were other calls to parseOdata that passed raw query strings.

[fisehara]

@shaunco Can you please check if using https://nodejs.org/dist/latest-v18.x/docs/api/all.html#all_querystring_query-string on the URI either by calling unescape or decode can solve the unescaping. Right now the implementation looks like unescaping the characters

[shaunco]

@shaunco Can you please check if using https://nodejs.org/dist/latest-v18.x/docs/api/all.html#all_querystring_query-string on the URI either by calling unescape or decode can solve the unescaping. Right now the implementation looks like unescaping the characters

Unescaping first works (see balena-io/pinejs#693 ) ... I went this route because there was already some unescaping logic in here for single-quotes and spaces, and it seemed odd to escape some reserved characters but not all. If the code in pinejs that the linked issue calls out is indeed the only place where the string is not unescaped before calling this library, then I agree that is a better place to do the unescaping.

[fisehara]

@shaunco @Page- I'm proposing to run the decodeURI and let the parser use unescaped characters, as the ABNF / OData spec is written in. 4107312

[shaunco]

@shaunco @Page- I'm proposing to run the decodeURI and let the parser use unescaped characters, as the ABNF / OData spec is written in. 4107312

Thanks for getting this in! I'll close this PR. I'll let you close #73.

[fisehara]

@shaunco Can you still please PR the test cases from your site?