diff --git a/CHANGELOG.md b/CHANGELOG.md index a98c00bef..af6a7e3ea 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index a35956814..72b1cbc0f 100644 --- a/src/PostgREST/Error.hs +++ b/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,8 +375,8 @@ 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 @@ -384,7 +384,21 @@ 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 diff --git a/test/io/test_io.py b/test/io/test_io.py index 79b386062..9b5c512b8 100644 --- a/test/io/test_io.py +++ b/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'", } diff --git a/test/spec/Feature/ConcurrentSpec.hs b/test/spec/Feature/ConcurrentSpec.hs index 0c77a060d..bced6160b 100644 --- a/test/spec/Feature/ConcurrentSpec.hs +++ b/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 = [] } diff --git a/test/spec/Feature/Query/DeleteSpec.hs b/test/spec/Feature/Query/DeleteSpec.hs index 5a3d18928..df76d7009 100644 --- a/test/spec/Feature/Query/DeleteSpec.hs +++ b/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 = [] } diff --git a/test/spec/Feature/Query/ErrorSpec.hs b/test/spec/Feature/Query/ErrorSpec.hs index da6f9cb82..73f191a5a 100644 --- a/test/spec/Feature/Query/ErrorSpec.hs +++ b/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" ] + } diff --git a/test/spec/Feature/Query/InsertSpec.hs b/test/spec/Feature/Query/InsertSpec.hs index 48342001c..0b1e2678a 100644 --- a/test/spec/Feature/Query/InsertSpec.hs +++ b/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 = [] } diff --git a/test/spec/Feature/Query/QuerySpec.hs b/test/spec/Feature/Query/QuerySpec.hs index ca51eaca3..41c809793 100644 --- a/test/spec/Feature/Query/QuerySpec.hs +++ b/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] } diff --git a/test/spec/Feature/Query/UpdateSpec.hs b/test/spec/Feature/Query/UpdateSpec.hs index ee18e3208..44e7c0c79 100644 --- a/test/spec/Feature/Query/UpdateSpec.hs +++ b/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