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

Clarify allowEmptyValue #1573

Closed
ehmicky opened this issue May 7, 2018 · 20 comments
Closed

Clarify allowEmptyValue #1573

ehmicky opened this issue May 7, 2018 · 20 comments
Labels

Comments

@ehmicky
Copy link

ehmicky commented May 7, 2018

From the specification it is a little ambiguous what the allowEmptyValue entails, notably what empty means:

  • is a string containing only whitespaces considered empty?
  • does it only affect a query parameter when its value is of type: string? Or would 0, false, [] or {} qualify as empty when a query parameter is of another type?
@mikunn
Copy link

mikunn commented May 9, 2018

This is a good point. I'm also interested what should happen if allowEmptyValue is set to true and minLength is, let's say 10, in the schema?

Does this say "it can be 'empty', but if it's not, it must be at least 10 characters long"?

@tedepstein
Copy link
Contributor

Just reading this section of the spec, I have to say I'm not 100% certain of the intent, but here's what I think is a likely interpretation:

I think allowEmptyValues determines whether a query parameter can be present in the query string without a value. For example, http://api.foo.com/orgs?includeLLCs as opposed to http://api.foo.com/orgs?includeLLCs=true.

In the first form, includeLLCs is an "empty-valued parameter" because it doesn't specify a value. Its presence or absence in the query string conveys the intended meaning; so I guess you could think of it as a boolean value: included (true) or omitted (false).

Unless the includeLLCs parameter specifies allowEmptyValues : true, a request with includeLLCs as an empty-valued parameter would be invalid, and the server should return an appropriate error response. In that sense, the OpenAPI spec seems to prescribe a certain level of expected validation.

But OpenAPI doesn't specify what kinds of values should be serialized as an empty-valued query parameter, nor what representation an empty-valued parameter should be deserialized to. So this is application-defined.

I suspect this is because different type systems, in different programming languages, can have many different value representations that could be interpreted as "empty." The serialization style examples include a JSON string, an object, and an array. But I don't think these are intended to specify a literal mapping of JSON values to serialized parameters. A mapping like that wouldn't be very useful in the usual case, where parameter values are directly read, written, or mapped to an in-memory representation, entirely dependent on the programming language. JSON isn't part of the picture here, so a direct JSON mapping isn't meaningful, in the concrete sense.

So I interpret those JSON values in the style examples as generic placeholders for what we commonly think of as strings, objects, and arrays, as common constructs used in many programming languages. It might be helpful to add an "empty" example there too. But in that case, if I understand the intent correctly, the spec needs to be clear that it is not prescribing what should constitute an empty value.

A given API might expect certain query parameters to always be empty-valued. It might be nice if OpenAPI allowed us to specify this explicitly, but it doesn't. That expectation would have to be captured in a parameter description (or elsewhere in the API documentation), and enforced by custom validation as needed.

Another API might allow certain query parameters to have empty or non-empty values. In the case of an empty (omitted) parameter value, the schema property would be ignored, because there is no value to be validated against that schema. In the case of a non-empty (included) parameter value, the schema would apply.

So I can attempt an answer to the specific questions here:

is a string containing only whitespaces considered empty?
does it only affect a query parameter when its value is of type: string? Or would 0, false, [] or {} qualify as empty when a query parameter is of another type?

IIUC, this is application-defined.

I'm also interested what should happen if allowEmptyValue is set to true and minLength is, let's say 10, in the schema? Does this say "it can be 'empty', but if it's not, it must be at least 10 characters long"?

Yes. If the parameter appears without a value, then schema constraints are ignored. If it has a value, the value must conform to the schema.

It would be great if one of the authors of this part of the spec could please confirm or correct my interpretation here...

@ehmicky
Copy link
Author

ehmicky commented May 9, 2018

This seems to make lots of sense to me.

I.e. in a nutshell:

  • allowEmptyValue defines whether a query parameter exampleParam can be written as ?exampleParam as opposed to ?exampleParam= or ?exampleParam=value
  • when allowEmptyValue is true and ?exampleParam is used, the parameter schema is ignored.

If this is the intent, should it be explicitly stated in the specification?

A follow-up question is: can a required parameter allow empty values?

@webron
Copy link
Member

webron commented May 9, 2018

So here's the clarification, and we can add more information in the spec if needed.

allowEmptyValue means a zero-length value can be provided. This translates to ?exampleParam= and not ?exampleParam. There is no way to describe ?exampleParam, and from what I recall in our discussions on this topic, many @OAI/tsc members claimed that such a thing doesn't really exist.

Combining allowEmptyValue with minLength=10 is meaningless as given you are defining a minimum length, 0 isn't it. Only the minLength will actually take effect here.

As for the follow-up question: yes, a required parameter can allow empty values, there's no contradiction there.

@tedepstein
Copy link
Contributor

@webron , your answer seems to conflict with what's shown in the style examples table. There, we can see that an empty-valued parameter with style: matrix is serialized as ;color.

@webron
Copy link
Member

webron commented May 9, 2018

You're right, only the conflict is worse. If you read the description of allowEmptyValue, it can only be applied to query parameters, and matrix is only applicable to path parameters - so there's something here that definitely needs to be fixed.

@tedepstein
Copy link
Contributor

tedepstein commented May 10, 2018

I did some research into this, and here's the history, as far as I can piece it together:

  • March 26 2015
    • The allowEmptyValue field was introduced into the OpenAPI 2.0 spec in PR 2.0 fixes #315 - "2.0 fixes." There's no link to an issue in the PR, nor in the first and second commits that specifically address this field.
    • There's a sprawling issue add "null value support" in spec? #229 that might have been the primordial soup from which allowEmptyValue emerged. But it seems to cover nullable values, message bodies, and all kinds of things that have nothing to do with empty-valued query parameters. There's also issue Swagger UI - Blank parameter input results in paramter being ommited from URL #81 that seems related. So it's hard to tell exactly where the requirement came from.
    • Original description was: "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 or an empty value. Default value is false."
    • Similar description with examples here on the Swagger.io site. Not part of the official spec, and I don't know the date or author of that content. But it seems to confirm that the original intent was to allow name-only parameters.
  • March 27, 2016
    • New issue Redefine allowEmptyValue #611, suggesting that the spec isn't clear enough; it needs to specify whether the empty-valued parameter should be ?foo&bar or ?foo=&bar=. This isn't addressed until later, in the 3.0 spec...
    • Swagger-UI#1396, from June 2015, seems to be the one that brought this question into focus.
  • February 22, 2017
    • PR Updates to parameter and requestBody #884 changes the spec for 3.0 to say that allowEmptyValue is only meaningful with serialization style values that support it. The styles with n/a in the empty column don't support empty values, and allowEmptyValue is ignored when these styles are used.
    • Issue Redefine allowEmptyValue #611, mentioned earlier, includes a comment saying that this was the way of clarifying whether an empty value takes the form of param= or param. But the clarification is not given explicitly, and it doesn't say how it was determined that param= is the correct, intended form.
  • June 14, 2017
  • May 7, 2018

@tedepstein
Copy link
Contributor

@webron, in light of the above history:

I guess my interpretation was not far off from the original intent. It does seem that when allowEmptyValue was first introduced in Swagger and OpenAPI 2.0, it was intended to allow name-only, flag-style parameters. It seems that it also may have been intended to allow empty values, and it left some ambiguity as to whether these two things should be considered equivalent. It did not distinguish them as separate forms.

There is no way to describe ?exampleParam, and from what I recall in our discussions on this topic, many @OAI/tsc members claimed that such a thing doesn't really exist.

If you can point us to the TSC conversation, it would help to see that. I honestly don't understand the claim that name-only parameters don't exist, when clearly they do exist in the wild. We can ask whether it's good practice. And we can ask what's the best way to model them. But "don't exist" isn't making much sense to me.

I don't have a strong opinion that name-only parameters are good practice. But there is one significant argument for allowing them: It means that OpenAPI can be used to describe existing APIs that use these parameters.

