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

Add shouldStripHeaderOnRedirect to Request #386

Merged
merged 1 commit into from
Feb 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions http-client/ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog for http-client

## 0.6.2

* Add `shouldStripHeaderOnRedirect` option to `Request` [#300](https://github.com/snoyberg/http-client/issues/300)

## 0.6.1.1

* Ensure that `Int` parsing doesn't overflow [#383](https://github.com/snoyberg/http-client/issues/383)
Expand Down
1 change: 1 addition & 0 deletions http-client/Network/HTTP/Client.hs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ module Network.HTTP.Client
, applyBasicProxyAuth
, decompress
, redirectCount
, shouldStripHeaderOnRedirect
, checkResponse
, responseTimeout
, cookieJar
Expand Down
1 change: 1 addition & 0 deletions http-client/Network/HTTP/Client/Request.hs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ defaultRequest = Request
Just (_ :: IOException) -> return ()
Nothing -> throwIO se
, requestManagerOverride = Nothing
, shouldStripHeaderOnRedirect = const False
}

-- | Parses a URL via 'parseRequest_'
Expand Down
6 changes: 5 additions & 1 deletion http-client/Network/HTTP/Client/Response.hs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ getRedirectedRequest req hs cookie_jar code
| 300 <= code && code < 400 = do
l' <- lookup "location" hs
let l = escapeURIString isAllowedInURI (S8.unpack l')
req' <- setUriRelative req =<< parseURIReference l
stripHeaders r =
r{requestHeaders =
filter (not . shouldStripHeaderOnRedirect req . fst) $
requestHeaders r}
req' <- fmap stripHeaders <$> setUriRelative req =<< parseURIReference l
return $
if code == 302 || code == 303
-- According to the spec, this should *only* be for status code
Expand Down
6 changes: 6 additions & 0 deletions http-client/Network/HTTP/Client/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,12 @@ data Request = Request
-- dealing with implicit global managers, such as in @Network.HTTP.Simple@
--
-- @since 0.4.28

, shouldStripHeaderOnRedirect :: HeaderName -> Bool
-- ^ Decide whether a header must be stripped from the request
-- when following a redirect. Default: keep all headers intact.
--
-- @since 0.6.2
}
deriving T.Typeable

Expand Down
2 changes: 1 addition & 1 deletion http-client/http-client.cabal
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: http-client
version: 0.6.1.1
version: 0.6.2
synopsis: An HTTP client engine
description: Hackage documentation generation is not reliable. For up to date documentation, please see: <http://www.stackage.org/package/http-client>.
homepage: https://github.com/snoyberg/http-client
Expand Down
37 changes: 28 additions & 9 deletions http-client/test-nonet/Network/HTTP/ClientSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import Network.HTTP.Client hiding (port)
import qualified Network.HTTP.Client as NC
import qualified Network.HTTP.Client.Internal as Internal
import Network.HTTP.Types (status413)
import Network.HTTP.Types.Header
import qualified Network.Socket as NS
import Test.Hspec
import qualified Data.Streaming.Network as N
Expand All @@ -38,21 +39,28 @@ silentIOError a = a `E.catch` \e -> do
let _ = e :: IOError
return ()

redirectServer :: (Int -> IO a) -> IO a
redirectServer inner = bracket
redirectServer :: Maybe Int
-- ^ If Just, stop redirecting after that many hops.
-> (Int -> IO a) -> IO a
redirectServer maxRedirects inner = bracket
(N.bindRandomPortTCP "*4")
(NS.close . snd)
$ \(port, lsocket) -> withAsync
(N.runTCPServer (N.serverSettingsTCPSocket lsocket) app)
(const $ inner port)
where
redirect ad = do
N.appWrite ad "HTTP/1.1 301 Redirect\r\nLocation: /\r\ncontent-length: 5\r\n\r\n"
threadDelay 10000
N.appWrite ad "hello\r\n"
threadDelay 10000
app ad = Async.race_
(silentIOError $ forever (N.appRead ad))
(silentIOError $ forever $ do
N.appWrite ad "HTTP/1.1 301 Redirect\r\nLocation: /\r\ncontent-length: 5\r\n\r\n"
threadDelay 10000
N.appWrite ad "hello\r\n"
threadDelay 10000)
(silentIOError $ case maxRedirects of
Nothing -> forever $ redirect ad
Just n ->
replicateM_ n (redirect ad) >>
N.appWrite ad "HTTP/1.1 200 OK\r\ncontent-length: 5\r\n\r\nhello\r\n")

redirectCloseServer :: (Int -> IO a) -> IO a
redirectCloseServer inner = bracket
Expand Down Expand Up @@ -158,7 +166,18 @@ spec = describe "Client" $ do
_ -> False
return ()
mapM_ test ["http://", "https://", "http://:8000", "https://:8001"]
it "redirecting #41" $ redirectServer $ \port -> do
it "headers can be stripped on redirect" $ redirectServer (Just 5) $ \port -> do
req' <- parseUrlThrow $ "http://127.0.0.1:" ++ show port
let req = req' { requestHeaders = [(hAuthorization, "abguvatgbfrrurer")]
, redirectCount = 10
, shouldStripHeaderOnRedirect = (== hAuthorization)
}
man <- newManager defaultManagerSettings
withResponseHistory req man $ \hr -> do
print $ map (requestHeaders . fst) $ hrRedirects hr
mapM_ (\r -> requestHeaders r `shouldBe` []) $
map fst $ tail $ hrRedirects hr
it "redirecting #41" $ redirectServer Nothing $ \port -> do
req' <- parseUrlThrow $ "http://127.0.0.1:" ++ show port
let req = req' { redirectCount = 1 }
man <- newManager defaultManagerSettings
Expand All @@ -167,7 +186,7 @@ spec = describe "Client" $ do
case e of
HttpExceptionRequest _ (TooManyRedirects _) -> True
_ -> False
it "redirectCount=0" $ redirectServer $ \port -> do
it "redirectCount=0" $ redirectServer Nothing $ \port -> do
req' <- parseUrlThrow $ "http://127.0.0.1:" ++ show port
let req = req' { redirectCount = 0 }
man <- newManager defaultManagerSettings
Expand Down
1 change: 1 addition & 0 deletions http-conduit/Network/HTTP/Conduit.hs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ module Network.HTTP.Conduit
, rawBody
, decompress
, redirectCount
, shouldStripHeaderOnRedirect
, checkResponse
, responseTimeout
, cookieJar
Expand Down