Skip to content

Commit

Permalink
Merge pull request #763 from zalando/714-rules-to-aos--AvoidLinkHeade…
Browse files Browse the repository at this point in the history
…rsRule

714-rules-to-aos--AvoidLinkHeadersRule
  • Loading branch information
David Dufour-Boivin committed Jul 26, 2018
2 parents add1ab6 + 10f787b commit 2702979
Show file tree
Hide file tree
Showing 6 changed files with 206 additions and 46 deletions.
44 changes: 30 additions & 14 deletions server/src/main/java/de/zalando/zally/rule/Context.kt
Original file line number Diff line number Diff line change
Expand Up @@ -122,30 +122,46 @@ class Context(openApi: OpenAPI, swagger: Swagger? = null) {
private val log = LoggerFactory.getLogger(Context::class.java)
val extensionNames = arrayOf("getVendorExtensions", "getExtensions")

fun createOpenApiContext(content: String): Context? {
fun createOpenApiContext(content: String, failOnParseErrors: Boolean = false): Context? {
val parseOptions = ParseOptions()
parseOptions.isResolve = true
// parseOptions.isResolveFully = true // https://github.com/swagger-api/swagger-parser/issues/682

return OpenAPIV3Parser().readContents(content, null, parseOptions)?.openAPI?.let {
val parseResult = OpenAPIV3Parser().readContents(content, null, parseOptions)
if (failOnParseErrors && parseResult.messages.orEmpty().isNotEmpty()) {
val sep = "\n - "
val messageBulletList = parseResult.messages.joinToString(sep)
throw RuntimeException("Swagger parsing failed with those errors:$sep$messageBulletList")
}
return parseResult?.openAPI?.let {
ResolverFully(true).resolveFully(it) // workaround for NPE bug in swagger-parser
Context(it)
}
}

fun createSwaggerContext(content: String): Context? =
SwaggerParser().readWithInfo(content, true)?.let {
val swagger = it.swagger ?: return null
val openApi = SwaggerConverter().convert(it)?.openAPI

openApi?.let {
try {
ResolverFully(true).resolveFully(it)
} catch (e: NullPointerException) {
log.warn("Failed to fully resolve Swagger schema.", e)
}
Context(openApi, swagger)
fun createSwaggerContext(content: String, failOnParseErrors: Boolean = false): Context? =
SwaggerParser().readWithInfo(content, true)?.let { parseResult ->
if (failOnParseErrors && parseResult.messages.orEmpty().isNotEmpty()) {
val sep = "\n - "
val messageBulletList = parseResult.messages.joinToString(sep)
throw RuntimeException("Swagger parsing failed with those errors:$sep$messageBulletList")
}
val swagger = parseResult.swagger ?: return null
val convertResult = SwaggerConverter().convert(parseResult)
if (failOnParseErrors && convertResult.messages.orEmpty().isNotEmpty()) {
val sep = "\n - "
val messageBulletList = parseResult.messages.joinToString(sep)
throw RuntimeException("Swagger conversion to OpenAPI 3 failed with those errors:$sep$messageBulletList")
}
convertResult?.openAPI?.let {
try {
ResolverFully(true).resolveFully(it)
} catch (e: NullPointerException) {
log.warn("Failed to fully resolve Swagger schema.", e)
if (failOnParseErrors) throw e
}
Context(it, swagger)
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,31 +1,32 @@
package de.zalando.zally.rule.zalando

import com.typesafe.config.Config
import de.zalando.zally.rule.Context
import de.zalando.zally.rule.HttpHeadersRule
import de.zalando.zally.rule.api.Check
import de.zalando.zally.rule.api.Rule
import de.zalando.zally.rule.api.Severity
import de.zalando.zally.rule.api.Violation
import de.zalando.zally.rule.api.Rule
import io.swagger.models.Swagger
import de.zalando.zally.util.getAllHeaders
import org.springframework.beans.factory.annotation.Autowired

@Rule(
ruleSet = ZalandoRuleSet::class,
id = "166",
severity = Severity.MUST,
title = "Avoid Link in Header Rule"
ruleSet = ZalandoRuleSet::class,
id = "166",
severity = Severity.MUST,
title = "Avoid Link in Header Rule"
)
class AvoidLinkHeadersRule(@Autowired rulesConfig: Config) : HttpHeadersRule(rulesConfig) {
private val description = "Do Not Use Link Headers with JSON entities"
class AvoidLinkHeadersRule(@Autowired rulesConfig: Config) {

@Check(severity = Severity.MUST)
override fun validate(swagger: Swagger): Violation? {
return super.validate(swagger)
}
private val headersWhitelist = rulesConfig.getStringList(HttpHeadersRule::class.simpleName + ".whitelist").toSet()

override fun isViolation(header: String) = header == "Link"
private val description = "Do Not Use Link Headers with JSON entities"

override fun createViolation(paths: List<String>): Violation {
return Violation(description, paths)
@Check(severity = Severity.MUST)
fun validate(context: Context): List<Violation> {
val allHeaders = context.api.getAllHeaders()
return allHeaders
.filter { it.name !in headersWhitelist && it.name == "Link" }
.map { context.violation(description, it.element) } // createViolation(context, it) }
}
}
37 changes: 37 additions & 0 deletions server/src/main/java/de/zalando/zally/util/OpenApiUtil.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package de.zalando.zally.util

import io.swagger.v3.oas.models.OpenAPI
import io.swagger.v3.oas.models.parameters.Parameter
import io.swagger.v3.oas.models.responses.ApiResponse

data class HeaderElement(
val name: String,
val element: Any
)

fun OpenAPI.getAllHeaders(): Set<HeaderElement> {

fun Collection<Parameter>?.extractHeaders() = orEmpty()
.filter { it.`in` == "header" }
.map { HeaderElement(it.name, it) }
.toSet()

fun Collection<ApiResponse>?.extractHeaders() = orEmpty()
.flatMap { it.headers.orEmpty().entries }
.map { HeaderElement(it.key, it.value) }
.toSet()

val fromParams = components.parameters.orEmpty().values.extractHeaders()

val fromPaths = paths.orEmpty().flatMap { (_, path) ->
val fromPathParameters = path.parameters.extractHeaders()
val fromOperations = path.readOperations().flatMap { operation ->
val fromOpParams = operation.parameters.extractHeaders()
val fromOpResponses = operation.responses.orEmpty().values.extractHeaders()
fromOpParams + fromOpResponses
}
fromPathParameters + fromOperations
}

return fromParams + fromPaths
}
6 changes: 4 additions & 2 deletions server/src/test/java/de/zalando/zally/TestUtil.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ val testConfig: Config by lazy {

fun getFixture(fileName: String): Swagger = SwaggerParser().read("fixtures/$fileName")

fun getContextFromFixture(fileName: String): Context? {
fun getContextFromFixture(fileName: String): Context {
val content = getResourceContent(fileName)
return Context.createOpenApiContext(content) ?: Context.createSwaggerContext(content)
return Context.createOpenApiContext(content)
?: Context.createSwaggerContext(content)
?: throw RuntimeException("Unable to create context.")
}

fun getResourceContent(fileName: String): String = ClasspathHelper.loadFileFromClasspath("fixtures/$fileName")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,31 +1,135 @@
package de.zalando.zally.rule.zalando

import de.zalando.zally.getFixture
import de.zalando.zally.rule.Context
import de.zalando.zally.rule.ZallyAssertions.Companion.assertThat
import de.zalando.zally.testConfig
import org.assertj.core.api.Assertions.assertThat
import org.intellij.lang.annotations.Language
import org.junit.Test

class AvoidLinkHeadersRuleTest {

private val rule = AvoidLinkHeadersRule(testConfig)

@Test
fun positiveCaseSpp() {
val swagger = getFixture("api_spp.json")
assertThat(rule.validate(swagger)).isNull()
fun `a Swagger API with no header called Link produces no violation`() {
@Language("YAML")
val context = Context.createSwaggerContext("""
swagger: 2.0
info:
title: Clean Swagger API
paths:
/foo:
get:
description: Lorem Ipsum
responses:
202:
description: Lorem Ipsum
headers:
Location: # should not violate since not called `Link`
type: string
format: url
parameters:
FlowId: # should not violate since not named `Link`
name: X-Flow-Id
in: header
type: string
Link: # should not violate since not a header
name: Link
in: query
type: string
ProductId: # should not violate since not a header nor named `Link`
name: product_id
in: path
type: string
""".trimIndent(), failOnParseErrors = true)!!
val violations = rule.validate(context)
assertThat(violations).isEmpty()
}

@Test
fun positiveCaseSpa() {
val swagger = getFixture("api_spa.yaml")
assertThat(rule.validate(swagger)).isNull()
fun `an OpenAPI 3 API with no header called Link produces no violation`() {
@Language("YAML")
val context = Context.createOpenApiContext("""
openapi: 3.0.0
info:
title: Clean Swagger API
version: 1.0.0
paths:
/foo:
get:
description: Lorem Ipsum
responses:
202:
description: Lorem Ipsum
headers:
Location:
schema:
type: string
format: url
components:
parameters:
FlowId:
name: X-Flow-Id
in: header
required: false
schema:
type: string
Authorization:
name: Authorization
in: header
required: true
schema:
type: string
ProductId:
name: product_id
in: path
required: true
schema:
type: string
""".trimIndent(), failOnParseErrors = true)!!
val violations = rule.validate(context)
assertThat(violations).isEmpty()
}

@Test
fun negativeCase() {
val swagger = getFixture("avoidLinkHeaderRuleInvalid.json")
val violation = rule.validate(swagger)!!
assertThat(violation.paths).hasSameElementsAs(
listOf("/product-put-requests/{product_path} Link", "/products Link"))
fun `an API with Link headers causes violations`() {
@Language("YAML")
val context = Context.createSwaggerContext("""
swagger: 2.0
info:
title: Clean Swagger API
paths:
/foo:
get:
parameters:
- name: Authorization
in: header
type: string
- name: Link
in: header
type: string
responses:
202:
description: Lorem Ipsum
headers:
Location:
type: string
format: url
post:
responses:
202:
description: Lorem Ipsum
headers:
Link:
type: string
format: url
""".trimIndent(), failOnParseErrors = true)!!
val violations = rule.validate(context)
assertThat(violations)
.descriptionsAllEqualTo("Do Not Use Link Headers with JSON entities")
.pointersEqualTo(
"/paths/~1foo/get/parameters/1",
"/paths/~1foo/post/responses/202/headers/Link"
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class MediaTypesRuleTest {

@Test
fun `the SPP API generates violations`() {
val context = getContextFromFixture("api_spp.json")!!
val context = getContextFromFixture("api_spp.json")
val result = rule.validate(context)
assertThat(result).hasSameElementsAs(listOf(
// --- consumes ---
Expand All @@ -137,7 +137,7 @@ class MediaTypesRuleTest {

@Test
fun `the SPA API generates no violations`() {
val context = getContextFromFixture("api_spa.yaml")!!
val context = getContextFromFixture("api_spa.yaml")
assertThat(rule.validate(context)).isEmpty()
}

Expand Down

0 comments on commit 2702979

Please sign in to comment.