allowEmptyValue means a zero-length value can be provided.

Does that specifically mean a zero-length string, as opposed to an empty array or empty object? And in that case, is OpenAPI saying that conforming implementations SHOULD or MUST interpret an empty parameter, in the specified syntax (from the 'empty' column in the style table) as an empty string?

Combining allowEmptyValue with minLength=10 is meaningless as given you are defining a minimum length, 0 isn't it. Only the minLength will actually take effect here.

In that case:

  • The spec should say this explicitly.
  • allowEmptyValue: true should be ignored, or (better, IMO) disallowed if the schema type is anything other than string.
  • allowEmptyValue: true might be the equivalent of type: string + minLength: 0, which AFAIK is the same as type: string with no minLength specified.
  • minLength: 0 + maxlength: 0 would specify that a parameter can only have an empty value.
  • So maybe allowEmptyValue is obsolete now that OpenAPI 3.0 has full schema support, and it should be deprecated.

This seems like it needs to be given some more thought...

@ehmicky
Copy link
Author

ehmicky commented May 10, 2018

allowEmptyValue: true should be ignored, or (better, IMO) disallowed if the schema type is anything other than string.
allowEmptyValue: true might be the equivalent of type: string + minLength: 0, which AFAIK is the same as type: string with no minLength specified.

Those two statements lead to: allowEmptyValue: true does not do anything.

However allowEmptyValue: false would be the same as minLength: 1 providing type is string.

@tedepstein
Copy link
Contributor

tedepstein commented May 10, 2018

I still think there's fundamental confusion about the original intent of allowEmptyValue, the motivation for revising the definition in 3.0, and the actual effect of that revision. I'm not at all sure that empty string is the correct interpretation of this.

Here's how I see it:

  • I believe the original spec regarded "name only" and "empty value" as the same thing.

    The original 2.0 spec was:

    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 or an empty value. Default value is false.

    (Emphasis added.)

    It doesn't sound like "name only" and "empty value" are considered to be separate use cases, with different syntax or different semantics. It sounds like they are considered to be the same thing.

    And it doesn't say what "empty value" means: null value, empty string, empty object or array, or unassigned value. Of these possible interpretations, it seems that "unassigned value" is probably the most likely interpretation, and this is practically synonymous with "name only." It is not synonymous with zero-length string, empty object or array.

    The Swagger documentation here also shows name-only parameters. It makes no mention of "empty value," and it doesn't show an example of the trailing-equal-sign param= syntax.

  • The perceived need to correct the spec seems to be based on questionable assumptions.

    Issue Redefine allowEmptyValue #611, which has only very terse comments and no discussion, takes the position that "no value (acting as a flag)" and "empty value" must necessarily be separate things, again without explaining what "empty value" means.

    It also assumes that the two syntax variants, ?foo&bar (name-only) and ?foo=&bar= (trailing-equal-sign) must also be semantically different. And it says that the name-only syntax means "no value", while the trailing-equal-sign syntax means "empty value."

    It doesn't seem to consider the possibility that "no value" and "empty value" are just two phrases that mean the same thing, and that these two syntax variants are two equivalent, interchangeable ways of saying the same thing.

    I'm really questioning this, because I don't know what "empty value" means, and because the trailing-equal-sign syntax ?param= seems like an anomaly. Not to say that this syntax should be disallowed, but I've never seen a programming language that would treat it as anything but a syntax error. The only intuitive meaning is "this parameter has no value", which again seems indistinguishable from the name-only case.

  • The changes to the language in OpenAPI 3.0 don't actually redefine allowEmptyValue.

    The new language is:

    Sets the ability to pass empty-valued parameters. This is valid only for query parameters and allows sending a parameter with an empty value. Default value is false. If style is used, and if behavior is n/a (cannot be serialized), the value of allowEmptyValue SHALL be ignored.

    If the intention was to redefine the meaning of allowEmptyParameters, as described in Redefine allowEmptyValue #611, it's not at all clear that the changes actually have this effect.

    If we all agreed and understood what "empty-value parameters" means, that it's distinct from no-value parameters, and that it is represented only using trailing-equal-sign syntax, not name-only syntax, then this change might be sufficient. But clearly, given the questions and doubts raised in this issue, these terms haven't been well defined.

    Besides, this would have to be considered a breaking change, since it disallows something that was allowed under 2.0. Breaking changes were given special scrutiny and public discussion during the 3.0 draft phases. Special attention was given to the language of the spec to make sure that any changes were clearly called out, or at least clearly visible, to reviewers.

    So whatever the intent, I don't think the changes in PRs Updates to parameter and requestBody #884 and edit Parameter Locations and its Fixed Fields #1168 actually had the effect of redefining allowEmptyValue.

  • Restricting this now to empty-value with trailing-equal-sign syntax is a breaking change, not a clarification.

    I think it's really important that we stand by what the spec actually says.

    Where the meaning might be ambiguous, we need to consider the history of what it meant before, whether there was a clear and well-founded decision to change it, and whether the change was actually implemented, i.e. properly written into the spec, in a way that makes the change clear. These things should matter more than a statement of what we intended the spec to mean.

    Trying to assert a narrow interpretation of the spec might seem like the conservative, safe thing to do. But it actually can cause considerable pain and confusion, especially if the narrow interpretation turns out to be incorrect. This is especially true for those of us who have to build and maintain working implementations! But it's also true for people writing OpenAPI documents.

    Example: Let's say I have an API that uses name-only syntax, and an OpenAPI 2.0 or 3.0 document that describes it, according to the spec. If the spec is then "clarified" (amended, by means of a comment in a discussion thread like this one) to disallow name-only syntax, now my API doesn't conform to my OpenAPI description, and in fact there may be no way for me to correct my OpenAPI description so that it does accurately describe my API.

    That why I'm taking the time to do this analysis and write-up. I hope we can converge on the right decision here.

