camaraproject / QualityOnDemand

Repository to describe, develop, document and test the QualityOnDemand API family
https://wiki.camaraproject.org/x/zwOeAQ
Apache License 2.0
38 stars 60 forks source link

Format 'ipv4' is more restrictive than the pattern in Data Type 'Ipv4Address' #146

Closed tlohmar closed 1 year ago

tlohmar commented 1 year ago

Problem description The Data Type 'Ipv4Address' is defined as

Ipv4Address:
      type: string
      format: ipv4
      pattern: '^([01]?\d\d?|2[0-4]\d|25[0-5])(?:\.(?:[01]?\d\d?|2[0-4]\d|25[0-5])){3}(\/([0-9]|[1-2][0-9]|3[0-2]))?$'
      example: "192.168.0.1/24"

The format 'ipv4' is defined by the OpenAPI specification as "An IPv4 address according to the "dotted-quad" ABNF syntax as defined in RFC 2673, section 3.2 [RFC2673].". The "dotted-quad" does not allow the addition of a netmask, e.g. "/24"

The regular expression of the pattern allows also the addition of a netmask. The provided example "192.168.0.1/24" is not comply with the format 'ipv4'

A similar issue exist for data type Ipv6Address.

Expected behavior The suggested correction is to replace 'ipv4' with 'string' and rely on the regular expression in pattern.

hdamker commented 1 year ago

@eric-murray @akoshunyadi @jlurien @patrice-conil any opinions on the proposed solution (expected behavior)?

eric-murray commented 1 year ago

From OAS v3.0.3:

Primitives have an optional modifier property: format. OAS uses several known formats to define in fine detail the data type being used. However, to support documentation needs, the format property is an open string-valued property, and can have any value. Formats such as "email", "uuid", and so on, MAY be used even though undefined by this specification.

ipv4 is not a format defined by OAS, and is not mandated by the v3.0.3 specification to use the same definition as the JSON schema specification. The tooling I am aware of ignores format: ipv4, hence the need for the pattern. But format: ipv4 is a good hint to the API caller, so I'd suggest to keep it, at least whilst API definitions are based on v3.0.3. Adding a suitable description would probably help here as well.

OAS v3.1.0 does link format to the JSON schema definitions, so maybe we will need to revisit the issue if and when we start using that version. But I don't see having both format: ipv4 and a pattern that supports subnets ever causing a conflict, assuming sensible implementation choices by tooling developers.

tlohmar commented 1 year ago

two questions: 1: "ipv4 is not a format defined by OAS, and is not mandated by the v3.0.3 specification...": Do all tooling chains ignore "undefined data types" or do some tooling chains throw an "format=ipv4 not defined" error? It might be generally better to only use defined data-types, either by OAS or within the QOD yaml. 2: The format "ipv4" is different than the "QOD pattern". The example '192.168.0.1/24' would not work, when moving to OAS 3.1.0.

hdamker commented 1 year ago

Done by #153