diff --git a/adapters/quick/src/main/scala/caliban/QuickRequestHandler.scala b/adapters/quick/src/main/scala/caliban/QuickRequestHandler.scala index ebfb1544c..e74f7f70b 100644 --- a/adapters/quick/src/main/scala/caliban/QuickRequestHandler.scala +++ b/adapters/quick/src/main/scala/caliban/QuickRequestHandler.scala @@ -168,10 +168,8 @@ final private class QuickRequestHandler[R]( private def executeRequest(method: Method, req: GraphQLRequest)(implicit trace: Trace - ): ZIO[R, Response, GraphQLResponse[Any]] = { - val calibanMethod = if (method == Method.GET) HttpRequestMethod.GET else HttpRequestMethod.POST - HttpRequestMethod.setWith(calibanMethod)(interpreter.executeRequest(req)) - } + ): ZIO[R, Response, GraphQLResponse[Any]] = + interpreter.executeRequest(HttpRequestMethod.updateRequest(method == Method.GET)(req)) private def responseHeaders(headers: Headers, cacheDirective: Option[String]): Headers = cacheDirective match { diff --git a/core/src/main/scala/caliban/GraphQL.scala b/core/src/main/scala/caliban/GraphQL.scala index cf393a70a..06db720e4 100644 --- a/core/src/main/scala/caliban/GraphQL.scala +++ b/core/src/main/scala/caliban/GraphQL.scala @@ -118,7 +118,7 @@ trait GraphQL[-R] { self => (for { doc <- wrap(parseZIO)(parsingWrappers, request.query.getOrElse("")) coercedVars <- coerceVariables(doc, request.variables.getOrElse(Map.empty)) - executionReq <- wrap(validation(request.operationName, coercedVars))(validationWrappers, doc) + executionReq <- wrap(validation(request, coercedVars))(validationWrappers, doc) result <- wrap(execution(schemaToExecute(doc), fieldWrappers))(executionWrappers, executionReq) } yield result).catchAll(Executor.fail) )(overallWrappers, request) @@ -138,20 +138,20 @@ trait GraphQL[-R] { self => } private def validation( - operationName: Option[String], + req: GraphQLRequest, coercedVars: Map[String, InputValue] )(doc: Document)(implicit trace: Trace): IO[ValidationError, ExecutionRequest] = Configurator.ref.getWith { config => Validator.prepare( doc, typeToValidate(doc), - operationName, + req.operationName, coercedVars, config.skipValidation, config.validations ) match { - case Right(value) => checkHttpMethod(config)(value) - case Left(error) => ZIO.fail(error) + case Right(value) => checkHttpMethod(config)(req, value) + case Left(error) => Exit.fail(error) } } @@ -183,16 +183,15 @@ trait GraphQL[-R] { self => private def schemaToExecute(doc: Document) = if (doc.isIntrospection) introspectionRootSchema else schema - private def checkHttpMethod(cfg: ExecutionConfiguration)(req: ExecutionRequest)(implicit - trace: Trace - ): IO[ValidationError, ExecutionRequest] = - if ((req.operationType eq OperationType.Mutation) && !cfg.allowMutationsOverGetRequests) - HttpRequestMethod.getWith { - case HttpRequestMethod.GET => ZIO.fail(HttpRequestMethod.MutationOverGetError) - case _ => Exit.succeed(req) - } + private def checkHttpMethod( + cfg: ExecutionConfiguration + )(gqlReq: GraphQLRequest, req: ExecutionRequest): IO[ValidationError, ExecutionRequest] = + if ( + req.operationType == OperationType.Mutation && + !cfg.allowMutationsOverGetRequests && + HttpRequestMethod.isGetRequest(gqlReq) + ) Exit.fail(HttpRequestMethod.MutationOverGetError) else Exit.succeed(req) - } } diff --git a/core/src/main/scala/caliban/GraphQLRequest.scala b/core/src/main/scala/caliban/GraphQLRequest.scala index 14dade1b3..e7b0c3e1f 100644 --- a/core/src/main/scala/caliban/GraphQLRequest.scala +++ b/core/src/main/scala/caliban/GraphQLRequest.scala @@ -16,7 +16,7 @@ case class GraphQLRequest( operationName: Option[String] = None, variables: Option[Map[String, InputValue]] = None, extensions: Option[Map[String, InputValue]] = None -) { +) { self => def withExtension(key: String, value: InputValue): GraphQLRequest = copy(extensions = Some(extensions.foldLeft(Map(key -> value))(_ ++ _))) diff --git a/core/src/main/scala/caliban/HttpRequestMethod.scala b/core/src/main/scala/caliban/HttpRequestMethod.scala index 72ad54f40..df31c572f 100644 --- a/core/src/main/scala/caliban/HttpRequestMethod.scala +++ b/core/src/main/scala/caliban/HttpRequestMethod.scala @@ -1,25 +1,36 @@ package caliban import caliban.CalibanError.ValidationError +import caliban.Value.StringValue import zio.stacktracer.TracingImplicits.disableAutoTrace import zio.{ FiberRef, Trace, Unsafe, ZIO } private[caliban] sealed trait HttpRequestMethod private[caliban] object HttpRequestMethod { - case object GET extends HttpRequestMethod + case object GET extends HttpRequestMethod { + val asStringValue: StringValue = StringValue("GET") + } + case object POST extends HttpRequestMethod + final val ExtensionKey = "requestHttpMethod" + val MutationOverGetError: ValidationError = ValidationError("Mutations are not allowed for GET requests", "") - private val fiberRef: FiberRef[HttpRequestMethod] = Unsafe.unsafe(implicit u => FiberRef.unsafe.make(POST)) + def updateRequest(isGetRequest: Boolean)(req: GraphQLRequest): GraphQLRequest = + if (isGetRequest) + req.withExtension(HttpRequestMethod.ExtensionKey, HttpRequestMethod.GET.asStringValue) + else req + + def isGetRequest(req: GraphQLRequest): Boolean = + req.extensions.flatMap(_.get(ExtensionKey)).contains(GET.asStringValue) + @deprecated("To be removed in the next major release") def getWith[R, E, A](zio: HttpRequestMethod => ZIO[R, E, A])(implicit trace: Trace): ZIO[R, E, A] = - fiberRef.getWith(zio) + zio(HttpRequestMethod.POST) + @deprecated("To be removed in the next major release") def setWith[R, E, B](method: HttpRequestMethod)(zio: ZIO[R, E, B])(implicit trace: Trace): ZIO[R, E, B] = - method match { - case POST => zio - case GET => fiberRef.locally(method)(zio) - } + zio } diff --git a/interop/tapir/src/main/scala/caliban/interop/tapir/HttpInterpreter.scala b/interop/tapir/src/main/scala/caliban/interop/tapir/HttpInterpreter.scala index ac8e6b7fc..58a9278ba 100644 --- a/interop/tapir/src/main/scala/caliban/interop/tapir/HttpInterpreter.scala +++ b/interop/tapir/src/main/scala/caliban/interop/tapir/HttpInterpreter.scala @@ -76,16 +76,10 @@ object HttpInterpreter { def executeRequest[BS]( graphQLRequest: GraphQLRequest, serverRequest: ServerRequest - )(implicit streamConstructor: StreamConstructor[BS]): ZIO[R, TapirResponse, CalibanResponse[BS]] = - setRequestMethod(serverRequest)(interpreter.executeRequest(graphQLRequest)) - .map(buildHttpResponse[E, BS](serverRequest)) - - private def setRequestMethod(req: ServerRequest)(exec: URIO[R, GraphQLResponse[E]]): URIO[R, GraphQLResponse[E]] = - req.method match { - case Method.POST => HttpRequestMethod.setWith(HttpRequestMethod.POST)(exec) - case Method.GET => HttpRequestMethod.setWith(HttpRequestMethod.GET)(exec) - case _ => exec - } + )(implicit streamConstructor: StreamConstructor[BS]): ZIO[R, TapirResponse, CalibanResponse[BS]] = { + val req = HttpRequestMethod.updateRequest(serverRequest.method == Method.GET)(graphQLRequest) + interpreter.executeRequest(req).map(buildHttpResponse[E, BS](serverRequest)) + } } private case class Prepended[R, E]( diff --git a/interop/tapir/src/test/scala/caliban/interop/tapir/TapirAdapterSpec.scala b/interop/tapir/src/test/scala/caliban/interop/tapir/TapirAdapterSpec.scala index f885ddec2..94b6cd00b 100644 --- a/interop/tapir/src/test/scala/caliban/interop/tapir/TapirAdapterSpec.scala +++ b/interop/tapir/src/test/scala/caliban/interop/tapir/TapirAdapterSpec.scala @@ -268,6 +268,12 @@ object TapirAdapterSpec { ZIO .serviceWithZIO[SttpBackend[Task, Capabilities]](run(GraphQLRequest(None)).send(_)) .map(r => assertTrue(r.code.code == 400)) + }, + test("returns 400 for mutations over GET methods") { + runHttpRequest( + method = Method.GET.method, + query = """mutation{ deleteCharacter(name: "Amos Burton") }""" + ).map(r => assertTrue(r.code.code == 400)) } ) ),