@darrelmiller
Copy link
Member

darrelmiller commented May 13, 2018

RFC 6570 is explicit about whether the = should be included and it depends on the operator used. Here is the data structure I use in my UriTemplate library:

private static Dictionary<char, OperatorInfo> _Operators = new Dictionary<char, OperatorInfo>() {
  {'\0', new OperatorInfo {Default = true, First = "", Seperator = ',', Named = false, IfEmpty = "",AllowReserved = false}},
  {'+', new OperatorInfo {Default = false, First = "", Seperator = ',', Named = false, IfEmpty = "",AllowReserved = true}},
  {'.', new OperatorInfo {Default = false, First = ".", Seperator = '.', Named = false, IfEmpty = "",AllowReserved = false}},
  {'/', new OperatorInfo {Default = false, First = "/", Seperator = '/', Named = false, IfEmpty = "",AllowReserved = false}},
  {';', new OperatorInfo {Default = false, First = ";", Seperator = ';', Named = true, IfEmpty = "",AllowReserved = false}},
  {'?', new OperatorInfo {Default = false, First = "?", Seperator = '&', Named = true, IfEmpty = "=",AllowReserved = false}},
  {'&', new OperatorInfo {Default = false, First = "&", Seperator = '&', Named = true, IfEmpty = "=",AllowReserved = false}},
  {'#', new OperatorInfo {Default = false, First = "#", Seperator = ',', Named = false, IfEmpty = "",AllowReserved = true}}
                                        };

The equal sign is included only for query string parameters.

As far as I understood, allowEmptyValue was a constraint on the value of the parameter, not on how it is serialized. Style was introduced to help controlling serialization. A sub-goal of that was to ensure that serialization matched 6570 rules as closely as possible. 6570 doesn't support a query parameter as a flag, therefore, I'm not convinced that we should either.

@tedepstein
Copy link
Contributor

tedepstein commented May 14, 2018

@darrelmiller,

As far as I understood, allowEmptyValue was a constraint on the value of the parameter, not on how it is serialized.

