aesiniath / http-streams

Haskell HTTP client library for use with io-streams
https://hackage.haskell.org/package/http-streams
BSD 3-Clause "New" or "Revised" License
50 stars 48 forks source link

add better support for query params / url-encoding #84

Closed cartazio closed 9 years ago

cartazio commented 9 years ago

the library seems to not support query params, at least when provided in a not url-encoded fashion. which can lead to some surprises for the unwary user

cartazio commented 9 years ago

in my case, it was some naive get's that were failing

istathar commented 9 years ago

@cartazio Do you have an example URL that demonstrates the problem?

istathar commented 9 years ago

@cartazio as requested, see urlEncode in Network.Http.Inconvenience. https://github.com/afcowie/http-streams/blob/master/lib/Network/Http/Inconvenience.hs#L89 that's not exposed right now, but it could be, or more to the point, a utility function for RequestBuilder that takes advantage of it.

cartazio commented 9 years ago

ok, a simplified version of the URL looks like http://redacted.org:6006/information/v1/date/014787?date=2014-07-24&pretty

i'll have a go at using your suggestions this morning

cartazio commented 9 years ago

so its just the following right?


urlEncode :: ByteString -> URL
urlEncode = Builder.toByteString . urlEncodeBuilder
{-# INLINE urlEncode #-}

--
-- | URL-escapes a string (see
-- <http://tools.ietf.org/html/rfc2396.html#section-2.4>) into a 'Builder'.
--
urlEncodeBuilder :: ByteString -> Builder
urlEncodeBuilder = go mempty
  where
    go !b !s = maybe b' esc (S.uncons y)
      where
        (x,y)     = S.span (flip HashSet.member urlEncodeTable) s
        b'        = b `mappend` Builder.fromByteString x
        esc (c,r) = let b'' = if c == ' '
                                then b' `mappend` Builder.fromWord8 (c2w '+')
                                else b' `mappend` hexd c
                    in go b'' r

hexd :: Char -> Builder
hexd c0 = Builder.fromWord8 (c2w '%') `mappend` Builder.fromWord8 hi
                                      `mappend` Builder.fromWord8 low
  where
    !c        = c2w c0
    toDigit   = c2w . intToDigit
    !low      = toDigit $ fromEnum $ c .&. 0xf
    !hi       = toDigit $ (c .&. 0xf0) `shiftr` 4

    shiftr (W8# a#) (I# b#) = I# (word2Int# (uncheckedShiftRL# a# b#))

urlEncodeTable :: HashSet Char
urlEncodeTable = HashSet.fromList $! filter f $! map w2c [0..255]
  where
    f c | c >= 'A' && c <= 'Z' = True
        | c >= 'a' && c <= 'z' = True
        | c >= '0' && c <= '9' = True
    f c = c `elem` ("$-_.!~*'(),"::String)
sirlensalot commented 9 years ago

Is the intent for the convenience API to handle query params in the provided ByteString? I would think there would be a way to specify query pairs, which is what I've done. Network.URI already has the necessary escaping logic ... however this doesn't weave BS.unpack through T.decodeUtf8 ....


type Queries = [(ByteString,ByteString)]

prepPath :: Queries -> URI.URI -> ByteString
prepPath qs (URI.URI _ _ path qry frag) = BS.pack $ path ++ qry' qry qs ++ frag where
    qry' q [] = q
    qry' "" _ = "?" ++ qs' 
    qry' q _ = q ++ "&" ++ qs'
    qs' = intercalate "&" $ map enc qs
    enc (k,v) = esc k ++ "=" ++ esc v
    esc = URI.escapeURIString URI.isUnreserved . BS.unpack
cartazio commented 9 years ago

totally a bug on my side completely. closing. pardon the confusion