My Contribution to a Popular Open-Source Package Caused a Panic in Golang Projects

Emre Savcı - Aug 27 '22 - - Dev Community

Image description

Hello, today I will tell you how my development to a popular open-source project caused errors to people using the project around the world and what I learned from this process.

Content
— Swaggo Project
— Our Requirement and My Contribution
— The Problem
— Reason of The Problem
— Function Definition Withtout Body (go:linkname)
— The Solution
— Learned Lessons


SWAGGO Project

https://raw.githubusercontent.com/swaggo/swag/master/assets/swaggo.png

Creating swagger documents for your API projects developed in Go language is a little different from languages such as Java, C#. Generally, you can create it by adding Swagger directives in the comment lines of the code you write, and you don't have many alternatives for this.

The swaggo/swag project is one of the most popular and widely used solutions.

The basic process of this project is creating the swagger document by browsing the Go files and processing the comment lines in the definitions. It does this by creating an Abstract Syntax Tree (AST) and navigating it.


Our Requirements and My Contribution

While trying to create a swagger document for a project with my teammate, we realized that we could not make the definition we wanted using the swaggo/swag tool.

What we wanted to do was to define the request and response objects inside the handler function we created and ensure that they can only be accessed from within the function. But when we did this, the swaggo/swag parser package could not access the related request and response objects.

I wanted to make a contribution to the swaggo/swag library in order to overcome this problem.

First of all, I opened an issue to the library and asked if we needed such a thing and that I wanted to contribute by making this development.
In response, they said that it is not possible to parse private definitions and when they asked how I would do it, they were curious about my proposal and were waiting to see it.

I rolled up my sleeves. I did the development at the first opportunity and opened PR for the project.

As a result of the development I made, request/response objects became definable within the function. 💪

add functio scoped struct parse, tests #1283

Describe the PR I added the functionality of parsing function scoped structs.

With this development we will be able to define our request response types in the same function scope as our handlers:

package main

// @Param request body main.Fun.request true "query params" 
// @Success 200 {object} main.Fun.response
// @Router /test [post]
func Fun()  {
    type request struct {
        Name string
    }
    
    type response struct {
        Name string
        Child string
    }
}

Relation issue https://github.com/swaggo/swag/issues/1274

Additional context All tests passed in parser_tests.go. Changes look backward compatible.

Image description

Image description


The Problem

It's a Friday evening... The work is over, there are minutes... I'm looking forward to the weekend... And... I got a message from my teammate that one of our projects got an error 😞

I actually want to share the full conversation:

Image description

Translation:

  • Vural: bro I will tell you something
  • Me: yes bro
  • Vural: one of our services getting error while generating swagger do, and guess what on which part :D
  • Me: is that because of my development asdfasdf
  • Vural: yes, I guess asdfsadfs
  • Me: you can't be serious asdfasdf

We immediately went to the project's Github page and looked at the issues. And what we saw made me a little uneasy.

Many swaggo/swag package users were experiencing errors and they were all getting errors in the CI/CD pipelines or while manually generating swagger due to my development.

Segmentation fault running `swag init` after 1.8.5 release #1309

Describe the bug Since 1.8.5 was released, when running swag init --parseDependency true on my code I encounter a segfault. After switching back to 1.8.4 (or other versions in the 1.8 line) the command completes successfully.

To Reproduce I'm unable to share the entire code this happens on. I will see if I can isolate this to a small code sample I can share.

Stack trace

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x85c8a8]

goroutine 1 [running]:
github.com/swaggo/swag.(*PackagesDefinitions).parseFunctionScopedTypesFromFile(0xc00000e7c8, 0xc00035b980, {0xc0003e15c0, 0x18}, 0xc000cff680)
    /home/sean/go/pkg/mod/github.com/swaggo/swag@v1.8.5/packages.go:168 +0xa8
github.com/swaggo/swag.(*PackagesDefinitions).ParseTypes(0xc00000e7c8)
    /home/sean/go/pkg/mod/github.com/swaggo/swag@v1.8.5/packages.go:110 +0xc9