I don't think this would have made much sense, given that OpenAPI 2.0 Parameter Objects already had type, minLength and maxLength constraints before allowEmptyValue was introduced. OpenAPI 2.0 didn't need a new keyword to specify whether it's OK to send a zero-length string as a parameter value.

I've attempted to pull together the history, and the evidence seems to suggest that allowEmptyValue was introduced to allow for one or possibly two use cases:

  • Flag Parameters - Essentially a "bit" that is on when the parameter is present, or off when the parameter is omitted. Could be mapped naturally to Boolean-typed params as an alternative format, though the OpenAPI spec did not go this route.
  • Unassigned Parameter Value - Very similar to the idea of sending an explicit null value. Essentially saying, "This parameter has no value." Could be useful for overriding a non-null default. For example, if /invoices has a post operation with a message query param that defaults to "Thank you!", the client might want to send ?message or ?message= to suppress the default.

There are other possible semantics:

  • Unspecified Parameter Value might be the intended semantics for brain-dead API clients that include every possible parameter in the query string, using param or param= for those that have not been assigned a value by the client. Whereas Unassigned Parameter Value semantics means the client is explicitly assigning a null value, Unspecified Parameter Value is the equivalent of omitting the parameter entirely. If the API assigns default values, those would be expected to apply here.
  • Empty String might be the intended semantics for APIs that assumed they had to do something special if they wanted to allow param or param= instead of param="". But it seems unlikely that OpenAPI 2.0 would have adopted allowEmptyValue for this purpose...

It might be most accurate to say that allowEmptyValues was added to allow for a certain syntactic form (or maybe two forms), with application-defined semantics.

@tedepstein
Copy link
Contributor

RFC 6570 is explicit about whether the = should be included and it depends on the operator used.
(snip)
A sub-goal of that was to ensure that serialization matched 6570 rules as closely as possible. 6570 doesn't support a query parameter as a flag, therefore, I'm not convinced that we should either.

I like the alignment with RFC 6570.

RFC 6570 seems to model parameter values only in terms of strings, string arrays, and Map<string, string> associative arrays. So mapping OpenAPI's schema object to RFC 6570 seems to require some extra interpretation, and I'm not sure (yet) whether this is fully captured in the "style" table in the spec.

