Skip to content

External $ref in paths doesn't follow a relative URI #53

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

Closed
pwfff opened this issue Jan 4, 2016 · 12 comments
Closed

External $ref in paths doesn't follow a relative URI #53

pwfff opened this issue Jan 4, 2016 · 12 comments

Comments

@pwfff
Copy link

pwfff commented Jan 4, 2016

Example (another_file.yaml is hosted alongside the parent yaml file, and works with swagger-ui):

paths:
  $ref: 'another_file.yaml'
  File "---/.virtualenvs/uad/lib/python2.7/site-packages/pyswagger/core.py", line 359, in create
    app.prepare(strict=strict)
  File "---/.virtualenvs/uad/lib/python2.7/site-packages/pyswagger/core.py", line 324, in prepare
    self._prepare_obj(strict=strict)
  File "---/.virtualenvs/uad/lib/python2.7/site-packages/pyswagger/core.py", line 267, in _prepare_obj
    s.scan(root=self.__root, route=[Resolve()])
  File "---/.virtualenvs/uad/lib/python2.7/site-packages/pyswagger/scan.py", line 125, in scan
    handle_cls(cls)
  File "---/.virtualenvs/uad/lib/python2.7/site-packages/pyswagger/scan.py", line 118, in handle_cls
    ret = ff(the_self, path, obj, self.app)
  File "---/.virtualenvs/uad/lib/python2.7/site-packages/pyswagger/scanner/v2_0/resolve.py", line 78, in _path_item
    _merge(obj, app, '#/paths', PathItemContext)
  File "---/.virtualenvs/uad/lib/python2.7/site-packages/pyswagger/scanner/v2_0/resolve.py", line 44, in _merge
    _resolve(cur, app, prefix, ctx)
  File "---/.virtualenvs/uad/lib/python2.7/site-packages/pyswagger/scanner/v2_0/resolve.py", line 26, in _resolve
    ro = app.resolve(normalize_jr(r, prefix), parser)
  File "---/.virtualenvs/uad/lib/python2.7/site-packages/pyswagger/core.py", line 400, in resolve
    obj = self.root.resolve(utils.jp_split(jp)[1:]) # heading element is #, mapping to self.root
  File "---/.virtualenvs/uad/lib/python2.7/site-packages/pyswagger/spec/base.py", line 276, in resolve
    obj = obj[t]
KeyError: 'another_file.yaml'
@mission-liao
Copy link
Member

I just verified the spec didn't have this part of description. (See Paths Object)

The only field could exist under "paths" is a map from relative urls to Path Item Object. And the $ref can be placed under Path-Item Object, but not Paths Object.

And it seems that relative issues raised againt the SPEC team, but nothing is confirmed to change yet.

Therefore, even swagger-ui supports this, I might choose not to support this behavior. The spec-compliant way to do this is to place the '$ref' under Path-Item Object. Thanks for reporting issues, very appreciated.

note: a bug has been found that relative reference in this case doesn't work well. I'll fix this part later.

mission-liao added a commit that referenced this issue Jan 5, 2016
- relative reference of Path Item Object
- #53
@pwfff
Copy link
Author

pwfff commented Jan 5, 2016

Oops, I made my example wrong. I did indeed have it under the Path-Item Object. Thank you for the fix!

@pwfff
Copy link
Author

pwfff commented Jan 5, 2016

I'm having problems with $ref in other places, but I might be doing it all wrong. I'm currently trying to work out a good test case for what I'm doing. Once I have something easily reproducible I'll post back here.

@mission-liao
Copy link
Member

No problem, raise any question if needed. :)

@pwfff
Copy link
Author

pwfff commented Jan 6, 2016

This should be a good example:

==> internal.yaml <==
swagger: '2.0'
info:
  title: Acme Internal API
  description: Docs for working with secret stuff
  version: 0.0.1
host: api.acme.com
basePath: /v1
schemes:
  - https
produces:
  - application/json
paths:
  '/login':
    $ref: 'login.yaml'
  '/secret_stuff':
    $ref: 'secret_stuff.yaml'

==> login.yaml <==
post:
  operationId: post_login
  summary: Log a user in
  description: Insert username and password, receive token
  parameters:
    - name: login_form
      in: body
      required: true
      schema:
        $ref: 'definitions/Login.yaml'
  responses:
    '200':
      description: Login result containing a session token
      schema:
        $ref: 'definitions/LoginResult.yaml'
    '403':
      description: Returned when credentials are incorrect

==> public.yaml <==
swagger: '2.0'
info:
  title: Acme Public API
  description: Docs for working with normal stuff
  version: 0.0.1
host: api.acme.com
basePath: /v1
schemes:
  - https
produces:
  - application/json
paths:
  '/login':
    $ref: 'login.yaml'
  '/stuff':
    $ref: 'stuff.yaml'

==> secret_stuff.yaml <==
post:
  operationId: post_secret_stuff
  security:
    - session_token: []
  description: does some secret stuff
  parameters:
    - name: secret_stuff_form
      in: body
      required: true
      schema:
        $ref: 'definitions/Stuff.yaml'
  responses:
    '200':
      description: Results of doing the stuff
      schema:
        $ref: 'definitions/StuffResults.yaml'
    '403':
      description: Returned when token is incorrect

==> stuff.yaml <==
post:
  operationId: post_stuff
  security:
    - session_token: []
  description: does some stuff
  parameters:
    - name: stuff_form
      in: body
      required: true
      schema:
        $ref: 'definitions/Stuff.yaml'
  responses:
    '200':
      description: Results of doing the stuff
      schema:
        $ref: 'definitions/StuffResults.yaml'
    '403':
      description: Returned when token is incorrect

==> definitions/Login.yaml <==
description: test
type: object
properties:
  id:
    type: integer

==> definitions/LoginResults.yaml <==
description: test
type: object
properties:
  id:
    type: integer

==> definitions/Stuff.yaml <==
description: test
type: object
properties:
  id:
    type: integer

==> definitions/StuffResults.yaml <==
description: test
type: object
properties:
  id:
    type: integer

And the error I get:

/Users/---/pyswagger/pyswagger/core.pyc in create(kls, url, strict)
    357         """
    358         app = kls.load(url)
--> 359         app.prepare(strict=strict)
    360
    361         return app

/Users/---/pyswagger/pyswagger/core.pyc in prepare(self, strict)
    322         """
    323
--> 324         self._prepare_obj(strict=strict)
    325
    326         # reducer for Operation

/Users/---/pyswagger/pyswagger/core.pyc in _prepare_obj(self, strict)
    265                 self.__schemes = [six.moves.urlparse(self.__url).schemes]
    266
--> 267         s.scan(root=self.__root, route=[Resolve()])
    268         s.scan(root=self.__root, route=[PatchObject()])
    269         s.scan(root=self.__root, route=[Aggregate()])

/Users/---/pyswagger/pyswagger/scan.pyc in scan(self, route, root, nexter, leaves)
    123                     if cls is BaseObj:
    124                         break
--> 125                     handle_cls(cls)
    126

/Users/---/pyswagger/pyswagger/scan.pyc in handle_cls(cls)
    116                     if f:
    117                         for ff in f:
--> 118                             ret = ff(the_self, path, obj, self.app)
    119                             if res:
    120                                 res(the_self, ret)

/Users/---/pyswagger/pyswagger/scanner/v2_0/resolve.pyc in _path_item(self, _, obj, app)
     76         # we need to merge properties of others if missing
     77         # in current object.
---> 78         _merge(obj, app, '#/paths', PathItemContext)
     79

/Users/---/pyswagger/pyswagger/scanner/v2_0/resolve.pyc in _merge(obj, app, prefix, ctx)
     42     to_resolve = []
     43     while not is_resolved(cur):
---> 44         _resolve(cur, app, prefix, ctx)
     45
     46         to_resolve.append(cur)

/Users/---/pyswagger/pyswagger/scanner/v2_0/resolve.pyc in _resolve(obj, app, prefix, parser)
     24
     25     r = getattr(obj, '$ref')
---> 26     ro = app.resolve(normalize_jr(r, prefix, app.url), parser)
     27
     28     if not ro:

/Users/---/pyswagger/pyswagger/core.pyc in resolve(self, jref, parser)
    387                 # This loaded SwaggerApp would be kept in app_cache.
    388                 app = SwaggerApp.load(url, parser=parser, app_cache=self.__app_cache, url_load_hook=self.__url_load_hook)
--> 389                 app.prepare()
    390
    391                 # nothing but only keeping a strong reference of

