Skip to content
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

Behavior of allowEmptyValue: false #3792

Open
handrews opened this issue May 8, 2024 · 10 comments
Open

Behavior of allowEmptyValue: false #3792

handrews opened this issue May 8, 2024 · 10 comments
Assignees
Labels
param serialization Issues related to parameter and/or header serialization
Milestone

Comments

@handrews
Copy link
Contributor

handrews commented May 8, 2024

TL;DR: I have no idea how to define compliance criteria for allowEmptyValue: false, which is the default behavior, with setting true being NOT RECOMMENDED.

<rant>
The clarification added in 3.0.2 just says using it is NOT RECOMMENDED, which would, AFAICT, leave the default false behavior in place. Looking at the issue regarding that clarification, there is a lot there that never made it into the spec. Some of that says that setting it to true allows the undefined value behavior already defined by RFC6570.

However, style, explode, and allowReseved already control the RFC6570 behavior. Are we by default forbidding empty values? Does that mean RFC6570's undefined values (which includes null, empty arrays, empty objects, and objects where all properties are null, but NOT the empty string)? Does it impact the empty string as well? If schema allows these does the default allowEmptyValue: false behavior contradict that? How are implementations expected to support this?

If using content, does allowEmptyValue have meaning? What if the content is legitimately a zero-length string?

This seems like a field with a default value that blocks common cases in a very non-obvious way, requiring complex code to work around the normal behavior of Schema Objects, Media Type Objects, and RFC6570. Do we really expect implementations to enforce this? If we are recommending against its use, how do we expect users to support empty values, which can easily happen by accident (particularly empty arrays)?
</rant>

@handrews handrews added review param serialization Issues related to parameter and/or header serialization labels May 8, 2024
@handrews handrews added this to the v3.0.4 milestone May 8, 2024
@handrews handrews self-assigned this May 8, 2024
@lornajane
Copy link
Contributor

We discussed in TSC and decided that since the use isn't recommended, we might try to continue as if this field does not exist.

@handrews
Copy link
Contributor Author

After thinking on this more, the only interpretation I can come up with that makes sense to me is:

  • allowEmptyValue: true was intended to allow empty values regardless of whether the schema allows them or not.
  • this would make allowEmptyValue: false (the default) be a no-op: if the schema allows an empty value, then it is still allowed

It just doesn't make sense to me that the default would contradict how schemas work everywhere else. So it must be that the non-default value is the one that contradicts the schema. This would also parallel how nullable works by contradicting the established type.

In the absence of anyone knowing how this was supposed to work, that's what I'm going to explain in the text.

@ralfhandl
Copy link
Contributor

ralfhandl commented May 15, 2024

I think that allowEmptyValue is a hint for Swagger UI and similar tools on whether to allow users sending query options with an empty value instead of not sending the query option at all:

image

Visualization & try-out tools REALLY SHOULD render such a check box if (and only if) the parameter's schema accepts the empty string.

And the OAS editors REALLY SHOULD remove this thingy in future versions of the specification 😄

@handrews
Copy link
Contributor Author

@ralfhandl The UI hint makes a little more sense, but would require different wording to clarify that intent.

But I'm not convinced of this interpretation either. What difference does it make to an API whether bar= (with an empty string value) is sent because of a web form or because of a programmatically constructed URL query string? Why would bar be omitted?

Is this about HTML's notion of what controls are successful, which defers whether to send an input that "lacks a value" or not to the implementation? (referencing HTML 4.0.1 because I can't be bothered to decipher WHATWG's pseudocode).

And again, how does that impact APIs? Are APIs expected to mimic web forms, in which case there's no difference, or is this a web-form-only thing, in which case why is it in the spec at all when many other web-form-related things are not?

If this is a UI hint, then I'll need to update the PR to say so.

@ralfhandl
Copy link
Contributor

What difference does it make to an API whether bar= (with an empty string value) is sent

The question is how many different meanings an API sees in these three requests:

GET /foo
GET /foo?bar
GET /foo?bar=

Do absence (1) and presence (2, 3) of bar mean different things?

Do presence of bar without a value (2) and with a zero-length value (3) mean different things?

If an API author intends to have three different meanings, how do they express their intent in OpenAPI?

With required:false the author can express that form (1) is acceptable.

Then things get murky.

OpenAPI 2.0 said

allowEmptyValue | boolean | Sets the ability to pass empty-valued parameters. This is valid only for either query or formData parameters and allows you to send a parameter with a name only(2) or an empty value(3). Default value is false.

So authors could express that forms (2) and (3) are allowed, but it was unclear how to distinguish (2) and (3).

Explicit mentioning of form (2) was taken away in OpenAPI 3.0.0, see here for the TSC's rationale.

Which leaves me puzzled why the keyword remained, instead of removing it and relying on style or content for allowing form (3).

My take:

  • Don't try to fix allowEmptyValue in 3.x, it is broken beyond repair.
  • Remove it in 4.0.

@miqui
Copy link
Contributor

miqui commented May 16, 2024

My take:

Don't try to fix allowEmptyValue in 3.x, it is broken beyond repair.
Remove it in 4.0.

I agree

@handrews
Copy link
Contributor Author

@ralfhandl no one's trying to "fix" this right now, I'm just trying to document what it does.

We're definitely all puzzled, and it was already agreed that it should be removed. But it is still here for now, and as written it appears to require contradicting schema and content. So is that what it does? Or is that not what it does? And if not, what does it do? Not "what should it do?", not "what do we want it to do", but "what is it doing right now that we don't understand?"

@ralfhandl
Copy link
Contributor

what it does

I'm afraid it does different things with/to different tools, and I definitely am unable to decide which of these is "right".

@lornajane
Copy link
Contributor

I assume we'll keep this open until we've forward-ported to 3.1.x and 3.2.x

@lornajane lornajane removed the review label May 21, 2024
@handrews
Copy link
Contributor Author

I assume we'll keep this open until we've forward-ported to 3.1.x and 3.2.x

@lornajane yes that's how I've been tracking/closing issues with PRs for multiple branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
param serialization Issues related to parameter and/or header serialization
Projects
None yet
Development

No branches or pull requests

4 participants