Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

addPostParam adds POST parameters backwards #1846

Open
jezen opened this issue Oct 6, 2024 · 0 comments
Open

addPostParam adds POST parameters backwards #1846

jezen opened this issue Oct 6, 2024 · 0 comments
Labels

Comments

@jezen
Copy link
Member

jezen commented Oct 6, 2024

Web browsers submit POST parameters from forms encoded with application/x-www-form-urlencoded in tree-order.

Here's a form containing two fields sharing the same name.

<form enctype="application/x-www-form-urlencoded" method="post">
  <input type="text" name="money" value="USD">
  <input type="text" name="money" value="100">
</form>

When this is submitted, the order of the values should be preserved, and matching on the order of those values should work (and indeed, in a web browser, it does).

moneyField :: YesodMoneyField app => Field (HandlerFor app) Money.SomeDense
moneyField = Field parse view UrlEncoded
  where
  parse [currency, amount] _fileVals =
    case readMay amount of
      Nothing -> pure $ Right Nothing
      Just n  -> pure $ Right $ Money.mkSomeDense currency (toRational n)
  parse _ _ = pure $ Right Nothing

  view theId name attrs _eVal isReq = do
    -- not relevant…

However, if you were to write a test for this…

{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE QuasiQuotes #-}

module MoneyFieldSpec where

import TestImport

spec :: Spec
spec = withApp $ do

  it "Fails with bad input" $ do
    get HomeR
    request $ do
      addToken
      setUrl HomeR
      setMethod "POST"
      addPostParam "money" "XXX"
      addPostParam "money" "XXX"
    statusIs 400

  it "Supports USD" $ do
    get HomeR
    request $ do
      addToken
      setUrl HomeR
      setMethod "POST"
      -- These two lines are in the wrong order, but that's because yesod-test
      -- adds POST parameters in the wrong order — instead of appending each
      -- parameter to the list of parameters, it conses them, which is not what
      -- web browers do.
      addPostParam "money" "100"
      addPostParam "money" "USD"
    location <- followRedirect
    liftIO $ location `shouldBe` Right "/"
    bodyContains "Amount is USD 100"

The test will fail because — as written in the comment — addPostParam conses params rather than appending them.

I expect fixing this is simply a case of appending each param instead of consing.

@jezen jezen added the Bug label Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant