-
Notifications
You must be signed in to change notification settings - Fork 103
side effects of failed modification #1452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| package com.foo.rest.examples.spring.openapi.v3.httporacle.failmodification | ||
|
|
||
| import org.springframework.boot.SpringApplication | ||
| import org.springframework.boot.autoconfigure.SpringBootApplication | ||
| import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration | ||
| import org.springframework.http.ResponseEntity | ||
| import org.springframework.web.bind.annotation.* | ||
|
|
||
|
|
||
| @SpringBootApplication(exclude = [SecurityAutoConfiguration::class]) | ||
| @RequestMapping(path = ["/api/resources"]) | ||
| @RestController | ||
| open class FailModificationApplication { | ||
|
|
||
| companion object { | ||
| @JvmStatic | ||
| fun main(args: Array<String>) { | ||
| SpringApplication.run(FailModificationApplication::class.java, *args) | ||
| } | ||
|
|
||
| private val data = mutableMapOf<Int, ResourceData>() | ||
|
|
||
| fun reset(){ | ||
| data.clear() | ||
| } | ||
| } | ||
|
|
||
| data class ResourceData( | ||
| var name: String, | ||
| var value: Int | ||
| ) | ||
|
|
||
| data class UpdateRequest( | ||
| val name: String?, | ||
| val value: Int? | ||
| ) | ||
|
|
||
|
|
||
| @PostMapping | ||
| open fun create(@RequestBody body: ResourceData): ResponseEntity<ResourceData> { | ||
| val id = data.size + 1 | ||
| data[id] = body.copy() | ||
| return ResponseEntity.status(201).body(data[id]) | ||
| } | ||
|
|
||
| @GetMapping(path = ["/{id}"]) | ||
| open fun get(@PathVariable("id") id: Int): ResponseEntity<ResourceData> { | ||
| val resource = data[id] | ||
| ?: return ResponseEntity.status(404).build() | ||
| return ResponseEntity.status(200).body(resource) | ||
| } | ||
|
|
||
| @PutMapping(path = ["/{id}"]) | ||
| open fun put( | ||
| @PathVariable("id") id: Int, | ||
| @RequestBody body: UpdateRequest | ||
| ): ResponseEntity<Any> { | ||
|
|
||
| val resource = data[id] | ||
| ?: return ResponseEntity.status(404).build() | ||
|
|
||
| // bug: modifies data even though it will return 4xx | ||
| if(body.name != null) { | ||
| resource.name = body.name | ||
| } | ||
| if(body.value != null) { | ||
| resource.value = body.value | ||
| } | ||
|
|
||
| // returns 400 Bad Request, but the data was already modified above | ||
| return ResponseEntity.status(400).body("Invalid request") | ||
| } | ||
|
|
||
| @PatchMapping(path = ["/{id}"]) | ||
| open fun patch( | ||
| @PathVariable("id") id: Int, | ||
| @RequestBody body: UpdateRequest | ||
| ): ResponseEntity<Any> { | ||
|
|
||
| val resource = data[id] | ||
| ?: return ResponseEntity.status(404).build() | ||
|
|
||
| // correct: validation first, reject without modifying | ||
| if(body.name == null && body.value == null) { | ||
| return ResponseEntity.status(400).body("No fields to update") | ||
| } | ||
|
|
||
| // correct: does NOT modify data, just returns 4xx | ||
| return ResponseEntity.status(403).body("Forbidden") | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package com.foo.rest.examples.spring.openapi.v3.httporacle.failmodification | ||
|
|
||
| import com.foo.rest.examples.spring.openapi.v3.SpringController | ||
|
|
||
|
|
||
| class FailModificationController: SpringController(FailModificationApplication::class.java){ | ||
|
|
||
| override fun resetStateOfSUT() { | ||
| FailModificationApplication.reset() | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| package org.evomaster.e2etests.spring.openapi.v3.httporacle.failmodification | ||
|
|
||
| import com.foo.rest.examples.spring.openapi.v3.httporacle.failmodification.FailModificationController | ||
| import org.evomaster.core.problem.enterprise.DetectedFaultUtils | ||
| import org.evomaster.core.problem.enterprise.ExperimentalFaultCategory | ||
| import org.evomaster.core.problem.rest.data.HttpVerb | ||
| import org.evomaster.e2etests.spring.openapi.v3.SpringTestBase | ||
| import org.junit.jupiter.api.Assertions.assertEquals | ||
| import org.junit.jupiter.api.Assertions.assertTrue | ||
| import org.junit.jupiter.api.BeforeAll | ||
| import org.junit.jupiter.api.Test | ||
|
|
||
| class FailModificationEMTest : SpringTestBase(){ | ||
|
|
||
| companion object { | ||
| @BeforeAll | ||
| @JvmStatic | ||
| fun init() { | ||
| initClass(FailModificationController()) | ||
| } | ||
| } | ||
|
|
||
|
|
||
| @Test | ||
| fun testRunEM() { | ||
|
|
||
| runTestHandlingFlakyAndCompilation( | ||
| "FailedModificationEM", | ||
| 500 | ||
| ) { args: MutableList<String> -> | ||
|
|
||
| setOption(args, "security", "false") | ||
| setOption(args, "schemaOracles", "false") | ||
| setOption(args, "httpOracles", "true") | ||
| setOption(args, "useExperimentalOracles", "true") | ||
|
|
||
| val solution = initAndRun(args) | ||
|
|
||
| assertTrue(solution.individuals.size >= 1) | ||
|
|
||
| val faults = DetectedFaultUtils.getDetectedFaultCategories(solution) | ||
| assertEquals(1, faults.size) | ||
| assertEquals(ExperimentalFaultCategory.HTTP_SIDE_EFFECTS_FAILED_MODIFICATION, faults.first()) | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,14 @@ | ||
| package org.evomaster.core.problem.rest.oracle | ||
|
|
||
| import com.google.gson.JsonParser | ||
| import org.evomaster.core.problem.rest.data.HttpVerb | ||
| import org.evomaster.core.problem.rest.data.RestCallAction | ||
| import org.evomaster.core.problem.rest.data.RestCallResult | ||
| import org.evomaster.core.problem.rest.data.RestIndividual | ||
| import org.evomaster.core.problem.rest.param.BodyParam | ||
| import org.evomaster.core.problem.rest.StatusGroup | ||
| import org.evomaster.core.search.action.ActionResult | ||
| import org.evomaster.core.search.gene.ObjectGene | ||
|
|
||
| object HttpSemanticsOracle { | ||
|
|
||
|
|
@@ -103,4 +107,141 @@ object HttpSemanticsOracle { | |
|
|
||
| return NonWorkingDeleteResult(checkingDelete, nonWorking, delete.getName(), actions.size - 2) | ||
| } | ||
| } | ||
|
|
||
| fun hasSideEffectFailedModification(individual: RestIndividual, | ||
| actionResults: List<ActionResult> | ||
| ): Boolean{ | ||
|
|
||
| if(individual.size() < 3){ | ||
| return false | ||
| } | ||
|
|
||
| val actions = individual.seeMainExecutableActions() | ||
|
|
||
| val before = actions[actions.size - 3] // GET (before state) | ||
| val modify = actions[actions.size - 2] // PUT or PATCH (failed modification) | ||
| val after = actions[actions.size - 1] // GET (after state) | ||
|
|
||
| // check verbs: GET, PUT|PATCH, GET | ||
| if(before.verb != HttpVerb.GET) { | ||
| return false | ||
| } | ||
| if(modify.verb != HttpVerb.PUT && modify.verb != HttpVerb.PATCH) { | ||
| return false | ||
| } | ||
| if(after.verb != HttpVerb.GET) { | ||
| return false | ||
| } | ||
|
|
||
| // all three must be on the same resolved path | ||
| if(!before.usingSameResolvedPath(modify) || !after.usingSameResolvedPath(modify)) { | ||
| return false | ||
| } | ||
|
|
||
| // auth should be consistent | ||
| if(before.auth.isDifferentFrom(modify.auth) || after.auth.isDifferentFrom(modify.auth)) { | ||
| return false | ||
| } | ||
|
|
||
| val resBefore = actionResults.find { it.sourceLocalId == before.getLocalId() } as RestCallResult? | ||
| ?: return false | ||
| val resModify = actionResults.find { it.sourceLocalId == modify.getLocalId() } as RestCallResult? | ||
| ?: return false | ||
| val resAfter = actionResults.find { it.sourceLocalId == after.getLocalId() } as RestCallResult? | ||
| ?: return false | ||
|
|
||
| // before GET must be 2xx | ||
| if(!StatusGroup.G_2xx.isInGroup(resBefore.getStatusCode())) { | ||
| return false | ||
| } | ||
|
|
||
| // PUT/PATCH must have failed with 4xx | ||
| if(!StatusGroup.G_4xx.isInGroup(resModify.getStatusCode())) { | ||
| return false | ||
| } | ||
|
|
||
| // after GET must be 2xx | ||
| if(!StatusGroup.G_2xx.isInGroup(resAfter.getStatusCode())) { | ||
| return false | ||
| } | ||
|
|
||
| val bodyBefore = resBefore.getBody() | ||
| val bodyAfter = resAfter.getBody() | ||
|
|
||
| // if both are null/empty, no side-effect detected | ||
| if(bodyBefore.isNullOrEmpty() && bodyAfter.isNullOrEmpty()) { | ||
| return false | ||
| } | ||
|
|
||
| // extract the field names sent in the PUT/PATCH request body | ||
| val modifiedFieldNames = extractModifiedFieldNames(modify) | ||
|
|
||
| // if we can identify specific fields, compare only those to avoid false positives from timestamps etc. | ||
| if(modifiedFieldNames.isNotEmpty() | ||
| && !bodyBefore.isNullOrEmpty() | ||
| && !bodyAfter.isNullOrEmpty()) { | ||
| return hasChangedModifiedFields(bodyBefore, bodyAfter, modifiedFieldNames) | ||
| } | ||
|
|
||
| // otherwise compare entire bodies | ||
| return bodyBefore != bodyAfter | ||
| } | ||
|
|
||
| /** | ||
| * Extract field names from the PUT/PATCH request body. | ||
| * These are the fields that the client attempted to modify. | ||
| */ | ||
| private fun extractModifiedFieldNames(modify: RestCallAction): Set<String> { | ||
|
|
||
| val bodyParam = modify.parameters.find { it is BodyParam } as BodyParam? | ||
| ?: return emptySet() | ||
|
|
||
| val gene = bodyParam.primaryGene() | ||
| val objectGene = gene.getWrappedGene(ObjectGene::class.java) as ObjectGene? | ||
| ?: if (gene is ObjectGene) gene else null | ||
|
|
||
| if(objectGene == null){ | ||
| return emptySet() | ||
| } | ||
|
|
||
| return objectGene.fields.map { it.name }.toSet() | ||
| } | ||
|
|
||
| /** | ||
| * Compare only the fields that were sent in the PUT/PATCH request. | ||
| * Returns true if any of those fields changed between before and after GET responses. | ||
| */ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add comment stating this only works where the payload is a JSON object matching the resource structure. it will not work for cases like JSON Patch RFC6902 |
||
| internal fun hasChangedModifiedFields( | ||
| bodyBefore: String, | ||
| bodyAfter: String, | ||
| fieldNames: Set<String> | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to avoid flakiness, i think we should pass as well the |
||
| ): Boolean { | ||
|
|
||
| try { | ||
| val jsonBefore = JsonParser.parseString(bodyBefore) | ||
| val jsonAfter = JsonParser.parseString(bodyAfter) | ||
|
|
||
| if(!jsonBefore.isJsonObject || !jsonAfter.isJsonObject){ | ||
| // not JSON objects, fallback to full comparison | ||
| return bodyBefore != bodyAfter | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. return |
||
| } | ||
|
|
||
| val objBefore = jsonBefore.asJsonObject | ||
| val objAfter = jsonAfter.asJsonObject | ||
|
|
||
| for(field in fieldNames){ | ||
| val valueBefore = objBefore.get(field) | ||
| val valueAfter = objAfter.get(field) | ||
|
|
||
| if(valueBefore != valueAfter){ | ||
| return true | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. besides being different from |
||
| } | ||
| } | ||
|
|
||
| return false | ||
| } catch (e: Exception) { | ||
| // JSON parsing failed, fallback to full comparison | ||
| return bodyBefore != bodyAfter | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. return |
||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we should return a
falsehere, as otherwise too prone to false positives due to non-deterministic fields