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

Absolute path in $ref resolving errors are not helpful #76

Closed
alasdairhurst opened this issue Oct 24, 2017 · 5 comments · Fixed by APIDevTools/json-schema-ref-parser#75

Comments

@alasdairhurst
Copy link

alasdairhurst commented Oct 24, 2017

Pre- 4.0.0 when failing to parse a $ref, the error would come out like this:
Error resolving $ref pointer "#/definitions/ApiResponse". \nToken "ApiResponse" does not exist.

Now it's kinda annoying since the cwd is added to the beginning by default. This path seems to be mostly useless since the document doesn't exist there and it's just guessing.

Error resolving $ref pointer "d:/git/node-app/#/definitions/ApiResponse". \nToken "ApiResponse" does not exist.

(actual location)
d:/git/node-app/test/files/invalid-refs.json#/definitions/ApiResponse". \nToken "ApiResponse" does not exist.'

It should probably just default to an empty string if the path is not provided, and output the same as it used to do. Then if a path is provided when calling validate/dereference, it uses that.

When calling swaggerParser.validate(file, swagger, ... I also see inconsistent results based on the value of the first parameter (path).

  1. swaggerParser.validate('', swagger, ... gives me Error resolving $ref pointer "d:/git/node-app/#/definitions/ApiResponse". \nToken "ApiResponse" does not exist.
    when i was expecting Error resolving $ref pointer "#/definitions/ApiResponse". \nToken "ApiResponse" does not exist.
  2. swaggerParser.validate('foo', swagger, ... gives me Error resolving $ref pointer "d:/git/node-app/foo#/definitions/ApiResponse". \nToken "ApiResponse" does not exist.
    when i was expecting Error resolving $ref pointer "foo#/definitions/ApiResponse". \nToken "ApiResponse" does not exist.
  3. swaggerParser.validate('d:/git/node-app/test/files/invalid-refs.json', swagger, ... gives me the expected Error resolving $ref pointer "d:/git/node-app/test/files/invalid-refs.json#/definitions/ApiResponse". \nToken "ApiResponse" does not exist.
  4. swaggerParser.validate('./test/files/invalid-refs.json', swagger, ... gives me Error resolving $ref pointer "d:/git/node-app/test/files/invalid-refs.json#/definitions/ApiResponse". \nToken "ApiResponse" does not exist. when i was expecting Error resolving $ref pointer "./test/files/invalid-refs.json#/definitions/ApiResponse". \nToken "ApiResponse" does not exist.

TLDR:

  1. The use of process.cwd() for the default path seems wrong
  2. I can't seem to specify a custom path without it trying to resolve it. This may be expected behaviour if it's trying to use the path to get the swagger, but if the swagger is already provided, this seems to only be used for logging so should be customizable.
  3. It's difficult to test error messages now since I now need to convert the path using the same logic as swagger-parser.
@JamesMessinger
Copy link
Member

Swagger-Parser relies on having absolute paths/URLs to each file in the schema, so it can accurately resolve $ref pointers. In all of the cases you listed above, the path you provided is being resolved to an absolute path. If the path you pass-in is relative, then it will be resolved relative to the CWD as documented here.

If you don't want Swagger-Parser to use the CWD, then you can pass in any absolute path or URL.
For example:

swaggerParser.validate('/some/absolute/path', myApiDefinition, ...);

swaggerParser.validate('http://example.com', myApiDefinition, ...);

Here's a live demo

@alasdairhurst
Copy link
Author

Ok fair enough. Something smells odd about it though.

  1. Rather than defaulting to cwd it seems like it should just fail to resolve relative references if you don't supply a path. In my case using cwd was meaningless and only gave an unexpected location in the error message
  2. the path which is created is different to paths generated by node. On windows ,I would expect the expect to be 'D:\\git\\node-app' and not d:/git/node-app. This would allow me to accurately test the errors that come back. I only use custom external schema references and internal references though so i don't know if there's any windows issue with this path when dereferencing.

@alasdairhurst
Copy link
Author

@BigstickCarpet Seems related but ran into another issue with absolute paths and refs when writing a custom resolver.
I have a database where you can register schemas, and access by ID. This ID can be anything defined in the schema. The resolver is implemented as follows:

resolve: {
					file: false, // Don't resolve local file references
					http: false, // Don't resolve http references
					schema: {
						order: 1,
						canRead: true,
						read: (file, callback) => {
							const schema = schemas.get(file.url);
							if (schema) {
								return callback(null, schema);
							}
							return callback(new Error(`Error resolving schema $ref: ${file.url}`));
						}
					}
				},

my resolver looks up the schema via the $ref. This ref doesn't seem like it is avaialble to the resolver though... If the ref is url-like it works ok, and i get back file: { url: "schema://foo" }, however if the ref is "foo-bar" then somewhere down the line it is assumed to be a file and converted into an absolute path, and i get back Error resolving schema $ref: d:/git/node-app/foo-bar

Surely only the file resolver should be working this way? any other resolver should just get the raw ref to handle as it needs to.

@realityking
Copy link

I just ran into this issue trying to update apiaryio/fury-adapter-swagger to version 4. Would it be possible to add an option to get the old behaviour back?

@JamesMessinger
Copy link
Member

I'd be happy to accept a PR that safely brings back the old behavior without breaking any of the existing functionality. Resolving and normalizing relative paths was the only way I could come up with to reliably and consistently resolve references. For example, consider that all of the following references could possibly point to the same file:

"$ref": "/root/path/to/file.yaml"
"$ref": "../../relative/path/to/file.yaml"
"$ref": "relative/path/to/file.yaml"
"$ref": "relative/path%20with%20URL-escaped%20characters/to/file.yaml"

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

Successfully merging a pull request may close this issue.

3 participants