Git Product home page Git Product logo

Comments (5)

daveshanley avatar daveshanley commented on June 18, 2024 1

Hi Sergey,

Regarding third-party code being required for validation - yes, you're right. Eventually, this will be made available as a utility along side request/response validation, see #36.

I ran this code:

    var operations []string
    duplicated := false
    for url, path := range model.Model.Paths.PathItems {
        for method, op := range path.GetOperations() {
            if len(op.OperationId) > 0 {
                if slices.Contains(operations, op.OperationId) {
                    fmt.Printf("Semantic error at paths.%s.%s.operationId\n", url, method)
                    fmt.Printf("Operations must have unique operationIds.\n\n")

                    duplicated = true
                    continue
                }
                operations = append(operations, op.OperationId)
            }
        }
    }
    fmt.Println(duplicated)

Against this spec:

openapi: 3.0.3
info:
  version: 1.0.0
  title: Example
servers:
  - url: 'http://127.0.0.1:8080'
paths:
  /v1/foo:
    get:
      summary: Some summary
      responses:
        '204':
          description: No content

No error, no panic, no segfault. I am unable to re-produce this error.

from libopenapi.

sergeyklay avatar sergeyklay commented on June 18, 2024 1

Well, it must be some kind of build issue. At least I did not change the code in this module, but added new dependencies to the project and the problem went away.

I'm going to close this issue because I can't reproduce it anymore (although I didn't change the code);

Thanks for your patience and answers.

from libopenapi.

sergeyklay avatar sergeyklay commented on June 18, 2024

Probably the problem is slightly different and I mistakenly used this incorrectly. However, it is noteworthy that it can lead to the panic. Bellow are other ways to face a similar issue.

Prepare the code to test:

func testMe(model *libopenapi.DocumentModel[v3high.Document]) {
	for url, path := range model.Model.Paths.PathItems {
		for method, op := range path.GetOperations() {
			fmt.Printf("path: %s, operation: %s (%s)\n", url, method, op.OperationId)
		}
	}
}

And some test data:

panic: runtime error: invalid memory address or nil pointer dereference

openapi: 3.0.3

panic: runtime error: invalid memory address or nil pointer dereference

  openapi: 3.0.3
+ paths:

All is OK. Test passed

  openapi: 3.0.3
+ components:
  paths:

All is OK. Test passed

  openapi: 3.0.3
  components:
  paths:
+   /v1/users:
+     get:
+       responses:
+         '204':
+           description: No content

panic: runtime error: invalid memory address or nil pointer dereference

  openapi: 3.0.3
- components:
  paths:
    /v1/users:
      get:
        responses:
          '204':
            description: No content

panic: runtime error: invalid memory address or nil pointer dereference

  openapi: 3.0.3
  paths:
    /v1/users:
      get:
+       operationId: users.list
        responses:
          '204':
            description: No content

All is OK. Test passed

  openapi: 3.0.3
+ components:
  paths:
    /v1/users:
      get:
        operationId: users.list
        responses:
          '204':
            description: No content

Well, apparently I lack some way to check the validity of a spec before parsing it, since the document comes from the outside (user input) and I can't guarantee that it meets the OpenAPI specification.

from libopenapi.

daveshanley avatar daveshanley commented on June 18, 2024

Hi Sergey,

I ran your first snippet of code:

for url, path := range model.Model.Paths.PathItems {
    for method, op := range path.GetOperations() {
        fmt.Printf("path: %s, operation: %s (%s)\n", url, method, op.OperationId)
     }
}

I got the following output:

path: /v1/foo, operation: get ()

This is the correct behavior.

As I read your examples, I think you may be overlooking a core principal of the model.

You cannot navigate through that which is not there

For example, if you take this test snippet:

  openapi: 3.0.3
+ paths:

And you write code that explicitly calls on something that does not exist:

for url, path := range model.Model.Paths.PathItems {

This will always throw a panic if you try to navigate to an object that isn't there in this case, there are no PathItems so trying to use that property of Paths will fail. PathItems is nil, there are none in the spec.

If you look at the model for Paths (

PathItems map[string]*PathItem
)

You can see that the PathItems is a map

type Paths struct {
    PathItems  map[string]*PathItem  
    Extensions map[string]any
    low        *low.Paths
}

This means if there are no PathItems the map is nil, and everything else will be nil.

So you won't be able to just blindly call every properly of every object, if you check the API, they are mostly ALL pointers, which will always be nil if they are not set in the specification. For every property, unless it's a primitive, you will need to check it's not nil, so to fix your code above, you would need to do this:

 model, errs := doc.BuildV3Model()
    if len(errs) > 0 {
        panic(errs)
    }

    if model.Model.Paths != nil {
        for url, path := range model.Model.Paths.PathItems {
            for method, op := range path.GetOperations() {
                fmt.Printf("path: %s, operation: %s (%s)\n", url, method, op.OperationId)
            }
        }
    } else {
        fmt.Println("No paths!")
    }

There isn't anything for me to fix here as this is all desired behavior. So to discuss your other desire: Validation.

It will be a very, very big job to try and use the libopenapi model to validate an OpenAPI specification. The spec is huge, and going through every variation and combination would take some time.

If you want to know how much work it would be, take a look at the what-changed module. That code performs a deep, 1-1 check of everything
between two specs - there is a lot of code.

What I would recommend for validation is that you validate the spec BEFORE reading it into a model using libopenapi.

The easiest and most reliable way to do this is to use JSON Schema.

  1. Read in the OpenAPI Schema for which ever version you're using here is the 3.0 schema
  2. Use https://github.com/santhosh-tekuri/jsonschema to validate the OpenAPI spec against the schema.
  3. If it's valid - you're ready to now use libopenapi
  4. Don't forget to check for nil

I hope this helps. Thanks for using my tool!

from libopenapi.

sergeyklay avatar sergeyklay commented on June 18, 2024

@daveshanley Thank you for your response! Many things do become clearer thanks to your answer. However, I just wanted to point out that currently libopenapi still needs to pre-check OpenAPI documents using third-party tools, as well as libopenapi design relies on the fact that user input meeting the specification.

Btw, what about this document and the test code I mentioned above? This document leads to segfault

openapi: 3.0.3


paths:
  /v1/users:
    get:
      summary: Some summary
      operationId: users.list
      responses:
        '204':
          description: No content

Realworld example:

func validateDoc(model *libopenapi.DocumentModel[v3high.Document]) bool {
	var operations []string
	var duplicated = false

	if model.Model.Paths == nil {
		return true
	}

	for url, path := range model.Model.Paths.PathItems {
		for method, op := range path.GetOperations() {
			if len(op.OperationId) > 0 {
				if slices.Contains(operations, op.OperationId) {
					fmt.Printf("Semantic error at paths.%s.%s.operationId\n", url, method)
					fmt.Printf("Operations must have unique operationIds.\n\n")

					duplicated = true
					continue
				}
				operations = append(operations, op.OperationId)
			}
		}
	}

	return !duplicated
}

And the spec

openapi: 3.0.3
info:
  version: 1.0.0
  title: Example
servers:
  - url: 'http://127.0.0.1:8080'
paths:
  /v1/foo:
    get:
      summary: Some summary
      responses:
        '204':
          description: No content

This code leads to:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x102eac614]

goroutine 1 [running]:

from libopenapi.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.