Conversation
|
addressing issue #23 |
|
Hi @grebnetiew , can you review these changes in the auth schema? And then "approve" if you agree, or suggest changes :) Even if there are no comments, ideally I would prefer not merging changes to the schema unless there is an "explicit" approval from at least one other fuzzer's maintainer thx! |
grebnetiew
left a comment
There was a problem hiding this comment.
LGTM. I made some suggestions for cleanup and enhancement.
| description: "The value of the header" | ||
| type: string | ||
| required: ["name","value"] | ||
| x-required: ["name","value"] |
There was a problem hiding this comment.
Is there a case in which only the name or only the value of a header occurs in a valid spec? I would guess they're very hard to merge if you don't know both values.
There was a problem hiding this comment.
AFAIK, in HTTP the name must be there. but, in theory the value could be empty. however, that would be equivalent to an empty string.
shall we remove the "value" from that x-required? I am unsure
There was a problem hiding this comment.
I think requiring the value is fine, people who absolutely must have an empty header can use "" as you say. I meant to say that for the headers, we could just use the regular old required without the x-.
There was a problem hiding this comment.
ah, i see. you are right. would need both anyway, as a merge on an array would not be well-defined otherwise. good point. i ll fix
| an auth token from the response payload." | ||
| type: boolean | ||
| required: ["verb"] | ||
| x-required: ["verb"] |
There was a problem hiding this comment.
Now that we aren't constrained to the type of required (array of strings), should we also express the requirement that there must be either endpoint or externalEndpointURL?
There was a problem hiding this comment.
expressing such requirement could be formally done using a oneOf constraint. however, we would end up with same issue that JSON Schema would validate the constraint before the merge :(
or there you think of something like a custom constrain declaration, like x-only-one-of['endpoint','externalEndpointURL']
There was a problem hiding this comment.
Perhaps we can say that x-required must be an array of string | object, and express it as
x-required:
- "verb"
- oneOf: ["endpoint","externalEndpointURL"](feel free to refuse if this is the way to madness, because whether we specify this or not, a fuzzer will error out if there is no URL to authenticate with)
There was a problem hiding this comment.
what about having:
x-required:
- allOf: ["verb"]
- oneOf: ["endpoint","externalEndpointURL"]There was a problem hiding this comment.
ie, x-required would be either an array of strings (with same semantic of required) or an object (with fields such as allOf and oneOf to express more fine-grained constraints)
Fixed JSON Schema compliance by replacing
requiredwithx-requiredwhere needed. Added some documentation to explain the issue.