Skip to content

Commit

Permalink
fix(2999): add validation on http query (#3143)
Browse files Browse the repository at this point in the history
  • Loading branch information
dekkku authored Nov 28, 2024
1 parent 2375475 commit 02c7821
Show file tree
Hide file tree
Showing 7 changed files with 217 additions and 2 deletions.
10 changes: 10 additions & 0 deletions src/core/blueprint/fixture/recursive-arg.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
schema @server(port: 8000) {
query: Query
}
type Query {
posts(id: PostData): Int @http(url: "upstream.com", query: [{key: "id", value: "{{.args.id.data}}"}])
}
type PostData {
author: String
data: PostData
}
1 change: 1 addition & 0 deletions src/core/blueprint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ mod operators;
mod schema;
mod server;
pub mod telemetry;
mod template_validation;
mod timeout;
mod union_resolver;
mod upstream;
Expand Down
12 changes: 11 additions & 1 deletion src/core/blueprint/operators/http.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use tailcall_valid::{Valid, Validator};
use template_validation::validate_argument;

use crate::core::blueprint::*;
use crate::core::config::group_by::GroupBy;
use crate::core::config::Field;
use crate::core::endpoint::Endpoint;
use crate::core::http::{HttpFilter, Method, RequestTemplate};
use crate::core::ir::model::{IO, IR};
Expand All @@ -10,8 +12,9 @@ use crate::core::{config, helpers, Mustache};
pub fn compile_http(
config_module: &config::ConfigModule,
http: &config::Http,
is_list: bool,
field: &Field,
) -> Valid<IR, BlueprintError> {
let is_list = field.type_of.is_list();
let dedupe = http.dedupe.unwrap_or_default();
let mustache_headers = match helpers::headers::to_mustache_headers(&http.headers).to_result() {
Ok(mustache_headers) => Valid::succeed(mustache_headers),
Expand All @@ -27,6 +30,13 @@ pub fn compile_http(
&& !http.batch_key.is_empty()
}),
)
.and(
Valid::from_iter(http.query.iter(), |query| {
validate_argument(config_module, Mustache::parse(query.value.as_str()), field)
})
.unit()
.trace("query"),
)
.and(Valid::succeed(http.url.as_str()))
.zip(mustache_headers)
.and_then(|(base_url, headers)| {
Expand Down
2 changes: 1 addition & 1 deletion src/core/blueprint/operators/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub fn compile_resolver(
config_module,
http,
// inner resolver should resolve only single instance of type, not a list
field.type_of.is_list(),
field,
)
.trace(config::Http::trace_name().as_str()),
Resolver::Grpc(grpc) => compile_grpc(super::CompileGrpc {
Expand Down
90 changes: 90 additions & 0 deletions src/core/blueprint/template_validation/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
use tailcall_valid::{Valid, Validator};

use super::BlueprintError;
use crate::core::config::{Config, Field};
use crate::core::mustache::Segment;
use crate::core::scalar::Scalar;
use crate::core::Mustache;

// given a path, it follows path till leaf node and provides callback at leaf
// node.
fn path_validator<'a>(
config: &Config,
mut path_iter: impl Iterator<Item = &'a String>,
type_of: &str,
leaf_validator: impl Fn(&str) -> bool,
) -> Valid<(), BlueprintError> {
match config.find_type(type_of) {
Some(type_def) => match path_iter.next() {
Some(field) => match type_def.fields.get(field) {
Some(field_type) => {
path_validator(config, path_iter, field_type.type_of.name(), leaf_validator)
}
None => Valid::fail(BlueprintError::FieldNotFound(field.to_string())),
},
None => Valid::fail(BlueprintError::ValueIsNotOfScalarType(type_of.to_string())),
},
None if leaf_validator(type_of) => Valid::succeed(()),
None => Valid::fail(BlueprintError::TypeNotFoundInConfig(type_of.to_string())),
}
}

/// Function to validate the arguments in the HTTP resolver.
pub fn validate_argument(
config: &Config,
template: Mustache,
field: &Field,
) -> Valid<(), BlueprintError> {
let scalar_validator =
|type_: &str| Scalar::is_predefined(type_) || config.find_enum(type_).is_some();

Valid::from_iter(template.segments(), |segment| match segment {
Segment::Expression(expr) if expr.first().map_or(false, |v| v.contains("args")) => {
match expr.get(1) {
Some(arg_name) if field.args.get(arg_name).is_some() => {
let arg_type_of = field.args.get(arg_name).as_ref().unwrap().type_of.name();
path_validator(config, expr.iter().skip(2), arg_type_of, scalar_validator)
.trace(arg_name)
}
Some(arg_name) => {
Valid::fail(BlueprintError::ArgumentNotFound(arg_name.to_string()))
.trace(arg_name)
}
None => Valid::fail(BlueprintError::TooFewPartsInTemplate),
}
}
_ => Valid::succeed(()),
})
.unit()
}

#[cfg(test)]
mod test {
use tailcall_valid::{Valid, Validator};

use super::validate_argument;
use crate::core::blueprint::BlueprintError;
use crate::core::Mustache;
use crate::include_config;

#[test]
fn test_recursive_case() {
let config = include_config!("../fixture/recursive-arg.graphql");
let config = config.unwrap();
let template = Mustache::parse("{{.args.id.data}}");
let field = config
.find_type("Query")
.and_then(|ty| ty.fields.get("posts"))
.unwrap();
let validation_result = validate_argument(&config, template, field);

assert!(validation_result.is_fail());
assert_eq!(
validation_result,
Valid::fail(BlueprintError::ValueIsNotOfScalarType(
"PostData".to_string()
))
.trace("id")
);
}
}
60 changes: 60 additions & 0 deletions tests/core/snapshots/non-scalar-value-in-query.md_error.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
---
source: tests/core/spec.rs
expression: errors
---
[
{
"message": "too few parts in template",
"trace": [
"Query",
"invalidArgument",
"@http",
"query"
],
"description": null
},
{
"message": "value 'Nested' is not of a scalar type",
"trace": [
"Query",
"invalidArgumentType",
"@http",
"query",
"criteria"
],
"description": null
},
{
"message": "no argument 'criterias' found",
"trace": [
"Query",
"unknownArgument",
"@http",
"query",
"criterias"
],
"description": null
},
{
"message": "Cannot find type Criteria in the config",
"trace": [
"Query",
"unknownArgumentType",
"@http",
"query",
"criteria"
],
"description": null
},
{
"message": "field unknown_field not found",
"trace": [
"Query",
"unknownField",
"@http",
"query",
"criteria"
],
"description": null
}
]
44 changes: 44 additions & 0 deletions tests/execution/non-scalar-value-in-query.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
error: true
---

# test objects in args

```graphql @config
schema @server @upstream {
query: Query
}

type Query {
invalidArgumentType(criteria: Nested): [Employee!]!
@http(
url: "http://localhost:8081/family/employees"
query: [{key: "nested", value: "{{.args.criteria}}", skipEmpty: true}]
)
unknownField(criteria: Nested): [Employee!]!
@http(
url: "http://localhost:8081/family/employees"
query: [{key: "nested", value: "{{.args.criteria.unknown_field}}", skipEmpty: true}]
)
unknownArgument(criteria: Nested): [Employee!]!
@http(
url: "http://localhost:8081/family/employees"
query: [{key: "nested", value: "{{.args.criterias}}", skipEmpty: true}]
)
invalidArgument(criteria: Nested): [Employee!]!
@http(url: "http://localhost:8081/family/employees", query: [{key: "nested", value: "{{.args}}", skipEmpty: true}])
unknownArgumentType(criteria: Criteria): [Employee!]!
@http(
url: "http://localhost:8081/family/employees"
query: [{key: "nested", value: "{{.args.criteria}}", skipEmpty: true}]
)
}

type Employee {
id: ID!
}

input Nested {
hasChildren: Boolean
}
```

1 comment on commit 02c7821

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running 30s test @ http://localhost:8000/graphql

4 threads and 100 connections

Thread Stats Avg Stdev Max +/- Stdev
Latency 4.13ms 1.97ms 26.75ms 79.27%
Req/Sec 6.22k 0.89k 8.43k 93.67%

743035 requests in 30.02s, 3.72GB read

Requests/sec: 24755.19

Transfer/sec: 127.06MB

Please sign in to comment.