dom96 / jester

A sinatra-like web framework for Nim.
MIT License
1.56k stars 120 forks source link

Decode all URL params no matter what #313

Closed ThomasTJdev closed 1 year ago

ThomasTJdev commented 1 year ago

This fixes issue https://github.com/dom96/jester/issues/312.

Base Request to roll back commit https://github.com/dom96/jester/commit/a03b5be0b72f1a686b6ca594c145cee665e6db7b which implemented PR https://github.com/dom96/jester/pull/171 which fixed Issue https://github.com/dom96/jester/issues/159.

Description Instead of a rollback of the previous commit, since that solves some problems, this commit fixes the bug which was introduces for non-URL params. We were already decoding non-URL parameters, the previous commit implemented a "double decode".

To fix this we now just decode all params on init. It adds a little overhead, but using benchy it doesn't look that bad.

ThomasTJdev commented 1 year ago

This PR decodes URL-params in the request.params, but then the user cannot access the initial raw URL parameters. This might be needed in some cases, e.g. like something below where you create params for a redirect.

  var params: string
  for k, v in request.params:
    if params != "":
      params.add("&")
    params.add(k & "=" & v)

The changelog needs an update with that.

dom96 commented 1 year ago

like something below where you create params for a redirect.

I suppose in this case the params can just be encodeUrl'd, right?

ThomasTJdev commented 1 year ago

Yes and no :)

If the user uses raw +-signs in the URL parameter, then it can become a problem. We use the decodeUrl with the decodePlus=true which substitutes the + with a space.

It is a "wrong" to include plus signs in that way, but.. sooo...

# post "/?calc=2 + 2"
# => calc=2%20+%202

let c = decodeUrl("2%20+%202")
assert c == "2   2"
c.encodeUrl() == "2+++2"
dom96 commented 1 year ago

I suppose you can access the original via request.url. So this isn't a big deal.

dom96 commented 1 year ago

Thank you for working through this :)

ThomasTJdev commented 1 year ago

I suppose you can access the original via request.url. So this isn't a big deal.

Touché - that's right! :)