fix(error): leaking table and function names when calculating hint

Increase similarity score to 0.75 from 0.33 for table and functions
error hint.

Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>

Author: taimoorzaeem

CHANGELOG.md

@@ -14,6 +14,10 @@ All notable changes to this project will be documented in this file. From versio
   + Removed unnecessary double count when building the `Content-Range`.
 - Add config `client_error_verbosity` to customize error verbosity by @taimoorzaeem in #4088, #3980, #3824
 
+### Fixed
+
+- Fix leaking table and function names when calculating error hint by @taimoorzaeem in #4675
+
 ### Changed
 
 - Log error when `db-schemas` config contains schema `pg_catalog` or `information_schema` by @taimoorzaeem in #4359

src/PostgREST/Error.hs

@@ -338,7 +338,7 @@ noRelBetweenHint parent child schema allRels = ("Perhaps you meant '" <>) <$>
 -- Just "Perhaps you meant to call the function api.test"
 --
 -- >>> noRpcHint "api" "other" [] procs []
--- Just "Perhaps you meant to call the function api.another"
+-- Nothing
 --
 -- >>> noRpcHint "api" "noclosealternative" [] procs []
 -- Nothing
@@ -375,16 +375,30 @@ noRpcHint schema procName params allProcs overloadedProcs =
     -- E.g. ["val", "param", "name"] into "(name, param, val)"
     listToText       = ("(" <>) . (<> ")") . T.intercalate ", " . sort
     possibleProcs
-      | null overloadedProcs = Fuzzy.getOne fuzzySetOfProcs procName
-      | otherwise            = (procName <>) <$> Fuzzy.getOne fuzzySetOfParams (listToText params)
+      | null overloadedProcs = getFuzzyHint HintProcedure fuzzySetOfProcs procName
+      | otherwise            = (procName <>) <$> getFuzzyHint HintParams fuzzySetOfParams (listToText params)
 
 -- |
 -- Do a fuzzy search in all tables in the same schema and return closest result
 tableNotFoundHint :: Text -> Text -> SchemaCache -> Maybe Text
 tableNotFoundHint schema tblName SchemaCache{dbTablesFuzzyIndex}
   = fmap (\tbl -> "Perhaps you meant the table '" <> schema <> "." <> tbl <> "'") perhapsTable
     where
-      perhapsTable = (`Fuzzy.getOne` tblName) =<< HM.lookup schema dbTablesFuzzyIndex
+      perhapsTable = (\fuzzySet -> getFuzzyHint HintTable fuzzySet tblName) =<< HM.lookup schema dbTablesFuzzyIndex
+
+data HintType
+  = HintTable
+  | HintProcedure
+  | HintParams
+
+-- | Get hint using Fuzzy Search with at least 0.75 similarity score
+getFuzzyHint :: HintType -> Fuzzy.FuzzySet -> Text -> Maybe Text
+getFuzzyHint hintType =
+  let minScore = 0.75 :: Double -- used for table and procedure name hints
+  in case hintType of
+    HintTable     -> Fuzzy.getOneWithMinScore minScore
+    HintProcedure -> Fuzzy.getOneWithMinScore minScore
+    HintParams    -> Fuzzy.getOne -- For params, we stick to `getOne` which defaults to 0.33 min score, not a security risk to reveal params
 
 compressedRel :: Relationship -> JSON.Value
 -- An ambiguousness error cannot happen for computed relationships TODO refactor so this mempty is not needed

test/io/test_io.py

@@ -1768,11 +1768,11 @@ def test_client_error_verbosity_config(defaultenv):
     }
 
     with run(env=env) as postgrest:
-        response = postgrest.session.get("/itemsxx")
+        response = postgrest.session.get("/itemsx")
         assert response.status_code == 404
         assert response.json() == {
             "code": "PGRST205",
-            "message": "Could not find the table 'public.itemsxx' in the schema cache",
+            "message": "Could not find the table 'public.itemsx' in the schema cache",
         }
 
     env = {
@@ -1781,11 +1781,11 @@ def test_client_error_verbosity_config(defaultenv):
     }
 
     with run(env=env) as postgrest:
-        response = postgrest.session.get("/itemsxx")
+        response = postgrest.session.get("/itemsx")
         assert response.status_code == 404
         assert response.json() == {
             "code": "PGRST205",
-            "message": "Could not find the table 'public.itemsxx' in the schema cache",
+            "message": "Could not find the table 'public.itemsx' in the schema cache",
             "details": None,
             "hint": "Perhaps you meant the table 'public.items'",
         }

test/spec/Feature/ConcurrentSpec.hs

@@ -25,7 +25,7 @@ spec =
       raceTest 10 $
         get "/fakefake"
           `shouldRespondWith`
