Conversation
| * PUT /path -> 2xx (successful update/create with body B) | ||
| * GET /path -> 2xx (must return exactly the fields that were PUT) | ||
| * | ||
| * Returns true if any field sent in the PUT body has a different value |
There was a problem hiding this comment.
this is partial. you need also to check that GET is not returning fields not used in the PUT. or, if those are returned, if they are null
There was a problem hiding this comment.
also, need to check the differences in what declared in the schema, eg, the resource might have fields that are not supposed to be changed via a PUT (eg timestamps or ids)
| val putBody = extractRequestBody(put) | ||
| val getBody = resGet.getBody() | ||
|
|
||
| if (putBody.isNullOrEmpty() || getBody.isNullOrEmpty()) return false |
There was a problem hiding this comment.
if putBody is empty, but getBody is not, then we got a bug
There was a problem hiding this comment.
I also added this statement: if putBody is not empty, but getBody is. What do you think about that? Is that correct or lead to false positives?
There was a problem hiding this comment.
@omursahin hi, yes, should be fine, i guess shouldn't harm. but, anyway, those cases should still be handled when looking at fields one by one, isn't it?
There was a problem hiding this comment.
For this,
if (putBody.isNullOrEmpty() && !getBody.isNullOrEmpty()) return true
yes, you are right. I am updating it now.
But I guess we don't need to check fields for this statement:
if (!putBody.isNullOrEmpty() && getBody.isNullOrEmpty()) return true
Because if we send any value with put, we expect that it should be returned in the get body, right?
There was a problem hiding this comment.
yep, if send with PUT, we expect in GET.
|
|
||
| if (putBody.isNullOrEmpty() || getBody.isNullOrEmpty()) return false | ||
|
|
||
| val fieldNames = extractModifiedFieldNames(put) |
There was a problem hiding this comment.
here it should rather check the field names of what declared in the schema of the PUT. i have added some more discussion and examples to 2026/httporacles/notes.txt
|
|
||
| if (!StatusGroup.G_2xx.isInGroup(resPut.getStatusCode())) return false | ||
| // if put returned 2xx but entity does not exist afterwards | ||
| if (resGet.getStatusCode() == 404) return true |
There was a problem hiding this comment.
that's a good point... can you update 2026/httporacles/notes.txt mentioning it? it will be helpful in few weeks/months when we are going to use those notes as starting point for a paper :)
| if (!StatusGroup.G_2xx.isInGroup(resGet.getStatusCode())) return false | ||
|
|
||
| val bodyParam = put.parameters.find { it is BodyParam } as BodyParam? | ||
| if (bodyParam != null && !bodyParam.isJson() && !bodyParam.isXml() && !bodyParam.isForm()) { |
There was a problem hiding this comment.
add a comment, specifying that for now we only deal with JSON/XML/FORM. also, clarify what would happen when bodyParam is null
|
|
||
| // PUT sent content but GET body is empty -> sent fields definitely missing | ||
| if (!putBody.isNullOrEmpty() && getBody.isNullOrEmpty()) return true | ||
| if (getBody.isNullOrEmpty()) return false |
There was a problem hiding this comment.
refactor for clarity, eg, something like:
if (getBody.isNullOrEmpty()) {
return !putBody.isNullOrEmpty()
}
| private fun isWipedFieldStillPresent(value: String?): Boolean { | ||
| if (value == null) return false | ||
| if (value.isEmpty()) return false | ||
| if (value == "null") return false |
There was a problem hiding this comment.
i m a bit unsure about this "null" check. are you sure that you can properly distinguish between:
{"x": null} (empty value)
and
{"x": "null"} (value is present, ie., a string with 4 characters)
make sure to have at least 1 test case to check this
| return objectGene.fields.map { it.name }.toSet() | ||
| } | ||
|
|
||
| // Extract only the field names that were actually sent in the request body. |
There was a problem hiding this comment.
for method comments, remember to use JavaDoc styel /** */ and not //
|
|
||
| val gene = bodyParam.primaryGene() | ||
| val objectGene = gene.getWrappedGene(ObjectGene::class.java) as ObjectGene? | ||
| return gene.getWrappedGene(ObjectGene::class.java) as ObjectGene? |
There was a problem hiding this comment.
keep in mind that a body could be optional (ie, not marked as required), and so inside an OptionalGene. if this latter is off (ie, not active), then nothing is sent. should check this
| * Returns the property names from the PUT request body schema in the OpenAPI spec. | ||
| * Used as a fallback to determine writable fields when no BodyParam is present on the action. | ||
| */ | ||
| internal fun extractPutRequestSchemaFields( |
There was a problem hiding this comment.
this feels like a general function... could be extracted in a utility class, as could be re-used in other places as well
| SchemaUtils.getReferenceSchema(schema, schema.main, it, mutableListOf()) | ||
| } ?: rawSchema | ||
|
|
||
| return resolved.properties?.keys?.toSet() ?: emptySet() |
There was a problem hiding this comment.
schema analysis is very TRICKY... as you could have $ref at each point in the tree... and those override any sibling node :( need more checks and unit tests for this
RestCallAction is created based on content of schema, but taint analysis could had been involved... so still make sense to look at original schema for these checks
| */ | ||
| internal fun extractGetResponseSchemaFields( | ||
| schema: RestSchema, | ||
| get: RestCallAction |
There was a problem hiding this comment.
as the other function, could be refactored into SchemaUtils
| val op = pathItem.get ?: return emptySet() | ||
|
|
||
| // pick the first 2xx response, falling back to "default" | ||
| val response = op.responses?.entries |
There was a problem hiding this comment.
as for other function, need to check for the presence of $ref at each point in the tree where it can be present
2 participants
No description provided.