Header values are split by default "\\s*,\\s*"

[Jonathan Willis]

headers are getting split on commas, which breaks validation when sending a jsonObject as a header value. Is there anyway we can make this customizable? Looks like by default withHeader always splits the header value. Also looks like it is bad practice to split header values by comma: https://stackoverflow.com/a/35192637/1359765

‌

public Builder withHeader(final String name, final List<String> values) {
    // available but not set headers are considered as empty
    putValuesToMapOrDefault(headers, name, values, "", true);
    return this;
}

static void putValuesToMapOrDefault(final Multimap<String, String> map, final String name,
                                    final List<String> values, final String defaultIfNotSet,
                                    final boolean splitValues) {
    if (values == null || values.isEmpty()) {
        map.putAll(name, splitValues ? splitHeaderValue(defaultIfNotSet) : Collections.singleton(null));
    } else {
        values.forEach(value -> map.putAll(name, splitValues ? splitHeaderValue(value) : Collections.singleton(value)));
    }
}

‌

Related Issue: https://bitbucket.org/atlassian/swagger-request-validator/issues/97 reviewed by @James Navin

[Jonathan Willis]

@James Navin Do you think this can be remedied?

[James Navin]

Hi Jonathan - Im happy to review PRs, but I won’t have time to look at this myself for a few weeks.

From a brief look over the links from that stack overflow response it does indeed look like this is going to be non-trivial to solve in a consistent way.

Im open to suggestions here - a couple options that spring to mind are:

• Allowing consumers to register a ‘split strategy’ on a per-header basis, with the default being a simple CSV
• Implementing header-specific split strategies based on standard headers from the RFCs (could build on top of the first approach)
• ?

‌

[Jonathan Willis]

I was thinking the first problem to solve is to not split header values by default.

‌

[Jonathan Willis]

@James Navin if you have a second could you give a mini summary of some of the code I would need to consider changing? I was thinking it would be nice from the spring mvc project to be able to users to designate whether or not they want splitting to happen. I’m just not sure where to add that sort of functionality.

[Sven Döring]

Just for my information:

What do you mean by ‘jsonObject’? Is it a string with JSON content?
Is this header described in your OpenAPI specification? Could you give a small example?

[Jonathan Willis]

UserContext:
  type: object
  properties:
    id:
      type: integer
    role:
      type: string
      enum: [Guest, User, Admin]
    accountId:
      type: integer

‌

parameters:
  - in: header
    name: usercontext
    schema:
      $ref: '#/components/schemas/UserContext'
    required: true

@Sven Döring As you can see the header value is a literal JsonObject. Does that help?

[Sven Döring]

I see. I only know header as some kind of list. I haven’t thought of adding schemas. Yeah, why not.

‌

Then I think a custom split strategy is not the right way.

The header shouldn’t be parsed at all in the SimpleRequest / SimpleResponse. It shall be parsed within the validation process depending on the OpenAPI specification.
If the header is a schema definition it shall be validated like the body.
If the header is a defined list then it shall be splitted by the specified splitting character in the interface definition.
If it’s just a number, string, etc. it’s validated like that - no splitting after all.

[Jonathan Willis]

Sven, does this mean you’ll be working on it. I would love to do it but I’m just not that familiar with the codebase and because of that don’t have the time to dive into it and update it

[Sven Döring]

This decision has not been made, yet.

I would like to here @James Navin opinion about the former comment. Perhaps I haven’t got it right.

[Jonathan Willis]

For the most part, i think you got it right. According to the RFC I don’t think headers are supposed to be split. However, if a header is listed multiple times then the header values can be combined into a comma separated list.

[Sven Döring]

Currently I’m looking at the OpenAPI documentation.

It says:

---

Header parameters always use the simple style, that is, comma-separated values. This corresponds to the {param_name} URI template. An optional explode keyword controls the object serialization.

---

Splitting the header values always at a comma seems about right.

Whole objects shall not be submitted as JSON strings but as exploded parameters.

See: https://swagger.io/docs/specification/serialization/

[Jonathan Willis]

Yeah, I looked through that swagger openapi 3 specification and I have mixed feelings about it. It’s kind of going against the HTTP standards which i instantly have a knee-jerk reaction against doing. So it just depends on what standard we would want to follow. Plus, the http standard has specific formats for different header fields. For example, this is an example of an Accept header field: Accept: foo/bar;p="A,B,C", bob/dole;x="apples,oranges" which would be parsed incorrectly by the swagger validator.

See: https://tools.ietf.org/html/rfc2616 https://tools.ietf.org/html/rfc7230 https://tools.ietf.org/html/rfc7231

[James Navin]

Thanks all for the input. I have revisited this issue while looking at fixing the behavior of array param validation.

After going back over the spec and the RFCs I am planning to do the following:

1. Remove the header splitting in SimpleRequest altogether - as discussed above this is almost certainly the wrong behavior in almost all cases
2. Perform the csv splitting during array param validation instead, for styles that require it (e.g. for the default simple style)
3. For Swagger v2 specs, will support multi-valued headers when the collectionFormat is multi
4. For OAI v3 specs, will support multi-valued headers when the style is form (even though this isn’t strictly supported in OAI specs I think it makes sense to allow it for consistency with swagger v2)
5. Changes will be released as a minor version bump

I have a branch open with most of the changes on it (just cleaning up some failing tests etc.) - https://bitbucket.org/atlassian/swagger-request-validator/branch/290-fix-array-params#diff

Feel free to comment further, otherwise I plan to get the changes out in the next few days.

[James Navin]

Available in v2.11.0