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

[BUG] Server Object field "url" description is inaccurate #274

Closed
gibson042 opened this issue Sep 23, 2019 · 10 comments
Closed

[BUG] Server Object field "url" description is inaccurate #274

gibson042 opened this issue Sep 23, 2019 · 10 comments
Labels
🐞 Bug keep-open Prevents stale bot from closing this issue or PR

Comments

@gibson042
Copy link
Contributor

Describe the bug
The AsyncAPI specification says about the Server Object url field that it contains a URL and "MAY be relative", but then uses as an example value "development.gigantic-server.com". This is not an (absolute) URL because it lacks a colon-terminated scheme such as "https", but if interpreted as a relative reference then the result will be something like "https://example.com/example-service/asyncapi.json/../development.gigantic-server.com" (specifically because "a relative reference that does not begin with a slash character is termed a relative-path reference" and inherits the authority component and path prefix of its base URI, cf. RFC 3986 section 4.2 and section 5.2.3).

To Reproduce
Steps to reproduce the behavior:

  1. Navigate to https://www.asyncapi.com/docs/specifications/2.0.0/#serverObject or https://www.asyncapi.com/docs/specifications/1.2.0/#serverObject, or their respective git snapshots (2.0.0, 1.2.0).
  2. Observe that url is a URL and "MAY be relative", but the absence of text allowing it to be a bare host name (or IP address, for that matter) despite the use of an example value that looks like one.

Expected behavior
Presumably the intent is to interpret values matching the RFC 1034 section 3.5 preferred name syntax as updated by RFC 1123 section 2.1 (i.e., non-address strings matching the regular expression pattern ^[a-zA-Z0-9]+(-+[a-zA-Z0-9]+)*(\.[a-zA-Z0-9]+(-+[a-zA-Z0-9]+)*)*$) as the host component of a URL with default scheme and path (e.g., "https://{value}/") and use the relative reference treatment only as a fallback. If so, the description should be updated accordingly.

And if there is also a desire to allow not just host names but also IP addresses, that should be documented as well.

Sample document
n/a

Screenshots
n/a

Additional context
n/a

@fmvilas
Copy link
Member

fmvilas commented Sep 30, 2019

Hey @gibson042! Thanks for reporting the bug. I think the problem is actually with the example. It should probably use tcp:// or kafka:// (not sure this one is valid). I don't think we should specify that IP addresses are allowed. They are because they can be part of a URL. I agree we could probably add an example using an IP address.

@stale
Copy link

stale bot commented Nov 29, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 29, 2019
@gibson042
Copy link
Contributor Author

This is still relevant, it just lacks a clear path to resolution.

@stale stale bot removed the stale label Nov 29, 2019
@nickspacek
Copy link

My suggestions to resolve:

  1. Update all examples to use URLs that include scheme.
  2. Add an additional example to illustrate how the server URL is determined when relative URLs are used and consider clarifying the language here.

@stale
Copy link

stale bot commented Feb 2, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 2, 2020
@fmvilas fmvilas added keep-open Prevents stale bot from closing this issue or PR and removed stale labels Feb 3, 2020
@fmvilas fmvilas added this to the AsyncAPI specification 2.0.1 milestone Mar 13, 2020
@fmvilas fmvilas modified the milestones: AsyncAPI specification 2.0.1, AsyncAPI specification 2.1.0 Jan 31, 2021
@fmvilas fmvilas removed this from the Next specification version milestone May 12, 2021
@fmvilas fmvilas added this to the 3.0.0 Release milestone Sep 14, 2021
@jonaslagoni
Copy link
Member

Is there any of you who wants to champion this? 🙂 Or can we consider this issue as needs champion? 🤔

@dalelane
Copy link
Collaborator

Can we say that this one is superseded by #547 ?

@derberg
Copy link
Member

derberg commented Jan 24, 2022

@dalelane definitely we can accept both 😄 I think you are right and better to just continue with #547

@fmvilas
Copy link
Member

fmvilas commented Jan 24, 2022

Yeah, I should have been clear on #547 that it's actually a proposal to fix this issue. Updating it. Thanks!

@fmvilas
Copy link
Member

fmvilas commented Dec 17, 2022

The proposal now moved to RFC 1 stage: #888. Have a look!

@smoya smoya closed this as completed Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug keep-open Prevents stale bot from closing this issue or PR
Projects
None yet
Development

No branches or pull requests

7 participants