From 30e772d3fa1fac0326c062e773d80850de4bcbdb Mon Sep 17 00:00:00 2001 From: Antoine Leblanc Date: Wed, 1 Feb 2023 21:31:23 +0000 Subject: [PATCH] add content-length header. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Adds a content-length response header to all endpoints. This PR tests this feature by checking the content-length of every request we send in the tests. ## Changelog ✍️ __Component__ : server __Type__: enhancement __Product__: community-edition ### Short Changelog add a content-length response header to all endpoints PR-URL: https://github.com/hasura/graphql-engine-mono/pull/7444 Co-authored-by: Manas Agarwal <5352361+manasag@users.noreply.github.com> GitOrigin-RevId: a0a811852053c5dde4b11b71ba11a7d456c84d76 --- .../hasura/catalogstate/catalogstate_test.go | 2 +- cli/internal/hasura/v2query/v2_query_test.go | 2 +- dc-agents/sqlite/test/db.sqlite | Bin 12288 -> 12288 bytes server/lib/api-tests/api-tests.cabal | 1 + .../src/Test/API/GraphQL/ContentLengthSpec.hs | 98 ++++++++++++++++++ .../test-harness/src/Harness/GraphqlEngine.hs | 1 - server/src-lib/Hasura/Server/App.hs | 11 +- 7 files changed, 109 insertions(+), 6 deletions(-) create mode 100644 server/lib/api-tests/src/Test/API/GraphQL/ContentLengthSpec.hs diff --git a/cli/internal/hasura/catalogstate/catalogstate_test.go b/cli/internal/hasura/catalogstate/catalogstate_test.go index 9f4e06bea14..6dbb973a6dd 100644 --- a/cli/internal/hasura/catalogstate/catalogstate_test.go +++ b/cli/internal/hasura/catalogstate/catalogstate_test.go @@ -72,7 +72,7 @@ func TestClientCatalogState_Set(t *testing.T) { require.IsType(t, &errors.Error{}, err) require.Equal(t, errors.Op("catalogstate.ClientCatalogState.Set"), err.(*errors.Error).Op) require.Equal(t, errors.KindHasuraAPI.String(), errors.GetKind(err).String()) - require.Equal(t, err.(*errors.Error).Err.Error(), `{ + require.JSONEq(t, err.(*errors.Error).Err.Error(), `{ "code": "parse-failed", "error": "When parsing Hasura.RQL.Types.Metadata.Common.CatalogStateType expected a String with the tag of a constructor but got some_state.", "path": "$.args.type" diff --git a/cli/internal/hasura/v2query/v2_query_test.go b/cli/internal/hasura/v2query/v2_query_test.go index 26217793e79..5dcb522419b 100644 --- a/cli/internal/hasura/v2query/v2_query_test.go +++ b/cli/internal/hasura/v2query/v2_query_test.go @@ -170,7 +170,7 @@ func TestClient_Bulk(t *testing.T) { require.IsType(tt, &errors.Error{}, err) require.Equal(tt, errors.KindHasuraAPI.String(), errors.GetKind(err).String()) require.Equal(tt, errors.Op("v2query.Client.Bulk"), err.(*errors.Error).Op) - require.Equal(tt, err.(*errors.Error).Err.Error(), `{ + require.JSONEq(tt, err.(*errors.Error).Err.Error(), `{ "code": "not-exists", "error": "source with name \"default\" does not exist", "path": "$.args[0].args" diff --git a/dc-agents/sqlite/test/db.sqlite b/dc-agents/sqlite/test/db.sqlite index 18035df8546c80dcb4e7b05922f9a199db077923..d42a85082db36b3756cee91d803b57021e505389 100644 GIT binary patch delta 154 zcmZojXh@hKE%=&&fq@x{nSk`Ni8{tWLA_`bULcP_fPsO(mGACmL4g9k$q)FJ8Un?t zrKK4gJxdaka#B+(5_1dS6q9q1t7C|(LWrZ2kE?CPY5|eULlQR;F5|c|(i{K0<=O9lk IqD2Y<04Vb@2><{9 diff --git a/server/lib/api-tests/api-tests.cabal b/server/lib/api-tests/api-tests.cabal index f9d52382565..7371828c609 100644 --- a/server/lib/api-tests/api-tests.cabal +++ b/server/lib/api-tests/api-tests.cabal @@ -91,6 +91,7 @@ library SpecHook Test.API.ConcurrentBulkSpec Test.API.ExplainSpec + Test.API.GraphQL.ContentLengthSpec Test.API.Metadata.ComputedFieldsSpec Test.API.Metadata.InconsistentSpec Test.API.Metadata.NativeQuerySpec diff --git a/server/lib/api-tests/src/Test/API/GraphQL/ContentLengthSpec.hs b/server/lib/api-tests/src/Test/API/GraphQL/ContentLengthSpec.hs new file mode 100644 index 00000000000..67a7cc7d325 --- /dev/null +++ b/server/lib/api-tests/src/Test/API/GraphQL/ContentLengthSpec.hs @@ -0,0 +1,98 @@ +{-# LANGUAGE QuasiQuotes #-} + +-- | Tests that the /graphql API correctly sets the Content-Length header. +-- +-- Importantly, we DO NOT check that the length is correct! That's because the +-- library we use for http calls actually *does* check that the Content-Length +-- header is correct *if it's present*, meaning that all other tests already +-- ensure its correctness. However, that library doesn't enforce that the header +-- is present in the first place, which is what that this test is for. +module Test.API.GraphQL.ContentLengthSpec (spec) where + +import Data.Aeson (Value, object, (.=)) +import Data.List.NonEmpty qualified as NE +import Harness.Backend.Postgres qualified as Postgres +import Harness.Http qualified as Http +import Harness.Quoter.Graphql (graphql) +import Harness.Test.Fixture qualified as Fixture +import Harness.Test.Schema (Table (..), table) +import Harness.Test.Schema qualified as Schema +import Harness.TestEnvironment (GlobalTestEnvironment, TestEnvironment (..), getServer, serverUrl) +import Hasura.Prelude +import Network.HTTP.Simple qualified as Http +import Test.Hspec (SpecWith, describe, it, shouldSatisfy) + +-------------------------------------------------------------------------------- +-- Preamble + +spec :: SpecWith GlobalTestEnvironment +spec = do + Fixture.run + ( NE.fromList + [ (Fixture.fixture $ Fixture.Backend Postgres.backendTypeMetadata) + { Fixture.setupTeardown = \(testEnv, _) -> + [ Postgres.setupTablesAction schema testEnv + ] + } + ] + ) + tests + +-------------------------------------------------------------------------------- +-- Schema + +schema :: [Schema.Table] +schema = + [ (table "author") + { tableColumns = + [ Schema.column "id" Schema.TInt, + Schema.column "name" Schema.TStr + ], + tablePrimaryKey = ["id"], + tableData = [] + } + ] + +-------------------------------------------------------------------------------- +-- Tests + +tests :: Fixture.Options -> SpecWith TestEnvironment +tests _opts = do + describe "GraphQL query contains a Content-Length response header" do + it "checks for the header in a valid GraphQL query" \testEnvironment -> do + let schemaName :: Schema.SchemaName + schemaName = Schema.getSchemaName testEnvironment + queryGQL :: Value + queryGQL = + [graphql| + query { + #{schemaName}_author { + id + name + } + } + |] + url :: String + url = serverUrl (getServer testEnvironment) ++ "/v1/graphql" + response <- Http.post url mempty $ object ["query" .= queryGQL] + let contentLengthHeaders = Http.getResponseHeader "Content-Length" response + contentLengthHeaders `shouldSatisfy` ((== 1) . length) + + describe "GraphQL query contains a Content-Length response header" do + it "checks for the header in an invalid GraphQL query" \testEnvironment -> do + let schemaName :: Schema.SchemaName + schemaName = Schema.getSchemaName testEnvironment + queryGQL :: Value + queryGQL = + [graphql| + query { + #{schemaName}_artist { + favouriteVideoGame + } + } + |] + url :: String + url = serverUrl (getServer testEnvironment) ++ "/v1/graphql" + response <- Http.post url mempty $ object ["query" .= queryGQL] + let contentLengthHeaders = Http.getResponseHeader "Content-Length" response + contentLengthHeaders `shouldSatisfy` ((== 1) . length) diff --git a/server/lib/test-harness/src/Harness/GraphqlEngine.hs b/server/lib/test-harness/src/Harness/GraphqlEngine.hs index e493f13348c..029c17c3112 100644 --- a/server/lib/test-harness/src/Harness/GraphqlEngine.hs +++ b/server/lib/test-harness/src/Harness/GraphqlEngine.hs @@ -46,7 +46,6 @@ where import Control.Concurrent.Async qualified as Async import Control.Monad.Trans.Managed (ManagedT (..), lowerManagedT) --- import Hasura.RQL.Types.Metadata (emptyMetadataDefaults) import Data.Aeson (Value, fromJSON, object, (.=)) import Data.Aeson.Encode.Pretty as AP import Data.Aeson.Types (Pair) diff --git a/server/src-lib/Hasura/Server/App.hs b/server/src-lib/Hasura/Server/App.hs index 57a9b0d9f17..4cfe8641761 100644 --- a/server/src-lib/Hasura/Server/App.hs +++ b/server/src-lib/Hasura/Server/App.hs @@ -33,6 +33,7 @@ import Data.Aeson qualified as J import Data.Aeson.Key qualified as K import Data.Aeson.KeyMap qualified as KM import Data.Aeson.Types qualified as J +import Data.ByteString.Builder qualified as BB import Data.ByteString.Char8 qualified as B8 import Data.ByteString.Lazy qualified as BL import Data.CaseInsensitive qualified as CI @@ -233,7 +234,7 @@ onlyAdmin = do unless (uRole == adminRoleName) $ throw400 AccessDenied "You have to be an admin to access this endpoint" -setHeader :: MonadIO m => HTTP.Header -> Spock.ActionT m () +setHeader :: MonadIO m => HTTP.Header -> Spock.ActionCtxT ctx m () setHeader (headerName, headerValue) = Spock.setHeader (bsToTxt $ CI.original headerName) (bsToTxt headerValue) @@ -397,9 +398,12 @@ mkSpockAction serverCtx@ServerCtx {..} qErrEncoder qErrModifier apiHandler = do Spock.ActionCtxT ctx m3 a3 logErrorAndResp userInfo reqId waiReq req includeInternal headers extraUserInfo qErr = do let httpLogMetadata = buildHttpLogMetadata @m3 emptyHttpLogGraphQLInfo extraUserInfo + jsonResponse = J.encode $ qErrEncoder includeInternal qErr + contentLength = ("Content-Length", B8.toStrict $ BB.toLazyByteString $ BB.int64Dec $ BL.length jsonResponse) lift $ logHttpError (_lsLogger scLoggers) scLoggingSettings userInfo reqId waiReq req qErr headers httpLogMetadata + setHeader contentLength Spock.setStatus $ qeStatus qErr - Spock.json $ qErrEncoder includeInternal qErr + Spock.lazyBytes jsonResponse logSuccessAndResp userInfo reqId waiReq req result qTime reqHeaders authHdrs httpLoggingMetadata = do let (respBytes, respHeaders) = case result of @@ -408,7 +412,8 @@ mkSpockAction serverCtx@ServerCtx {..} qErrEncoder qErrModifier apiHandler = do (compressedResp, encodingType) = compressResponse (Wai.requestHeaders waiReq) respBytes encodingHeader = maybeToList (contentEncodingHeader <$> encodingType) reqIdHeader = (requestIdHeader, txtToBs $ unRequestId reqId) - allRespHeaders = pure reqIdHeader <> encodingHeader <> respHeaders <> authHdrs + contentLength = ("Content-Length", B8.toStrict $ BB.toLazyByteString $ BB.int64Dec $ BL.length compressedResp) + allRespHeaders = [reqIdHeader, contentLength] <> encodingHeader <> respHeaders <> authHdrs lift $ logHttpSuccess (_lsLogger scLoggers) scLoggingSettings userInfo reqId waiReq req respBytes compressedResp qTime encodingType reqHeaders httpLoggingMetadata mapM_ setHeader allRespHeaders Spock.lazyBytes compressedResp