github.com/swaggo/swag.(*Parser).ParseAPIMultiSearchDir(0xc0001e22a0, {0xc0001d7fb0?, 0x1?, 0x0?}, {0x9587e8?, 0x7?}, 0x64)
    /home/sean/go/pkg/mod/github.com/swaggo/swag@v1.8.5/parser.go:362 +0x3bf
github.com/swaggo/swag/gen.(*Gen).Build(0xc0001db980, 0xc0001e1790)
    /home/sean/go/pkg/mod/github.com/swaggo/swag@v1.8.5/gen/gen.go:177 +0x5c9
main.initAction(0xc0001af680?)
    /home/sean/go/pkg/mod/github.com/swaggo/swag@v1.8.5/cmd/swag/main.go:151 +0x757
github.com/urfave/cli/v2.(*Command).Run(0xc0001ad560, 0xc0001ec580)
    /home/sean/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/command.go:163 +0x5bb
github.com/urfave/cli/v2.(*App).RunContext(0xc000204000, {0xa25860?, 0xc0000240f0}, {0xc000020080, 0x4, 0x4})
    /home/sean/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/app.go:313 +0xb48
github.com/urfave/cli/v2.(*App).Run(...)
    /home/sean/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/app.go:224
main.main()
    /home/sean/go/pkg/mod/github.com/swaggo/swag@v1.8.5/cmd/swag/main.go:221 +0x55d

Your swag version 1.8.5 (1.8.4 and earlier worked)

Go version: 1.18

Desktop (please complete the following information):

  • OS: Pop! OS 22.04 (essentially Ubuntu)

Image description

Frankly, I was surprised by this situation because I wrote unit tests for my development and the code was working correctly. In fact, the successful passing of the current unit tests proved that it did not break any previous development.

So what could be the problem? 🤔


Reason of The Problem

I'm attaching a screenshot from my development for swaggo/swag.

Image description

As we can see in the error messages, the error is received in the parseFunctionScopedTypesFromFile method. In fact, the exact line number of the error is 168.

Now let's take a closer look at this method I wrote.

Image description

In line 168, AST is visited and if there is FunctionDecleration among the definitions, what written inside the defined function is iterated with a for loop.

So if we are sure that a function definition exists, why do we get an error when trying to iterate this function definition with a for loop?
What causes the nil pointer error?

When we look at the functionDeclaration.Body variable, we see that it is a pointer type.

Image description

I didn't do a nil check here because I assumed that if a function was defined, that function would have a body definition.

But in reality it may not be so.👇


Function Definition Without Body

Does this term sound foreign to you? Can we define a function without writing a body?

Consider the code below, do you think this code is a valid definition in Go?

Will we get an error if we try to run it?

Yes, as you guessed, we cannot run or even compile this code because it is not a valid definition.

Can we run the code below?

Here, unlike the previous one, the answer will be yes, we can run it.

A little-known feature of Go is that we can link a function definition to the function definitions of other packages with the go:linkname directive.

Thus, we can define a function without defining a body.

And if you have noticed, this feature allowed me to use a private function defined under the Go runtime package.

External dependencies that we use in our projects can have such function definitions. And when we tried to create a swagger document with the swag --parseDependency command, functions with this definition caused a panic in my development because it also parsed dependencies.


The Solution

I was going to do a development to fix the problem right away, but the good thing about the open-source community is that another developer wrote the code that fixed the bug before me and created a PR.

fix: funcDeclaration body check #1310

rytsh avatar
rytsh posted on

Describe the PR Nil pointer check in function scoped types.

Relation issue #1309

This is fixes problem on running swag build.

Image description


Learned Lessons

With this problem, I learned some lessons for myself.

✅ Never bypass the nil pointer check hypothetically
✅ Unit tests may not always be enough. Do the end to end test
✅ We can write functions without body definition using go:linkname with Go, we can import functions from private packages
✅ Never make the package dependencies the latest version, specify the version you are sure is working


I hope it was a fun and useful content for you. Take care of yourself, I wish you bug-free codes 🙏


Let's Connect
Enter fullscreen mode Exit fullscreen mode

Twitter
Github

You can support me on Github: Support mstrYoda on Github

Buy Me A Coffee

. . . . . . . . . . . .