URL-decoding query and form parameters + decoding FormData

[pbrinkmeier]

Hiya, first time contributor.

This PR adds

• Tests for URL-decoding in queryParam and formParam
• A current implementation of the formData action from Add for decoding x-www-form-urlencoded data #216

Let me know if this looks good to you guys or if I need to change anything.

closes #321 , closes #170 .

[ocramz]

thank you @pbrinkmeier ! I'll take a look soon.

[ocramz]

Thank you! Looks good but just a minor comment in ScottySpec, I think we should reuse the implementation from hspec-wai (unless I'm missing something).

Edit: Also could you please add a line in the Changelog describing your changes?

[pbrinkmeier]

This is where I got the implementation from.

[pbrinkmeier]

I added the changelog entry

[pbrinkmeier]

Fixed the conflict in changelog.md

[ocramz]

I would like to understand how much does the new function overlap in functionality with formParams and related. Also, I'm not sure using body is a good idea (depends on whether it respects #147 or not)

[ocramz]

What's the relationship between this and formParam(s) ?

If the only difference is which form encoding they can handle, we might want to extend the current functions rather than introducing a new one.

[pbrinkmeier]

formData could be rewritten in terms of formParams, Form and fromForm.

Which shows that yes, it is basically just support for a new encoding.

I initially added this because it was mentioned in #321 but I don't personally think that this is important functionality...

[ocramz]

Does body preserve the request body across calls to next? see #308

[pbrinkmeier]

I'm not 100% sure what the desired behavior is, I assumed this was "right" because of the way jsonData is written :)

Should I add a test here?

[pbrinkmeier]

Internally, both formParams and formData use getBodyAction which I believe leads to consistent behavior unless I'm missing something.

[ocramz]

Correct! (beautiful diagram btw, how did you make it?)

Basically this "lazier" parsing of request bodies (underneath the implementation of files, formParams etc.) is what I introduced in #369 and I think it would make sense for form parameters too.

[pbrinkmeier]

The diagram is made using plant UML, basically a text-based UML tool. Getting the call graph from Haskell isn't automatic tho, I did that by hand. Below is the source code for it.

@startuml
skinparam dpi 300

hide members
hide circle

package "Web.Scotty.Action" {
    formParams -down-> "Web.Scotty.Internal.Types.formParamsAndFilesWith"
    formData -down-> body
    body -down-> "Web.Scotty.Internal.Types.envBody"
}

package "Web.Scotty.Internal.Types" {
    formParamsAndFilesWith -left-> envFormDataAction
    envFormDataAction -down-> "Web.Scotty.Body.getFormParamsAndFilesAction"
    envBody -down-> Web.Scotty.Body.getBodyAction
}

note top of Web.Scotty.Internal.Types.envFormDataAction
Initialized by mkEnv
end note

note top of Web.Scotty.Internal.Types.envBody
Initialized by mkEnv
end note

package "Web.Scotty.Body" {
    getFormParamsAndFilesAction -down-> getBodyAction
    getFormParamsAndFilesAction -right-> parseRequestBodyExBS
    parseRequestBodyExBS -down-> "Network.Wai.Parse.sinkRequestBodyEx"
}

package "Network.Wai.Parse" {
    sinkRequestBodyEx -down-> conduitRequestBodyEx
    conduitRequestBodyEx -down-> "Network.HTTP.Types.parseSimpleQuery"
}

package Network.HTTP.Types {
    parseSimpleQuery -right-> parseQuery
    parseQuery -right-> urlDecode
}
@enduml

[pbrinkmeier]

I am still not entirely sure how to go forward with formData. Should I just remove it or rewrite it in terms of formParams?

[ocramz]

I think rewriting it in terms of formParams would be a great way forward!

[ocramz]

👍

Just a changelog conflict to resolve and a comment to clarify why an implementation looks the way it does.

[ocramz]

Thank you @pbrinkmeier , all good! 👍