From 493f5acd16ad92adf99c963659cd40dc5eac1219 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 24 Jan 2024 11:39:08 -0500 Subject: [PATCH] Additional `SourceSpecDefinition` validations (#2914) Follow-up to PR #2910. --- .changeset/selfish-cooks-nail.md | 5 + composition-js/src/__tests__/compose.test.ts | 111 +++++++- docs/source/errors.md | 1 + internals-js/src/error.ts | 7 + internals-js/src/specs/sourceSpec.ts | 264 ++++++++++--------- 5 files changed, 266 insertions(+), 122 deletions(-) create mode 100644 .changeset/selfish-cooks-nail.md diff --git a/.changeset/selfish-cooks-nail.md b/.changeset/selfish-cooks-nail.md new file mode 100644 index 000000000..e849da07b --- /dev/null +++ b/.changeset/selfish-cooks-nail.md @@ -0,0 +1,5 @@ +--- +"@apollo/federation-internals": patch +--- + +Additional `SourceSpecDefinition` validations and bug fixes diff --git a/composition-js/src/__tests__/compose.test.ts b/composition-js/src/__tests__/compose.test.ts index b4956afd1..c3770a657 100644 --- a/composition-js/src/__tests__/compose.test.ts +++ b/composition-js/src/__tests__/compose.test.ts @@ -4996,9 +4996,30 @@ describe('@source* directives', () => { } `; + // TODO Test the following errors using badSchema: + // - [x] SOURCE_FEDERATION_VERSION_REQUIRED, + // - [x] SOURCE_API_NAME_INVALID, + // - [x] SOURCE_API_PROTOCOL_INVALID, + // - [x] SOURCE_API_HTTP_BASE_URL_INVALID, + // - [x] SOURCE_HTTP_HEADERS_INVALID, + // - [x] SOURCE_TYPE_API_ERROR, + // - [x] SOURCE_TYPE_PROTOCOL_INVALID, + // - [x] SOURCE_TYPE_HTTP_METHOD_INVALID, + // - [ ] SOURCE_TYPE_HTTP_PATH_INVALID, + // - [ ] SOURCE_TYPE_HTTP_BODY_INVALID, + // - [x] SOURCE_TYPE_ON_NON_OBJECT_OR_NON_ENTITY, + // - [ ] SOURCE_TYPE_SELECTION_INVALID, + // - [x] SOURCE_FIELD_API_ERROR, + // - [ ] SOURCE_FIELD_PROTOCOL_INVALID, + // - [x] SOURCE_FIELD_HTTP_METHOD_INVALID, + // - [ ] SOURCE_FIELD_HTTP_PATH_INVALID, + // - [ ] SOURCE_FIELD_HTTP_BODY_INVALID, + // - [ ] SOURCE_FIELD_SELECTION_INVALID, + // - [x] SOURCE_FIELD_NOT_ON_ROOT_OR_ENTITY_FIELD, + const badSchema = gql` extend schema - @link(url: "https://specs.apollo.dev/federation/v2.7", import: ["@key"]) + @link(url: "https://specs.apollo.dev/federation/v2.5", import: ["@key"]) @link(url: "https://specs.apollo.dev/source/v0.1", import: [ "@sourceAPI" "@sourceType" @@ -5008,6 +5029,18 @@ describe('@source* directives', () => { name: "A?!" # Should be valid GraphQL identifier http: { baseURL: "https://api.a.com/v1" } ) + @sourceAPI( + name: "Bogus" + http: { + baseURL: "not a url" + headers: [ + { name: "i n v a l i d", value: "header value", as: "re|named" } + ] + } + ) + @sourceAPI( + name: "NoProtocol" + ) { query: Query } @@ -5015,7 +5048,10 @@ describe('@source* directives', () => { type Query { resources: [Resource!]! @sourceField( api: "A" - http: { GET: "/resources" } + http: { + GET: "/resources" + DELETE: "/resources" + } ) } @@ -5023,10 +5059,30 @@ describe('@source* directives', () => { api: "A" http: { GET: "/resources/{id}" } selection: "id description" + ) + @sourceType( + api: "Bogus" + http: { + GET: "/resources/{id}" + POST: "/resources" + } + selection: "id" ) { id: ID! description: String! } + + type NonEntity @sourceType( + api: "A" + # http: { GET: "/nonentities/{id}" } + selection: "id some_field" + ) { + id: ID! + someField: String! @sourceField( + api: "A" + selection: ".some_field" + ) + } `; it('good schema composes without validation errors', () => { @@ -5046,7 +5102,15 @@ describe('@source* directives', () => { const messages = result.errors!.map(e => e.message); expect(messages).toContain( - '[bad] @sourceAPI(name: "A?!") must specify valid GraphQL name' + '[bad] Schemas that @link to https://specs.apollo.dev/source must also @link to federation version v2.7 or later (found v2.5)' + ); + + expect(messages).toContain( + '[bad] @sourceAPI(name: "A?!") must specify name using only [a-zA-Z0-9-_] characters' + ); + + expect(messages).toContain( + '[bad] @sourceAPI must specify one protocol from the set {http}' ); expect(messages).toContain( @@ -5056,6 +5120,45 @@ describe('@source* directives', () => { expect(messages).toContain( '[bad] @sourceField specifies unknown api A' ); + + try { + new URL('not a url'); + throw new Error('should have thrown'); + } catch (e) { + expect(messages).toContain( + // Different versions of Node.js stringify the URL error differently, + // so we avoid hard-coding that part of the expected error. + `[bad] @sourceAPI http.baseURL \"not a url\" must be valid URL (error: ${e.message})` + ); + } + + expect(messages).toContain( + '[bad] @sourceAPI header {\"name\":\"i n v a l i d\",\"value\":\"header value\",\"as\":\"re|named\"} specifies invalid name' + ); + + expect(messages).toContain( + '[bad] @sourceAPI header {\"name\":\"i n v a l i d\",\"value\":\"header value\",\"as\":\"re|named\"} specifies invalid \'as\' name' + ); + + expect(messages).toContain( + '[bad] @sourceType must specify exactly one of http.GET or http.POST' + ); + + expect(messages).toContain( + '[bad] @sourceField allows at most one of http.{GET,POST,PUT,PATCH,DELETE}' + ); + + expect(messages).toContain( + '[bad] @sourceType must be applied to an entity type that also has a @key directive' + ); + + expect(messages).toContain( + '[bad] @sourceType must specify same http argument as corresponding @sourceAPI for api A' + ); + + expect(messages).toContain( + '[bad] @sourceField must be applied to root Query or Mutation field or field of entity type' + ); }); const renamedSchema = gql` @@ -5100,7 +5203,7 @@ describe('@source* directives', () => { const messages = result.errors!.map(e => e.message); expect(messages).toContain( - '[renamed] @api(name: "not an identifier") must specify valid GraphQL name' + '[renamed] @api(name: "not an identifier") must specify name using only [a-zA-Z0-9-_] characters' ); }); }); diff --git a/docs/source/errors.md b/docs/source/errors.md index 5315a82ba..a2aa743ac 100644 --- a/docs/source/errors.md +++ b/docs/source/errors.md @@ -93,6 +93,7 @@ The following errors might be raised during composition: | `SOURCE_API_HTTP_BASE_URL_INVALID` | The `@sourceAPI` directive must specify a valid http.baseURL | 2.7.0 | | | `SOURCE_API_NAME_INVALID` | Each `@sourceAPI` directive must take a unique and valid name as an argument | 2.7.0 | | | `SOURCE_API_PROTOCOL_INVALID` | Each `@sourceAPI` directive must specify exactly one of the known protocols | 2.7.0 | | +| `SOURCE_FEDERATION_VERSION_REQUIRED` | Schemas using `@source{API,Type,Field}` directives must @link-import v2.7 or later of federation | 2.7.1 | | | `SOURCE_FIELD_API_ERROR` | The `api` argument of the `@sourceField` directive must match a valid `@sourceAPI` name | 2.7.0 | | | `SOURCE_FIELD_HTTP_BODY_INVALID` | If `@sourceField` specifies http.body, it must be a valid `JSONSelection` matching available arguments and fields | 2.7.0 | | | `SOURCE_FIELD_HTTP_METHOD_INVALID` | The `@sourceField` directive must specify at most one of `http.{GET,POST,PUT,PATCH,DELETE}` | 2.7.0 | | diff --git a/internals-js/src/error.ts b/internals-js/src/error.ts index f13a97a4b..aac2c205a 100644 --- a/internals-js/src/error.ts +++ b/internals-js/src/error.ts @@ -561,6 +561,12 @@ const INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE = makeCodeDefinition( { addedIn: '2.3.0' }, ) +const SOURCE_FEDERATION_VERSION_REQUIRED = makeCodeDefinition( + 'SOURCE_FEDERATION_VERSION_REQUIRED', + 'Schemas using `@source{API,Type,Field}` directives must @link-import v2.7 or later of federation', + { addedIn: '2.7.1' }, +); + const SOURCE_API_NAME_INVALID = makeCodeDefinition( 'SOURCE_API_NAME_INVALID', 'Each `@sourceAPI` directive must take a unique and valid name as an argument', @@ -757,6 +763,7 @@ export const ERRORS = { INTERFACE_KEY_NOT_ON_IMPLEMENTATION, INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE, // Errors related to @sourceAPI, @sourceType, and/or @sourceField + SOURCE_FEDERATION_VERSION_REQUIRED, SOURCE_API_NAME_INVALID, SOURCE_API_PROTOCOL_INVALID, SOURCE_API_HTTP_BASE_URL_INVALID, diff --git a/internals-js/src/specs/sourceSpec.ts b/internals-js/src/specs/sourceSpec.ts index 31765a5fd..ad59abbf2 100644 --- a/internals-js/src/specs/sourceSpec.ts +++ b/internals-js/src/specs/sourceSpec.ts @@ -1,4 +1,4 @@ -import { DirectiveLocation, GraphQLError, assertName } from 'graphql'; +import { DirectiveLocation, GraphQLError, Kind } from 'graphql'; import { FeatureDefinition, FeatureDefinitions, FeatureUrl, FeatureVersion, LinkDirectiveArgs } from "./coreSpec"; import { Schema, @@ -16,7 +16,7 @@ import { ERRORS } from '../error'; export const sourceIdentity = 'https://specs.apollo.dev/source'; export class SourceSpecDefinition extends FeatureDefinition { - constructor(version: FeatureVersion, minimumFederationVersion?: FeatureVersion) { + constructor(version: FeatureVersion, readonly minimumFederationVersion: FeatureVersion) { super(new FeatureUrl(sourceIdentity, 'source', version), minimumFederationVersion); this.registerDirective(createDirectiveSpecification({ @@ -147,17 +147,20 @@ export class SourceSpecDefinition extends FeatureDefinition { return this.directive(schema, 'sourceField')!; } - private getSourceDirectives(schema: Schema) { + private getSourceDirectives(schema: Schema, errors: GraphQLError[]) { const result: { sourceAPI?: DirectiveDefinition; sourceType?: DirectiveDefinition; sourceField?: DirectiveDefinition; } = {}; + let federationVersion: FeatureVersion | undefined; + schema.schemaDefinition.appliedDirectivesOf('link') .forEach(linkDirective => { const { url, import: imports } = linkDirective.arguments(); - if (imports && FeatureUrl.maybeParse(url)?.identity === sourceIdentity) { + const featureUrl = FeatureUrl.maybeParse(url); + if (imports && featureUrl && featureUrl.identity === sourceIdentity) { imports.forEach(nameOrRename => { const originalName = typeof nameOrRename === 'string' ? nameOrRename : nameOrRename.name; const importedName = typeof nameOrRename === 'string' ? nameOrRename : nameOrRename.as || originalName; @@ -172,17 +175,35 @@ export class SourceSpecDefinition extends FeatureDefinition { } }); } + if (featureUrl && featureUrl.name === 'federation') { + federationVersion = featureUrl.version; + } }); + if (result.sourceAPI || result.sourceType || result.sourceField) { + // Since this subgraph uses at least one of the @source{API,Type,Field} + // directives, it must also use v2.7 or later of federation. + if (!federationVersion || federationVersion.lt(this.minimumFederationVersion)) { + errors.push(ERRORS.SOURCE_FEDERATION_VERSION_REQUIRED.err( + `Schemas that @link to ${ + sourceIdentity + } must also @link to federation version ${ + this.minimumFederationVersion + } or later (found ${federationVersion})`, + )); + } + } + return result; } override validateSubgraphSchema(schema: Schema): GraphQLError[] { + const errors = super.validateSubgraphSchema(schema); const { sourceAPI, sourceType, sourceField, - } = this.getSourceDirectives(schema); + } = this.getSourceDirectives(schema, errors); if (!(sourceAPI || sourceType || sourceField)) { // If none of the @source* directives are present, nothing needs @@ -191,7 +212,6 @@ export class SourceSpecDefinition extends FeatureDefinition { } const apiNameToProtocol = new Map(); - const errors: GraphQLError[] = []; if (sourceAPI) { this.validateSourceAPI(sourceAPI, apiNameToProtocol, errors); @@ -216,20 +236,18 @@ export class SourceSpecDefinition extends FeatureDefinition { sourceAPI.applications().forEach(application => { const { name, ...rest } = application.arguments(); - if (apiNameToProtocol.has(name)) { + if (!isValidSourceAPIName(name)) { errors.push(ERRORS.SOURCE_API_NAME_INVALID.err( - `${sourceAPI} must specify unique name`, + `${sourceAPI}(name: ${ + JSON.stringify(name) + }) must specify name using only [a-zA-Z0-9-_] characters`, { nodes: application.sourceAST }, )); } - try { - assertName(name); - } catch (e) { + if (apiNameToProtocol.has(name)) { errors.push(ERRORS.SOURCE_API_NAME_INVALID.err( - `${sourceAPI}(name: ${ - JSON.stringify(name) - }) must specify valid GraphQL name`, + `${sourceAPI} must specify unique name (${JSON.stringify(name)} reused)`, { nodes: application.sourceAST }, )); } @@ -239,7 +257,9 @@ export class SourceSpecDefinition extends FeatureDefinition { if (rest[knownProtocol]) { if (protocol) { errors.push(ERRORS.SOURCE_API_PROTOCOL_INVALID.err( - `${sourceAPI} must specify only one of ${KNOWN_SOURCE_PROTOCOLS.join(', ')}`, + `${sourceAPI} must specify only one of ${ + KNOWN_SOURCE_PROTOCOLS.join(', ') + } but specified both ${protocol} and ${knownProtocol}`, { nodes: application.sourceAST }, )); } @@ -258,7 +278,7 @@ export class SourceSpecDefinition extends FeatureDefinition { new URL(baseURL); } catch (e) { errors.push(ERRORS.SOURCE_API_HTTP_BASE_URL_INVALID.err( - `${sourceAPI} http.baseURL ${JSON.stringify(baseURL)} must be valid URL`, + `${sourceAPI} http.baseURL ${JSON.stringify(baseURL)} must be valid URL (error: ${e.message})`, { nodes: application.sourceAST }, )); } @@ -267,7 +287,7 @@ export class SourceSpecDefinition extends FeatureDefinition { } } else { errors.push(ERRORS.SOURCE_API_PROTOCOL_INVALID.err( - `${sourceAPI} must specify one of ${KNOWN_SOURCE_PROTOCOLS.join(', ')}`, + `${sourceAPI} must specify one protocol from the set {${KNOWN_SOURCE_PROTOCOLS.join(',')}}`, { nodes: application.sourceAST }, )); } @@ -286,58 +306,58 @@ export class SourceSpecDefinition extends FeatureDefinition { `${sourceType} specifies unknown api ${api}`, { nodes: application.sourceAST }, )); - } else { - const expectedProtocol = apiNameToProtocol.get(api); - const protocolValue = expectedProtocol && rest[expectedProtocol]; - if (expectedProtocol && !protocolValue) { - errors.push(ERRORS.SOURCE_TYPE_API_ERROR.err( - `${sourceType} must specify same ${ - expectedProtocol - } argument as corresponding @sourceAPI for api ${api}`, + } + + const expectedProtocol = apiNameToProtocol.get(api) || HTTP_PROTOCOL; + const protocolValue = expectedProtocol && rest[expectedProtocol]; + if (expectedProtocol && !protocolValue) { + errors.push(ERRORS.SOURCE_TYPE_PROTOCOL_INVALID.err( + `${sourceType} must specify same ${ + expectedProtocol + } argument as corresponding @sourceAPI for api ${api}`, + { nodes: application.sourceAST }, + )); + } + + if (protocolValue && expectedProtocol === HTTP_PROTOCOL) { + const { GET, POST, headers, body } = protocolValue as HTTPSourceType; + + if ([GET, POST].filter(Boolean).length !== 1) { + errors.push(ERRORS.SOURCE_TYPE_HTTP_METHOD_INVALID.err( + `${sourceType} must specify exactly one of http.GET or http.POST`, { nodes: application.sourceAST }, )); + } else { + const urlPathTemplate = (GET || POST)!; + try { + // TODO Validate URL path template uses only available @key fields + // of the type. + parseURLPathTemplate(urlPathTemplate); + } catch (e) { + errors.push(ERRORS.SOURCE_TYPE_HTTP_PATH_INVALID.err( + `${sourceType} http.GET or http.POST must be valid URL path template (error: ${e.message})` + )); + } } - if (protocolValue && expectedProtocol === HTTP_PROTOCOL) { - const { GET, POST, headers, body } = protocolValue as HTTPSourceType; + validateHTTPHeaders(headers, errors, sourceType.name); - if ([GET, POST].filter(Boolean).length !== 1) { - errors.push(ERRORS.SOURCE_TYPE_HTTP_METHOD_INVALID.err( - `${sourceType} must specify exactly one of http.GET or http.POST`, + if (body) { + if (GET) { + errors.push(ERRORS.SOURCE_TYPE_HTTP_BODY_INVALID.err( + `${sourceType} http.GET cannot specify http.body`, { nodes: application.sourceAST }, )); - } else { - const urlPathTemplate = (GET || POST)!; - try { - // TODO Validate URL path template uses only available @key fields - // of the type. - parseURLPathTemplate(urlPathTemplate); - } catch (e) { - errors.push(ERRORS.SOURCE_TYPE_HTTP_PATH_INVALID.err( - `${sourceType} http.GET or http.POST must be valid URL path template` - )); - } } - validateHTTPHeaders(headers, errors, sourceType.name); - - if (body) { - if (GET) { - errors.push(ERRORS.SOURCE_TYPE_HTTP_BODY_INVALID.err( - `${sourceType} http.GET cannot specify http.body`, - { nodes: application.sourceAST }, - )); - } - - try { - parseJSONSelection(body); - // TODO Validate body selection matches the available fields. - } catch (e) { - errors.push(ERRORS.SOURCE_TYPE_HTTP_BODY_INVALID.err( - `${sourceType} http.body not valid JSONSelection: ${e.message}`, - { nodes: application.sourceAST }, - )); - } + try { + parseJSONSelection(body); + // TODO Validate body selection matches the available fields. + } catch (e) { + errors.push(ERRORS.SOURCE_TYPE_HTTP_BODY_INVALID.err( + `${sourceType} http.body not valid JSONSelection (error: ${e.message})`, + { nodes: application.sourceAST }, + )); } } } @@ -357,7 +377,7 @@ export class SourceSpecDefinition extends FeatureDefinition { // TODO Validate selection is valid JSONSelection for type. } catch (e) { errors.push(ERRORS.SOURCE_TYPE_SELECTION_INVALID.err( - `${sourceType} selection not valid JSONSelection: ${e.message}`, + `${sourceType} selection not valid JSONSelection (error: ${e.message})`, { nodes: application.sourceAST }, )); } @@ -383,59 +403,59 @@ export class SourceSpecDefinition extends FeatureDefinition { `${sourceField} specifies unknown api ${api}`, { nodes: application.sourceAST }, )); - } else { - const expectedProtocol = apiNameToProtocol.get(api); - const protocolValue = expectedProtocol && rest[expectedProtocol]; - if (protocolValue && expectedProtocol === HTTP_PROTOCOL) { - const { - GET, POST, PUT, PATCH, DELETE, - headers, - body, - } = protocolValue as HTTPSourceField; - - const usedMethods = [GET, POST, PUT, PATCH, DELETE].filter(Boolean); - if (usedMethods.length > 1) { - errors.push(ERRORS.SOURCE_FIELD_HTTP_METHOD_INVALID.err( - `${sourceField} allows at most one of http.{GET,POST,PUT,PATCH,DELETE}`, + } + + const expectedProtocol = apiNameToProtocol.get(api) || HTTP_PROTOCOL; + const protocolValue = expectedProtocol && rest[expectedProtocol]; + if (protocolValue && expectedProtocol === HTTP_PROTOCOL) { + const { + GET, POST, PUT, PATCH, DELETE, + headers, + body, + } = protocolValue as HTTPSourceField; + + const usedMethods = [GET, POST, PUT, PATCH, DELETE].filter(Boolean); + if (usedMethods.length > 1) { + errors.push(ERRORS.SOURCE_FIELD_HTTP_METHOD_INVALID.err( + `${sourceField} allows at most one of http.{GET,POST,PUT,PATCH,DELETE}`, + )); + } else if (usedMethods.length === 1) { + const urlPathTemplate = usedMethods[0]!; + try { + // TODO Validate URL path template uses only available fields of + // the type and/or argument names of the field. + parseURLPathTemplate(urlPathTemplate); + } catch (e) { + errors.push(ERRORS.SOURCE_FIELD_HTTP_PATH_INVALID.err( + `${sourceField} http.{GET,POST,PUT,PATCH,DELETE} must be valid URL path template (error: ${e.message})` )); - } else if (usedMethods.length === 1) { - const urlPathTemplate = usedMethods[0]!; - try { - // TODO Validate URL path template uses only available fields of - // the type and/or argument names of the field. - parseURLPathTemplate(urlPathTemplate); - } catch (e) { - errors.push(ERRORS.SOURCE_FIELD_HTTP_PATH_INVALID.err( - `${sourceField} http.{GET,POST,PUT,PATCH,DELETE} must be valid URL path template` - )); - } } + } - validateHTTPHeaders(headers, errors, sourceField.name); - - if (body) { - if (GET) { - errors.push(ERRORS.SOURCE_FIELD_HTTP_BODY_INVALID.err( - `${sourceField} http.GET cannot specify http.body`, - { nodes: application.sourceAST }, - )); - } else if (DELETE) { - errors.push(ERRORS.SOURCE_FIELD_HTTP_BODY_INVALID.err( - `${sourceField} http.DELETE cannot specify http.body`, - { nodes: application.sourceAST }, - )); - } + validateHTTPHeaders(headers, errors, sourceField.name); - try { - parseJSONSelection(body); - // TODO Validate body string matches the available fields of the - // parent type and/or argument names of the field. - } catch (e) { - errors.push(ERRORS.SOURCE_FIELD_HTTP_BODY_INVALID.err( - `${sourceField} http.body not valid JSONSelection: ${e.message}`, - { nodes: application.sourceAST }, - )); - } + if (body) { + if (GET) { + errors.push(ERRORS.SOURCE_FIELD_HTTP_BODY_INVALID.err( + `${sourceField} http.GET cannot specify http.body`, + { nodes: application.sourceAST }, + )); + } else if (DELETE) { + errors.push(ERRORS.SOURCE_FIELD_HTTP_BODY_INVALID.err( + `${sourceField} http.DELETE cannot specify http.body`, + { nodes: application.sourceAST }, + )); + } + + try { + parseJSONSelection(body); + // TODO Validate body string matches the available fields of the + // parent type and/or argument names of the field. + } catch (e) { + errors.push(ERRORS.SOURCE_FIELD_HTTP_BODY_INVALID.err( + `${sourceField} http.body not valid JSONSelection (error: ${e.message})`, + { nodes: application.sourceAST }, + )); } } } @@ -447,7 +467,7 @@ export class SourceSpecDefinition extends FeatureDefinition { // the parent type and/or argument names of the field. } catch (e) { errors.push(ERRORS.SOURCE_FIELD_SELECTION_INVALID.err( - `${sourceField} selection not valid JSONSelection: ${e.message}`, + `${sourceField} selection not valid JSONSelection (error: ${e.message})`, { nodes: application.sourceAST }, )); } @@ -456,14 +476,14 @@ export class SourceSpecDefinition extends FeatureDefinition { // @sourceField is allowed only on root Query and Mutation fields or // fields of entity object types. const fieldParent = application.parent; - if (fieldParent.sourceAST?.kind !== "FieldDefinition") { + if (fieldParent.sourceAST?.kind !== Kind.FIELD_DEFINITION) { errors.push(ERRORS.SOURCE_FIELD_NOT_ON_ROOT_OR_ENTITY_FIELD.err( `${sourceField} must be applied to field`, { nodes: application.sourceAST }, )); } else { const typeGrandparent = fieldParent.parent as SchemaElement; - if (typeGrandparent.sourceAST?.kind !== "ObjectTypeDefinition") { + if (typeGrandparent.sourceAST?.kind !== Kind.OBJECT_TYPE_DEFINITION) { errors.push(ERRORS.SOURCE_FIELD_NOT_ON_ROOT_OR_ENTITY_FIELD.err( `${sourceField} must be applied to field of object type`, { nodes: application.sourceAST }, @@ -486,6 +506,10 @@ export class SourceSpecDefinition extends FeatureDefinition { } } +function isValidSourceAPIName(name: string): boolean { + return /^[a-z-_][a-z0-9-_]*$/i.test(name); +} + function isValidHTTPHeaderName(name: string): boolean { // https://developers.cloudflare.com/rules/transform/request-header-modification/reference/header-format/ return /^[a-zA-Z0-9-_]+$/.test(name); @@ -504,15 +528,19 @@ function validateHTTPHeaders( // Ensure name is a valid HTTP header name. if (!isValidHTTPHeaderName(name)) { errors.push(ERRORS.SOURCE_HTTP_HEADERS_INVALID.err( - `${directiveName} headers[${i}].name == ${ - JSON.stringify(name) - } is not valid HTTP header name`, + `${directiveName} header ${JSON.stringify(headers[i])} specifies invalid name`, + )); + } + + if (as && !isValidHTTPHeaderName(as)) { + errors.push(ERRORS.SOURCE_HTTP_HEADERS_INVALID.err( + `${directiveName} header ${JSON.stringify(headers[i])} specifies invalid 'as' name`, )); } - if (!as === !value) { + if (as && value) { errors.push(ERRORS.SOURCE_HTTP_HEADERS_INVALID.err( - `${directiveName} headers[${i}] must specify exactly one of as or value`, + `${directiveName} header ${JSON.stringify(headers[i])} should specify at most one of 'as' or 'value'`, )); }