Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions internal/test/spec_validation/spec_validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Package spec_validation exercises codegen.ValidateSpec against specs that
// place a hostile value in a single sensitive field. Each testdata spec targets
// exactly one field, because ValidateSpec reports every problem it finds and we
// want each case to assert on a specific, friendly message. A separate spec
// proves that values which merely look suspicious but are legitimate (media-type
// parameters with quotes, multi-line descriptions) are accepted.
package spec_validation

import (
"path/filepath"
"testing"

"github.com/oapi-codegen/oapi-codegen/v2/pkg/codegen"
"github.com/oapi-codegen/oapi-codegen/v2/pkg/util"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestValidateSpecRejects(t *testing.T) {
// file -> substring expected in the (friendly) error message.
cases := map[string]string{
"path_newline.yaml": "path",
"parameter_name_quote.yaml": "parameter name",
"property_name_backtick.yaml": "property name",
"content_type_newline.yaml": "content type",
"enum_value_newline.yaml": "enum value",
"discriminator_quote.yaml": "discriminator",
"header_name_quote.yaml": "header",
"extra_tags_backtick.yaml": "x-oapi-codegen-extra-tags",
"x_go_name_invalid.yaml": "x-go-name",
"x_go_type_semicolon.yaml": "x-go-type",
"ref_sibling_x_go_type.yaml": "x-go-type",
"security_scope_quote.yaml": "scope",
}

for file, want := range cases {
t.Run(file, func(t *testing.T) {
Comment on lines +22 to +37

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 No test case for x-go-type-name validation

ValidateSpec validates x-go-type-name via checkIdentifier in checkExtensions, but TestValidateSpecRejects has no entry for it. The test matrix covers x-go-name and x-go-type but leaves x-go-type-name (the extGoTypeName extension) untested. A test spec with an invalid x-go-type-name value (e.g. x-go-type-name: "9lives") should be added alongside the other x_go_* fixtures.

Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/test/spec_validation/spec_validation_test.go
Line: 22-35

Comment:
**No test case for `x-go-type-name` validation**

`ValidateSpec` validates `x-go-type-name` via `checkIdentifier` in `checkExtensions`, but `TestValidateSpecRejects` has no entry for it. The test matrix covers `x-go-name` and `x-go-type` but leaves `x-go-type-name` (the `extGoTypeName` extension) untested. A test spec with an invalid `x-go-type-name` value (e.g. `x-go-type-name: "9lives"`) should be added alongside the other `x_go_*` fixtures.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

spec, err := util.LoadSwagger(filepath.Join("testdata", file))
require.NoError(t, err, "test spec should load")

err = codegen.ValidateSpec(spec)
require.Error(t, err, "ValidateSpec should reject %s", file)
assert.Contains(t, err.Error(), want, "error message should point at the offending field")
})
}
}

func TestValidateSpecAcceptsLegitimateSpec(t *testing.T) {
spec, err := util.LoadSwagger(filepath.Join("testdata", "valid.yaml"))
require.NoError(t, err)
require.NoError(t, codegen.ValidateSpec(spec), "a legitimate spec must not be rejected")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
openapi: "3.0.0"
info: {title: t, version: "1.0.0"}
paths:
/foo:
post:
operationId: postFoo
requestBody:
content:
"application/json\ninjected":
schema: {type: string}
responses:
'200': {description: ok}
14 changes: 14 additions & 0 deletions internal/test/spec_validation/testdata/discriminator_quote.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
openapi: "3.0.0"
info: {title: t, version: "1.0.0"}
paths: {}
components:
schemas:
Pet:
oneOf:
- $ref: '#/components/schemas/Cat'
discriminator:
propertyName: 'ev"il'
Cat:
type: object
properties:
name: {type: string}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
openapi: "3.0.0"
info: {title: t, version: "1.0.0"}
paths: {}
components:
schemas:
Color:
type: string
enum:
- "blue\ninjected"
12 changes: 12 additions & 0 deletions internal/test/spec_validation/testdata/extra_tags_backtick.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
openapi: "3.0.0"
info: {title: t, version: "1.0.0"}
paths: {}
components:
schemas:
Thing:
type: object
properties:
name:
type: string
x-oapi-codegen-extra-tags:
validate: 'ev`il'
12 changes: 12 additions & 0 deletions internal/test/spec_validation/testdata/header_name_quote.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
openapi: "3.0.0"
info: {title: t, version: "1.0.0"}
paths:
/foo:
get:
operationId: getFoo
responses:
'200':
description: ok
headers:
'X-Ev"il':
schema: {type: string}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
openapi: "3.0.0"
info: {title: t, version: "1.0.0"}
paths:
/foo:
get:
operationId: getFoo
parameters:
- name: 'ev"il'
in: query
schema: {type: string}
responses:
'200': {description: ok}
8 changes: 8 additions & 0 deletions internal/test/spec_validation/testdata/path_newline.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
openapi: "3.0.0"
info: {title: t, version: "1.0.0"}
paths:
"/foo\ninjected":
get:
operationId: getFoo
responses:
'200': {description: ok}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
openapi: "3.0.0"
info: {title: t, version: "1.0.0"}
paths: {}
components:
schemas:
Thing:
type: object
properties:
'ev`il': {type: string}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
openapi: "3.0.0"
info: {title: t, version: "1.0.0"}
paths: {}
components:
schemas:
Thing:
type: object
properties:
child:
# An x-go-type sibling next to a $ref is honoured by codegen, so the
# ref node's own extensions must be validated even though the target
# (Child) is not traversed here.
$ref: '#/components/schemas/Child'
x-go-type: "int; var pwned = 1"
Child:
type: object
properties:
name: {type: string}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
openapi: "3.0.0"
info: {title: t, version: "1.0.0"}
paths:
/foo:
get:
operationId: getFoo
# The scope string here is copied verbatim into a generated []string, so a
# quote in it must be rejected.
security:
- petstore_auth: ['ev"il']
responses:
'200': {description: ok}
components:
securitySchemes:
petstore_auth:
type: oauth2
flows:
implicit:
authorizationUrl: https://example.com/auth
scopes:
read: read access
21 changes: 21 additions & 0 deletions internal/test/spec_validation/testdata/valid.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
openapi: "3.0.0"
info: {title: valid, version: "1.0.0"}
paths:
/things:
post:
operationId: createThing
requestBody:
content:
'application/ld+json; profile="https://www.w3.org/ns/activitystreams"':
schema: {$ref: '#/components/schemas/Thing'}
responses:
'200': {description: ok}
components:
schemas:
Thing:
type: object
description: |
A multi-line description
spanning several lines is fine.
properties:
name: {type: string}
11 changes: 11 additions & 0 deletions internal/test/spec_validation/testdata/x_go_name_invalid.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
openapi: "3.0.0"
info: {title: t, version: "1.0.0"}
paths: {}
components:
schemas:
Thing:
type: object
properties:
name:
type: string
x-go-name: "9lives has spaces"
11 changes: 11 additions & 0 deletions internal/test/spec_validation/testdata/x_go_type_semicolon.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
openapi: "3.0.0"
info: {title: t, version: "1.0.0"}
paths: {}
components:
schemas:
Thing:
type: object
properties:
name:
type: string
x-go-type: "int; var pwned = 1"
8 changes: 8 additions & 0 deletions pkg/codegen/codegen.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,14 @@ func Generate(spec *openapi3.T, opts Configuration) (string, error) {
pruneUnusedComponents(spec)
}

// Reject spec values that cannot be represented in the generated Go source
// (names, media types, enum values, extension hints containing quotes,
// backticks, or control characters). Run after filtering/pruning so only
// values that will actually be emitted are considered.
if err := ValidateSpec(spec); err != nil {
return "", err
}

// if we are provided an override for the response type suffix update it
if opts.OutputOptions.ResponseTypeSuffix != "" {
responseTypeSuffix = opts.OutputOptions.ResponseTypeSuffix
Expand Down
Loading