Skip to content

Commit

Permalink
Use request extensions to identify GET requests
Browse files Browse the repository at this point in the history
  • Loading branch information
kyri-petrou committed Jul 6, 2024
1 parent f53133f commit df9145c
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
27 changes: 13 additions & 14 deletions core/src/main/scala/caliban/GraphQL.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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)

}
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/caliban/GraphQLRequest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))(_ ++ _)))
Expand Down
25 changes: 18 additions & 7 deletions core/src/main/scala/caliban/HttpRequestMethod.scala
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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](
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
)
),
Expand Down

0 comments on commit df9145c

Please sign in to comment.