And of course, adopting serialization rules like RFC 6570 clashes with options like allowEmptyValues, which (as far as I'm able to interpret it) is trying to directly specify allowed syntax, without a strict derivation from a schema or type system.

@darrelmiller
Copy link
Member

@tedepstein Yes, based on your comment about minLength being sufficient to identify values being allowed to be empty, then it appears that, despite it's name, allowEmptyValues was more about serialization than empty values. Considering that, perhaps it is unfortunate that we didn't kill it when moving to V3.

On the other hand, maybe we do want to flag style query parameters even though, RFC6570 doesn't. At which point, maybe allowEmptyValues can keep that meaning.

If you are available for the TSC meeting, we can discuss it further. I'm sure @webron will have some light to shine on the subject.

@MikeRalphson
Copy link
Member

@darrelmiller although these relate to OAS 2.0 - two (and only two) Microsoft Azure API definitions specifiy allowEmptyValue: true. These are:

  • applicationinsights/resource-manager/microsoft.insights/stable/2015-05-01/favorites_API
  • trafficmanager/resource-manager/Microsoft.Network/preview/2017-09-01-preview/trafficmanageranalytics

Perhaps reaching out to find the intent of using the flag in these cases might be an interesting data point?

Only two other API definitions across the whole of APIs.guru (1,400+ definitions) use allowEmptyValue so it certainly does not seem to be widely adopted in public APIs.

@tedepstein
Copy link
Contributor

Thanks @MikeRalphson. APIs.guru is an awesome resource. Very helpful in situations like this.

I looked at these:

applicationinsights/resource-manager/microsoft.insights/stable/2015-05-01/favorites_API`

ResourceNameParameter:
    description: The name of the Application Insights component resource.
    in: path
    name: resourceName
    required: true
    type: string
    x-ms-parameter-location: method
  SourceTypeParameter:
    allowEmptyValue: true
    description: 'Source type of favorite to return. When left out, the source type defaults to ''other'' (not present in this enum).'
    enum:
      - retention
      - notebook
      - sessions
      - events
      - userflows
      - funnel
      - impact
      - segmentation
    in: query
    name: sourceType
    required: false
    type: string
    x-ms-enum:
      modelAsString: true
      name: FavoriteSourceType
    x-ms-parameter-location: method

This looks like an Unassigned Value or Unspecified Value use case, though it's not clear which one. With Unassigned Value, sending an empty value would override the default value of "other" with null or empty string. With Unspecified Value, it would be the equivalent of omitting the parameter altogether, and therefore applying the default value of "other."

trafficmanager/resource-manager/Microsoft.Network/preview/2017-09-01-preview/trafficmanageranalytics

        - allowEmptyValue: true
          collectionFormat: csv
          description: 'The top left latitude,longitude pair of the rectangular viewport to query for.'
          in: query
          items:
            format: double
            type: number
          maxItems: 2
          minItems: 2
          name: topLeft
          required: false
          type: array
        - allowEmptyValue: true
          collectionFormat: csv
          description: 'The bottom right latitude,longitude pair of the rectangular viewport to query for.'
          in: query
          items:
            format: double
            type: number
          maxItems: 2
          minItems: 2
          name: botRight
          required: false
          type: array

Probably the Unspecified Value use case, equivalent to omitting the parameters.

@tedepstein
Copy link
Contributor

Apologies to @webron, who has apparently been through a couple of frustrating episodes with allowEmptyValue.

After today's call, and taking into account the apparent low usage as reported in the APIs.guru directory, it seems like we really should deprecate this feature. Aside from having a somewhat misleading name, and seemingly conflating at least two different use cases, it clashes with RFC 6570 because it tries to dictate syntax independently of the schema and serialization rules.

Whether or not we deprecate it, we should agree on its meaning, and update the spec to clarify accordingly.

I was arguing for a more permissive interpretation, in line with the original spec as I understood it. But if we're deprecating this anyway, we probably want to stick with the narrower definition that was intended in the 3.0 revision, and just add language to make that meaning more clear.

If everyone agrees with the following plan, I'll open a PR with the required changes:

  • Deprecate allowEmptyValue, starting with the OpenAPI 3.0.2 specification release.
  • Add language explaining that allowEmptyValue: true permits the use of an "empty value" syntax, as specified in certain styles (in the style table).
    • The meaning of an empty value is application-defined. It may denote an Unassigned Value (very similar or identical to an explicit null value), an Unspecified Value (equivalent to omitting the parameter), a flag (boolean value), or some other meaning as determined by the API provider. (In the case of a flag, it does not take on a different syntactic form, without the equal sign. It's just a different possible interpretation of the same syntax.)
    • Where a parameter specifies allowEmptyValue: true, an empty value can be used regardless of the type of the parameter.
    • In some styles, empty value is indistinguishable from empty string. String-typed parameters that use one of these styles and specify allowEmptyValue: true SHOULD use minLength, enum, or other available constraints to disallow an empty string, so that the empty value syntax in this context is unambiguous.
  • In the planning and community outreach for OpenAPI 4.x, if we find that there is real demand for any of the use cases we identified, including Flag-style Parameters, Unassigned Value, Unspecified Value, etc., we can add new features to OpenAPI that capture these concepts in a way harmonized with the rest of the specification.

@tedepstein
Copy link
Contributor

In PR #1573, targeted for the 3.0.2 patch revision, we have made use of allowEmptyValue NOT RECOMMENDED. Consensus on the TSC is that this feature has ambiguous meanings, and interacts in awkward ways with Schema Object and Parameter style. See previous comments in this issue thread for more details.

We can revisit in OpenAPI 4.x if there's a good use case.

@webron
Copy link
Member

webron commented Jul 12, 2018

Thanks, @tedepstein! Closing.

@vasilich6107
Copy link

it's a crazy spec)
the whole issue to investigate what does it mean

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants