Fix empty headers being added on POST/PATCH/DELETE (#1458)

src/PostgREST/App.hs

@@ -168,17 +168,19 @@ app dbStructure proc cols conf apiRequest =
                 Left _ -> return . errorResponseFor $ GucHeadersError
                 Right ghdrs -> do
                   let
-                    (ctHeader, rBody) = if iPreferRepresentation apiRequest == Full then (toHeader contentType, toS body) else (mempty, mempty)
+                    (ctHeader, rBody) = if iPreferRepresentation apiRequest == Full
-                    headers = addHeadersIfNotIncluded [
+                                          then (Just $ toHeader contentType, toS body)
+                                          else (Nothing, mempty)
+                    headers = addHeadersIfNotIncluded (catMaybes [
                           if null fields
-                            then mempty
+                            then Nothing
-                            else locationH tName fields
+                            else Just $ locationH tName fields
                         , ctHeader
-                        , contentRangeH 1 0 $ if shouldCount then Just queryTotal else Nothing
+                        , Just $ contentRangeH 1 0 $ if shouldCount then Just queryTotal else Nothing
                         , if null pkCols && isNothing (iOnConflict apiRequest)
-                            then mempty
+                            then Nothing
-                            else maybe mempty (\x -> ("Preference-Applied", show x)) $ iPreferResolution apiRequest
+                            else (\x -> ("Preference-Applied", show x)) <$> iPreferResolution apiRequest
-                        ] (unwrapGucHeader <$> ghdrs)
+                        ]) (unwrapGucHeader <$> ghdrs)
                   if contentType == CTSingularJSON && queryTotal /= 1
                     then do
                       HT.condemn
@@ -204,8 +206,10 @@ app dbStructure proc cols conf apiRequest =
                            | iPreferRepresentation apiRequest == Full = status200
                            | otherwise                                = status204
                     contentRangeHeader = contentRangeH 0 (queryTotal - 1) $ if shouldCount then Just queryTotal else Nothing
-                    (ctHeader, rBody) = if iPreferRepresentation apiRequest == Full then (toHeader contentType, toS body) else (mempty, mempty)
+                    (ctHeader, rBody) = if iPreferRepresentation apiRequest == Full
-                    headers = addHeadersIfNotIncluded [contentRangeHeader, ctHeader] (unwrapGucHeader <$> ghdrs)
+                                          then (Just $ toHeader contentType, toS body)
+                                          else (Nothing, mempty)
+                    headers = addHeadersIfNotIncluded (catMaybes [Just contentRangeHeader, ctHeader]) (unwrapGucHeader <$> ghdrs)
                   if contentType == CTSingularJSON && queryTotal /= 1
                     then do
                       HT.condemn
@@ -263,8 +267,10 @@ app dbStructure proc cols conf apiRequest =
                   let
                     status = if iPreferRepresentation apiRequest == Full then status200 else status204
                     contentRangeHeader = contentRangeH 1 0 $ if shouldCount then Just queryTotal else Nothing
-                    (ctHeader, rBody) = if iPreferRepresentation apiRequest == Full then (toHeader contentType, toS body) else (mempty, mempty)
+                    (ctHeader, rBody) = if iPreferRepresentation apiRequest == Full
-                    headers = addHeadersIfNotIncluded [contentRangeHeader, ctHeader] (unwrapGucHeader <$> ghdrs)
+                                          then (Just $ toHeader contentType, toS body)
+                                          else (Nothing, mempty)
+                    headers = addHeadersIfNotIncluded (catMaybes [Just contentRangeHeader, ctHeader]) (unwrapGucHeader <$> ghdrs)
                   if contentType == CTSingularJSON
                      && queryTotal /= 1
                     then do

test/Feature/PgVersion96Spec.hs

@@ -2,6 +2,7 @@ module Feature.PgVersion96Spec where
 
 import Network.HTTP.Types
 import Network.Wai        (Application)
+import Network.Wai.Test   (SResponse (simpleHeaders))
 
 import Test.Hspec
 import Test.Hspec.Wai
@@ -132,6 +133,34 @@ spec =
           , matchHeaders = ["Location" <:> "/stuff?id=eq.1&overriden=true"]
           }
 
+    -- On https://github.com/PostgREST/postgrest/issues/1427#issuecomment-595907535
+    -- it was reported that blank headers ` : ` where added and that cause proxies to fail the requests.
+    -- These tests are to ensure no blank headers are added.
+    context "Blank headers bug" $ do
+      it "shouldn't add blank headers on POST" $ do
+        r <- request methodPost "/loc_test" [] [json|{"id": "1", "c": "c1"}|]
+        liftIO $ do
+          let respHeaders = simpleHeaders r
+          respHeaders `shouldSatisfy` noBlankHeader
+
+      it "shouldn't add blank headers on PATCH" $ do
+        r <- request methodPatch "/loc_test?id=eq.1" [] [json|{"c": "c2"}|]
+        liftIO $ do
+          let respHeaders = simpleHeaders r
+          respHeaders `shouldSatisfy` noBlankHeader
+
+      it "shouldn't add blank headers on GET" $ do
+        r <- request methodGet "/loc_test" [] ""
+        liftIO $ do
+          let respHeaders = simpleHeaders r
+          respHeaders `shouldSatisfy` noBlankHeader
+
+      it "shouldn't add blank headers on DELETE" $ do
+        r <- request methodDelete "/loc_test?id=eq.1" [] ""
+        liftIO $ do
+          let respHeaders = simpleHeaders r
+          respHeaders `shouldSatisfy` noBlankHeader
+
     context "Use of the phraseto_tsquery function" $ do
       it "finds matches" $
         get "/tsearch?text_search_vector=phfts.The%20Fat%20Cats" `shouldRespondWith`

test/SpecHelper.hs

@@ -178,6 +178,9 @@ matchHeader :: CI BS.ByteString -> BS.ByteString -> [Header] -> Bool
 matchHeader name valRegex headers =
   maybe False (=~ valRegex) $ lookup name headers
 
+noBlankHeader :: [Header] -> Bool
+noBlankHeader = notElem mempty
+
 authHeaderBasic :: BS.ByteString -> BS.ByteString -> Header
 authHeaderBasic u p =
   (hAuthorization, "Basic " <> (toS . B64.encode . toS $ u <> ":" <> p))

test/fixtures/privileges.sql

@@ -121,6 +121,7 @@ GRANT ALL ON TABLE
     , activities
     , unit_workdays
     , stuff
+    , loc_test
 TO postgrest_test_anonymous;
 
 GRANT INSERT ON TABLE insertonly TO postgrest_test_anonymous;

test/fixtures/schema.sql

@@ -1681,10 +1681,15 @@ begin
       perform set_config(
         'response.headers'
       , format('[{"Location": "/%s?id=eq.%s&overriden=true"}]', tg_table_name, new.id)
-      , false
+      , true
       );
     end if;
     return new;
 end
 $$ language plpgsql security definer;
 create trigger location_for_stuff instead of insert on test.stuff for each row execute procedure test.location_for_stuff();
+
+create table loc_test (
+  id int primary key
+, c text
+);