Skip to content

Commit

Permalink
tsparser validation improvements (#1645)
Browse files Browse the repository at this point in the history
- Reapply "tsparser: improved app validation" (#1633)
- tsparser: support validation in query/header/path
  • Loading branch information
eandre authored Dec 11, 2024
2 parents dde6fd5 + c9276da commit 2b0336c
Show file tree
Hide file tree
Showing 29 changed files with 2,143 additions and 775 deletions.
448 changes: 232 additions & 216 deletions proto/encore/parser/meta/v1/meta.pb.go

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions proto/encore/parser/meta/v1/meta.proto
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ message PathSegment {
SegmentType type = 1;
string value = 2;
ParamType value_type = 3;

optional schema.v1.ValidationExpr validation = 4;
}

message Gateway {
Expand Down
1 change: 1 addition & 0 deletions runtimes/core/src/api/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ pub fn endpoints_from_meta(
r#type: meta::path_segment::SegmentType::Literal as i32,
value_type: meta::path_segment::ParamType::String as i32,
value: format!("/{}.{}", ep.ep.service_name, ep.ep.name),
validation: None,
}],
}),
handshake,
Expand Down
3 changes: 0 additions & 3 deletions runtimes/core/src/api/jsonschema/meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,6 @@ impl BuilderCtx<'_, '_> {

#[inline]
fn struct_field<'c>(&mut self, f: &'c schema::Field) -> Result<(&'c String, Field)> {
// Note: Our JS/TS support don't include the ability to change
// the JSON name from the field name, so we use the field name unconditionally.

let typ = self.typ(&f.typ)?;
let value = match typ {
Value::Basic(basic) => BasicOrValue::Basic(basic),
Expand Down
2 changes: 1 addition & 1 deletion runtimes/core/src/api/jsonschema/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ mod de;
mod meta;
mod parse;
mod ser;
mod validation;
pub mod validation;

use crate::api::jsonschema::parse::ParseWithSchema;
use crate::api::APIResult;
Expand Down
1 change: 1 addition & 0 deletions runtimes/core/src/api/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ mod tests {
r#type: typ as i32,
value: value.to_string(),
value_type: meta::path_segment::ParamType::String as i32,
validation: None,
}
}

Expand Down
2 changes: 2 additions & 0 deletions runtimes/core/src/api/schema/encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ pub fn handshake_encoding(
value: format!("{}.{}", rpc.service_name, rpc.name),
r#type: SegmentType::Literal as i32,
value_type: meta::path_segment::ParamType::String as i32,
validation: None,
}],
r#type: meta::path::Type::Url as i32,
};
Expand Down Expand Up @@ -489,6 +490,7 @@ pub fn request_encoding(
value: format!("{}.{}", rpc.service_name, rpc.name),
r#type: SegmentType::Literal as i32,
value_type: meta::path_segment::ParamType::String as i32,
validation: None,
}],
r#type: meta::path::Type::Url as i32,
};
Expand Down
44 changes: 37 additions & 7 deletions runtimes/core/src/api/schema/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::encore::parser::meta::v1::path_segment::ParamType;
pub struct Path {
/// The path segments.
segments: Vec<Segment>,
dynamic_segments: Vec<Basic>,
dynamic_segments: Vec<(Basic, Option<jsonschema::validation::Expr>)>,

/// The capacity to use for generating requests.
capacity: usize,
Expand All @@ -32,6 +32,15 @@ impl Path {
let mut segments = Vec::with_capacity(path.segments.len());
for seg in &path.segments {
use meta::path_segment::SegmentType;
let validation = seg
.validation
.as_ref()
.map(|v| {
jsonschema::validation::Expr::try_from(v)
.context("invalid path segment validation")
})
.transpose()?;

match SegmentType::try_from(seg.r#type).context("invalid path segment type")? {
SegmentType::Literal => {
segments.push(Segment::Literal(seg.value.clone().into_boxed_str()))
Expand Down Expand Up @@ -59,14 +68,17 @@ impl Path {
segments.push(Segment::Param {
name: name.clone().into_boxed_str(),
typ,
validation,
});
}

SegmentType::Wildcard => segments.push(Segment::Wildcard {
name: seg.clone().value.into_boxed_str(),
validation,
}),
SegmentType::Fallback => segments.push(Segment::Fallback {
name: seg.clone().value.into_boxed_str(),
validation,
}),
}
}
Expand All @@ -82,14 +94,16 @@ impl Path {
capacity += 1; // slash
match seg {
Literal(lit) => capacity += lit.len(),
Param { typ, .. } => {
Param {
typ, validation, ..
} => {
capacity += 10; // assume path parameters on average are 10 characters long
dynamic_segments.push(*typ);
dynamic_segments.push((*typ, validation.clone()));
}
Wildcard { .. } | Fallback { .. } => {
Wildcard { validation, .. } | Fallback { validation, .. } => {
// Assume path parameters on average are 10 characters long.
capacity += 10;
dynamic_segments.push(jsonschema::Basic::String);
dynamic_segments.push((jsonschema::Basic::String, validation.clone()));
}
}
}
Expand All @@ -110,12 +124,15 @@ pub enum Segment {
name: Box<str>,
/// The type of the path parameter.
typ: jsonschema::Basic,
validation: Option<jsonschema::validation::Expr>,
},
Wildcard {
name: Box<str>,
validation: Option<jsonschema::validation::Expr>,
},
Fallback {
name: Box<str>,
validation: Option<jsonschema::validation::Expr>,
},
}

Expand All @@ -129,7 +146,7 @@ impl Path {
use Segment::*;
match seg {
Literal(lit) => path.push_str(lit),
Param { name, .. } | Wildcard { name } | Fallback { name } => {
Param { name, .. } | Wildcard { name, .. } | Fallback { name, .. } => {
let Some(payload) = payload else {
return Err(api::Error {
code: api::ErrCode::InvalidArgument,
Expand Down Expand Up @@ -220,7 +237,7 @@ impl Path {

// For each param, find the corresponding segment and deserialize it.
for (idx, (name, val)) in params.into_iter().enumerate() {
if let Some(typ) = self.dynamic_segments.get(idx) {
if let Some((typ, validation)) = self.dynamic_segments.get(idx) {
// Decode it into the correct type based on the type.
let val = match &typ {
// For strings and any, use the value directly.
Expand Down Expand Up @@ -270,6 +287,19 @@ impl Path {
Basic::Null => PValue::Null,
};

// Validate the value, if we have a validation expression.
if let Some(validation) = validation.as_ref() {
if let Err(err) = validation.validate_pval(&val) {
return Err(api::Error {
code: api::ErrCode::InvalidArgument,
message: format!("invalid path parameter {}: {}", name, err),
internal_message: None,
stack: None,
details: None,
});
}
}

map.insert(name, val);
}
}
Expand Down
1 change: 1 addition & 0 deletions runtimes/go/storage/sqldb/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ func (db *Database) Begin(ctx context.Context) (*Tx, error) {
// this will be made with backwards compatibility in mind, providing ample notice and
// time to migrate in an opt-in fashion.
func Driver[T SupportedDrivers](db *Database) T {
db.init()
if db.noopDB {
var zero T
return zero
Expand Down
105 changes: 94 additions & 11 deletions tsparser/src/app/mod.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};

use matchit::InsertError;
use swc_common::errors::HANDLER;
use swc_common::Span;

use crate::encore::parser::meta::v1;
use crate::legacymeta::compute_meta;
use crate::parser::parser::{ParseContext, ParseResult};
use crate::parser::resources::apis::api::{Method, Methods};
use crate::parser::resources::apis::api::{Endpoint, Method, Methods};
use crate::parser::resources::Resource;
use crate::parser::respath::Path;
use crate::parser::types::visitor::VisitWith;
use crate::parser::types::{validation, visitor, ObjectId, ResolveState, Type, Validated};
use crate::parser::Range;
use litparser::ParseResult as PResult;
use crate::span_err::ErrReporter;
use litparser::Sp;

#[derive(Debug)]
pub struct AppDesc {
Expand Down Expand Up @@ -60,9 +64,31 @@ impl Router {
}
}

impl AppDesc {
pub fn validate_and_describe(pc: &ParseContext, parse: ParseResult) -> Option<AppDesc> {
AppValidator { pc, parse: &parse }.validate();

if pc.errs.has_errors() {
return None;
}

match compute_meta(pc, &parse) {
Ok(meta) => Some(AppDesc { parse, meta }),
Err(err) => {
err.report();
None
}
}
}

struct AppValidator<'a> {
pc: &'a ParseContext,
parse: &'a ParseResult,
}

impl AppValidator<'_> {
fn validate(&self) {
self.validate_apis()
self.validate_apis();
self.validate_pubsub()
}

fn validate_apis(&self) {
Expand All @@ -72,17 +98,74 @@ impl AppDesc {
if let Resource::APIEndpoint(endpoint) = &bind.resource {
let encoding = &endpoint.encoding;
router.try_add(&encoding.methods, &encoding.path, endpoint.range);

self.validate_endpoint(endpoint);
}
}
}
}
}

pub fn validate_and_describe(pc: &ParseContext, parse: ParseResult) -> PResult<AppDesc> {
let meta = compute_meta(pc, &parse)?;
let desc = AppDesc { parse, meta };
fn validate_endpoint(&self, ep: &Endpoint) {
if let Some(schema) = &ep.encoding.raw_req_schema {
self.validate_validations(schema);
}
if let Some(schema) = &ep.encoding.raw_resp_schema {
self.validate_validations(schema);
}
}

fn validate_validations(&self, schema: &Sp<Type>) {
struct Visitor<'a> {
state: &'a ResolveState,
span: Span,
seen_decls: HashSet<ObjectId>,
}

impl visitor::Visit for Visitor<'_> {
fn resolve_state(&self) -> &ResolveState {
self.state
}
fn seen_decls(&mut self) -> &mut HashSet<ObjectId> {
&mut self.seen_decls
}

desc.validate();
fn visit_validated(&mut self, node: &Validated) {
if let Err(err) = node.expr.supports_type(&node.typ) {
let s = err.to_string();
self.span.err(&s);
} else {
// Don't recurse into the validation expression, as it would report an error
// below as if the expression was standalone.
node.typ.visit_with(self);
}
}

Ok(desc)
fn visit_validation(&mut self, node: &validation::Expr) {
HANDLER.with(|h| {
h.struct_span_err(
self.span,
&format!("unsupported standalone validation expression: {}", node),
)
.note("validation expressions must be attached to a regular type using '&'")
.emit()
});
}
}

let state = self.pc.type_checker.state();
let mut visitor = Visitor {
state,
span: schema.span(),
seen_decls: HashSet::new(),
};
schema.visit_with(&mut visitor);
}

fn validate_pubsub(&self) {
for res in self.parse.resources.iter() {
if let Resource::PubSubTopic(topic) = &res {
self.validate_validations(&topic.message_type);
}
}
}
}
8 changes: 1 addition & 7 deletions tsparser/src/builder/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,7 @@ impl Builder<'_> {
let parser = Parser::new(pc, pass1);

let result = parser.parse();
let desc = match validate_and_describe(pc, result) {
Ok(desc) => desc,
Err(err) => {
err.report();
return None;
}
};
let desc = validate_and_describe(pc, result)?;

if pc.errs.has_errors() {
None
Expand Down
15 changes: 12 additions & 3 deletions tsparser/src/legacymeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::parser::resources::apis::{authhandler, gateway};
use crate::parser::resources::infra::cron::CronJobSchedule;
use crate::parser::resources::infra::{cron, objects, pubsub_subscription, pubsub_topic, sqldb};
use crate::parser::resources::Resource;
use crate::parser::types::validation;
use crate::parser::types::{Object, ObjectId};
use crate::parser::usageparser::Usage;
use crate::parser::{respath, FilePath, Range};
Expand Down Expand Up @@ -696,25 +697,33 @@ impl respath::Path {
r#type: SegmentType::Literal as i32,
value_type: ParamType::String as i32,
value: lit.clone(),
validation: None,
},
Segment::Param { name, value_type } => v1::PathSegment {
Segment::Param {
name,
value_type,
validation,
} => v1::PathSegment {
r#type: SegmentType::Param as i32,
value_type: match value_type {
ValueType::String => ParamType::String as i32,
ValueType::Int => ParamType::Int as i32,
ValueType::Bool => ParamType::Bool as i32,
},
value: name.clone(),
validation: validation.as_ref().map(validation::Expr::to_pb),
},
Segment::Wildcard { name } => v1::PathSegment {
Segment::Wildcard { name, validation } => v1::PathSegment {
r#type: SegmentType::Wildcard as i32,
value_type: ParamType::String as i32,
value: name.clone(),
validation: validation.as_ref().map(validation::Expr::to_pb),
},
Segment::Fallback { name } => v1::PathSegment {
Segment::Fallback { name, validation } => v1::PathSegment {
r#type: SegmentType::Fallback as i32,
value_type: ParamType::String as i32,
value: name.clone(),
validation: validation.as_ref().map(validation::Expr::to_pb),
},
})
.collect(),
Expand Down
Loading

0 comments on commit 2b0336c

Please sign in to comment.