-          [json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.factories'","message":"Could not find the table 'test.fakefake' in the schema cache"} |]
+          [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find the table 'test.fakefake' in the schema cache"} |]
           { matchStatus  = 404
           , matchHeaders = []
           }

test/spec/Feature/Query/DeleteSpec.hs

@@ -116,7 +116,7 @@ spec =
       it "fails with 404" $
         request methodDelete "/foozle?id=eq.101" [] ""
           `shouldRespondWith`
-          [json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.foo'","message":"Could not find the table 'test.foozle' in the schema cache"} |]
+          [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find the table 'test.foozle' in the schema cache"} |]
           { matchStatus = 404
           , matchHeaders = []
           }

test/spec/Feature/Query/ErrorSpec.hs

@@ -42,10 +42,10 @@ pgErrorCodeMapping = do
       it "works with SchemaCache error" $
         get "/non_existent_table"
           `shouldRespondWith`
-          [json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.collision_test_table'","message":"Could not find the table 'test.non_existent_table' in the schema cache"} |]
+          [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find the table 'test.non_existent_table' in the schema cache"} |]
           { matchStatus  = 404
           , matchHeaders = [ "Proxy-Status" <:> "PostgREST; error=PGRST205"
-                           , "Content-Length" <:> "182" ]
+                           , "Content-Length" <:> "129" ]
           }
 
       it "works with Jwt error" $ do
@@ -75,3 +75,30 @@ pgErrorCodeMapping = do
           , matchHeaders = [ "Proxy-Status" <:> "PostgREST; error=123"
                            , "Content-Length" <:> "59" ]
           }
+
+    context "show hint on PGRST205 table not found error" $ do
+      it "show hint when similarity score is at least 75%" $ do
+        get "/projectx" -- at least 75% similar to "projects"
+          `shouldRespondWith`
+          [json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.projects'","message":"Could not find the table 'test.projectx' in the schema cache"} |]
+          { matchStatus  = 404
+          , matchHeaders = [ "Proxy-Status" <:> "PostgREST; error=PGRST205"
+                           , "Content-Length" <:> "160" ]
+          }
+
+        get "/projecxx" -- at least 75% similar to "projects"
+          `shouldRespondWith`
+          [json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.projects'","message":"Could not find the table 'test.projecxx' in the schema cache"} |]
+          { matchStatus  = 404
+          , matchHeaders = [ "Proxy-Status" <:> "PostgREST; error=PGRST205"
+                           , "Content-Length" <:> "160" ]
+          }
+
+      it "don't show hint when similarity score is less than 75%" $
+        get "/projxxxx" -- less than 75% similar to "projects"
+          `shouldRespondWith`
+          [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find the table 'test.projxxxx' in the schema cache"} |]
+          { matchStatus  = 404
+          , matchHeaders = [ "Proxy-Status" <:> "PostgREST; error=PGRST205"
+                           , "Content-Length" <:> "119" ]
+          }

test/spec/Feature/Query/InsertSpec.hs

@@ -483,7 +483,7 @@ spec actualPgVersion = do
             {"id": 204, "body": "yyy"},
             {"id": 205, "body": "zzz"}]|]
           `shouldRespondWith`
-          [json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.articles'","message":"Could not find the table 'test.garlic' in the schema cache"} |]
+          [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find the table 'test.garlic' in the schema cache"} |]
           { matchStatus  = 404
           , matchHeaders = []
           }

test/spec/Feature/Query/QuerySpec.hs

@@ -26,9 +26,9 @@ spec = do
     it "causes a 404" $
       get "/faketable"
       `shouldRespondWith`
-      [json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.private_table'","message":"Could not find the table 'test.faketable' in the schema cache"} |]
+      [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find the table 'test.faketable' in the schema cache"} |]
       { matchStatus = 404
-      , matchHeaders = ["Content-Length" <:> "166"]
+      , matchHeaders = ["Content-Length" <:> "120"]
       }
 
   describe "Filtering response" $ do
@@ -852,7 +852,7 @@ spec = do
       -- the existence of first table, #3869
       it "table not found error if first table does not exist" $
         get "/car_model_sales_202101?select=id,name,car_models(id,name)&order=id.asc" `shouldRespondWith`
-          [json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.car_model_sales'","message":"Could not find the table 'test.car_model_sales_202101' in the schema cache"} |]
+          [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find the table 'test.car_model_sales_202101' in the schema cache"} |]
           { matchStatus  = 404
           , matchHeaders = [matchContentTypeJson]
           }
@@ -870,7 +870,7 @@ spec = do
 
       it "table not found error if first table does not exist" $
         get "/car_models_default?select=id,name,car_model_sales(id,name)&order=id.asc" `shouldRespondWith`
-          [json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.car_model_sales'","message":"Could not find the table 'test.car_models_default' in the schema cache"} |]
+          [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find the table 'test.car_models_default' in the schema cache"} |]
           { matchStatus  = 404
           , matchHeaders = [matchContentTypeJson]
           }

test/spec/Feature/Query/UpdateSpec.hs

@@ -18,9 +18,9 @@ spec = do
         request methodPatch "/fake" []
           [json| { "real": false } |]
           `shouldRespondWith`
-          [json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.factories'","message":"Could not find the table 'test.fake' in the schema cache"} |]
+          [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find the table 'test.fake' in the schema cache"} |]
           { matchStatus = 404
-          , matchHeaders = ["Content-Length" <:> "157"]
+          , matchHeaders = ["Content-Length" <:> "115"]
           }
 
 
@@ -363,9 +363,9 @@ spec = do
             {"id": 204, "body": "yyy"},
             {"id": 205, "body": "zzz"}]|]
           `shouldRespondWith`
-          [json| {"code":"PGRST205","details":null,"hint":"Perhaps you meant the table 'test.articles'","message":"Could not find the table 'test.garlic' in the schema cache"} |]
+          [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find the table 'test.garlic' in the schema cache"} |]
           { matchStatus  = 404
-          , matchHeaders = ["Content-Length" <:> "158"]
+          , matchHeaders = ["Content-Length" <:> "117"]
           }
 
       context "apply defaults on missing values" $ do