/Users/---/pyswagger/pyswagger/core.pyc in prepare(self, strict)
    322         """
    323
--> 324         self._prepare_obj(strict=strict)
    325
    326         # reducer for Operation

/Users/---/pyswagger/pyswagger/core.pyc in _prepare_obj(self, strict)
    265                 self.__schemes = [six.moves.urlparse(self.__url).schemes]
    266
--> 267         s.scan(root=self.__root, route=[Resolve()])
    268         s.scan(root=self.__root, route=[PatchObject()])
    269         s.scan(root=self.__root, route=[Aggregate()])

/Users/---/pyswagger/pyswagger/scan.pyc in scan(self, route, root, nexter, leaves)
    123                     if cls is BaseObj:
    124                         break
--> 125                     handle_cls(cls)
    126

/Users/---/pyswagger/pyswagger/scan.pyc in handle_cls(cls)
    116                     if f:
    117                         for ff in f:
--> 118                             ret = ff(the_self, path, obj, self.app)
    119                             if res:
    120                                 res(the_self, ret)

/Users/---/pyswagger/pyswagger/scanner/v2_0/resolve.pyc in _schema(self, _, obj, app)
     60     @Disp.register([Schema])
     61     def _schema(self, _, obj, app):
---> 62         _resolve(obj, app, '#/definitions', SchemaContext)
     63
     64     @Disp.register([Parameter])

/Users/---/pyswagger/pyswagger/scanner/v2_0/resolve.pyc in _resolve(obj, app, prefix, parser)
     24
     25     r = getattr(obj, '$ref')
---> 26     ro = app.resolve(normalize_jr(r, prefix, app.url), parser)
     27
     28     if not ro:

/Users/---/pyswagger/pyswagger/core.pyc in resolve(self, jref, parser)
    393                 self.__strong_refs.append(app)
    394
--> 395             return self.__app_cache[url].resolve(jp)
    396
    397         if not jp.startswith('#'):

/Users/---/pyswagger/pyswagger/core.pyc in resolve(self, jref, parser)
    398             raise ValueError('Invalid Path, root element should be \'#\', but [{0}]'.format(jref))
    399
--> 400         obj = self.root.resolve(utils.jp_split(jp)[1:]) # heading element is #, mapping to self.root
    401
    402         if obj == None:

/Users/---/pyswagger/pyswagger/spec/base.pyc in resolve(self, ts)
    270
    271             if issubclass(obj.__class__, BaseObj):
--> 272                 obj = getattr(obj, t)
    273             elif isinstance(obj, list):
    274                 obj = obj[int(t)]

AttributeError: 'PathItem' object has no attribute 'definitions'

@mission-liao
Copy link
Member

You are correct, the fix I proposed only applied to PathItem Object, and according to spec, all Reference Object should support relative files reference.

I'll fix it and sorry for bothering you again. :(

@pwfff
Copy link
Author

pwfff commented Jan 6, 2016

No problem! It's a pleasure working with you!

@mission-liao
Copy link
Member

note: In older version of 2.0 spec, this schema is valid:

{
  "definitions":{
    "User": {},
    "AuthorizedUser":{
       "$ref": "User"
    }
  }
}

The AuthorizedUser would automatically reference to "#definitions/User". It's not clear for Swagger 2.0 to support this behaviour or not. If I fix relative file reference rooted with Schema Object, this behaviour is no longer supported.

mission-liao added a commit that referenced this issue Jan 7, 2016
- deprecate implicit dereference
- support relative file reference
- #53
@pwfff
Copy link
Author

pwfff commented Jan 7, 2016

The second example here doesn't seem to work: https://github.com/OAI/OpenAPI-Specification/blob/master/guidelines/REUSE.md#guidelines-for-referencing

It looks like it's expecting whatever file it loads there to be of the Schema type, though the schema could be nested anywhere in the yaml/json that gets loaded. Also, to get that far I had to do the following:

diff --git a/pyswagger/utils.py b/pyswagger/utils.py
index 3269f57..160bcba 100644
--- a/pyswagger/utils.py
+++ b/pyswagger/utils.py
@@ -328,7 +328,8 @@ def normalize_jr(jr, url=None):
         if p.scheme == '' and url:
             p = six.moves.urllib.parse.urlparse(url)
             # it's the path of relative file
-            path = six.moves.urllib.parse.urlunparse(p[:2]+(os.path.join(os.path.dirname(p.path), jr),)+p[3:])
+            basepath = os.path.dirname(p.path)
+            path = six.moves.urllib.parse.urlunparse(p[:2]+(os.path.join(basepath, path),)+p[3:])
     else:
         path = url

I'm testing it now by just having each definition in its own file and it seems to be working.

@mission-liao
Copy link
Member

At the time when Swagger 2.0 just came out, the only 'external referencible' object is Path Item. And I didn't follow the later discussion. I'm a python guy so I prefer explicit to implicit. I would definitely complained about this kind of design if I joined those discussion.

It's still possible to make this work, let me investigate it for a while. Big thanks for your efforts on trying these scenario!


It's actually much more complicated than I expect, refer to this discussion

It's definitely legit in JSON Schema. I sometimes wish it wasn't...the horrors I've seen

My head gets hurt now.......


Asking question here, await replies.


Discussion kept going...


It seems clear now, will proceed to redesign the resolving procedure.


The implementation of pyswagger about "external $ref" is not correct according to REUSE Guide. In short, pyswagger requires the root object externally referenced be either PathItem, Schema, Parameter, Response, or Swagger object, but the spec suggests that the root object can be any valid structure, as long as the object pointed by $ref is either PathItem, Schema, Parameter, Response, or Swagger.

@mission-liao
Copy link
Member

@pwfff A PR is merged to master branch and a new build would be provided next Tuesday, any further issue found later would also be included.

It's getting harder to provide a fix for pyswagger :(

@mission-liao
Copy link
Member

Those fixes should be included in 0.8.17, reopen this issue if you find anything broken. :)

mission-liao added a commit to pyopenapi/pyopenapi that referenced this issue Aug 12, 2017
- relative reference of Path Item Object
- pyopenapi/pyswagger#53
mission-liao added a commit to pyopenapi/pyopenapi that referenced this issue Aug 12, 2017
- deprecate implicit dereference
- support relative file reference
- pyopenapi/pyswagger#53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants