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

Swagger spec gen #159

Merged
merged 16 commits into from
Nov 21, 2022
Merged

Swagger spec gen #159

merged 16 commits into from
Nov 21, 2022

Conversation

parrasajad
Copy link
Contributor

@parrasajad parrasajad commented Aug 24, 2022

Proposed changes

Example

$ swaggergen --api example.com  --spec openapi.yaml --log-dir /tmp/proxify/logs

Known Issue:

  • differentiating path param from resource path #164

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

cmd/swaggergen/swaggergen.go Fixed Show fixed Hide fixed
f.Close()
return err
}
defer f.Close()

Check warning

Code scanning / SonarCloud

<!--SONAR_ISSUE_KEY:AYLPiw-sQ9IVopdsQzhY-->Deferring unsafe method "Close" on type "*os.File" <p>See more on <a href="https://sonarcloud.io/project/issues?id=projectdiscovery_proxify&issues=AYLPiw-sQ9IVopdsQzhY&open=AYLPiw-sQ9IVopdsQzhY&pullRequest=159">SonarCloud</a></p>

<!--SONAR_ISSUE_KEY:AYLPiw-sQ9IVopdsQzhY-->Deferring unsafe method "Close" on type "*os.File" <p>See more on <a href="https://sonarcloud.io/project/issues?id=projectdiscovery_proxify&issues=AYLPiw-sQ9IVopdsQzhY&open=AYLPiw-sQ9IVopdsQzhY&pullRequest=159">SonarCloud</a></p>
cmd/swaggergen/swaggergen.go Fixed Show fixed Hide fixed
cmd/swaggergen/swaggergen.go Fixed Show fixed Hide fixed
return nil
}
// open file
f, err := os.Open(path)

Check warning

Code scanning / SonarCloud

<!--SONAR_ISSUE_KEY:AYLPiw-sQ9IVopdsQzhV-->Potential file inclusion via variable <p>See more on <a href="https://sonarcloud.io/project/issues?id=projectdiscovery_proxify&issues=AYLPiw-sQ9IVopdsQzhV&open=AYLPiw-sQ9IVopdsQzhV&pullRequest=159">SonarCloud</a></p>

<!--SONAR_ISSUE_KEY:AYLPiw-sQ9IVopdsQzhV-->Potential file inclusion via variable <p>See more on <a href="https://sonarcloud.io/project/issues?id=projectdiscovery_proxify&issues=AYLPiw-sQ9IVopdsQzhV&open=AYLPiw-sQ9IVopdsQzhV&pullRequest=159">SonarCloud</a></p>
cmd/swaggergen/swaggergen.go Fixed Show fixed Hide fixed
cmd/swaggergen/swaggergen.go Fixed Show fixed Hide fixed
@parrasajad parrasajad marked this pull request as ready for review September 5, 2022 08:53
cmd/swaggergen/swaggergen.go Fixed Show fixed Hide fixed
cmd/swaggergen/swaggergen.go Fixed Show fixed Hide fixed
@parrasajad parrasajad linked an issue Sep 5, 2022 that may be closed by this pull request
Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggested some changes

cmd/swaggergen/swaggergen.go Outdated Show resolved Hide resolved
cmd/swaggergen/swaggergen.go Outdated Show resolved Hide resolved
cmd/swaggergen/swaggergen.go Outdated Show resolved Hide resolved
cmd/swaggergen/swaggergen.go Outdated Show resolved Hide resolved
cmd/swaggergen/swaggergen.go Outdated Show resolved Hide resolved
cmd/swaggergen/swaggergen.go Outdated Show resolved Hide resolved
cmd/swaggergen/swaggergen.go Outdated Show resolved Hide resolved
cmd/swaggergen/swaggergen.go Outdated Show resolved Hide resolved
cmd/swaggergen/swaggergen.go Outdated Show resolved Hide resolved

requestResponseString := string(buf)

// split requestResponseString into request and response parts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this can be done without splitting by first parsing the request with http.ReadRequest and then passing the generated requested as second argument to http.ReadResponse (ref. https://stackoverflow.com/questions/33963467/parse-http-requests-and-responses-from-text-file-in-go)

Copy link
Contributor

@forgedhallpass forgedhallpass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also make sure the generated files are valid, using the Swagger/OpenAPI editors:

https://editor.swagger.io/
https://editor-next.swagger.io/

cmd/swaggergen/spec.go Outdated Show resolved Hide resolved
cmd/swaggergen/swaggergen.go Outdated Show resolved Hide resolved
if err != nil {
return err
}
defer f.Close()

Check warning

Code scanning / SonarCloud

<!--SONAR_ISSUE_KEY:AYMM3F2syReJwSTgeoCe-->Deferring unsafe method "Close" on type "*os.File" <p>See more on <a href="https://sonarcloud.io/project/issues?id=projectdiscovery_proxify&issues=AYMM3F2syReJwSTgeoCe&open=AYMM3F2syReJwSTgeoCe&pullRequest=159">SonarCloud</a></p>

<!--SONAR_ISSUE_KEY:AYMM3F2syReJwSTgeoCe-->Deferring unsafe method "Close" on type "*os.File" <p>See more on <a href="https://sonarcloud.io/project/issues?id=projectdiscovery_proxify&issues=AYMM3F2syReJwSTgeoCe&open=AYMM3F2syReJwSTgeoCe&pullRequest=159">SonarCloud</a></p>
if err != nil {
return err
}
defer f.Close()

Check warning

Code scanning / SonarCloud

<!--SONAR_ISSUE_KEY:AYLPiw-sQ9IVopdsQzhZ-->Deferring unsafe method "Close" on type "*os.File" <p>See more on <a href="https://sonarcloud.io/project/issues?id=projectdiscovery_proxify&issues=AYLPiw-sQ9IVopdsQzhZ&open=AYLPiw-sQ9IVopdsQzhZ&pullRequest=159">SonarCloud</a></p>

<!--SONAR_ISSUE_KEY:AYLPiw-sQ9IVopdsQzhZ-->Deferring unsafe method "Close" on type "*os.File" <p>See more on <a href="https://sonarcloud.io/project/issues?id=projectdiscovery_proxify&issues=AYLPiw-sQ9IVopdsQzhZ&open=AYLPiw-sQ9IVopdsQzhZ&pullRequest=159">SonarCloud</a></p>
// WriteSpec writes the spec to a yaml file
func (g *Generator) WriteSpec() error {
// create/open openapi specification yaml file
f, err := os.OpenFile(g.Options.outputSpec, os.O_CREATE|os.O_WRONLY, 0644)

Check warning

Code scanning / SonarCloud

<!--SONAR_ISSUE_KEY:AYMX7KMqlwq2_6qjpiNj-->Expect file permissions to be 0600 or less <p>See more on <a href="https://sonarcloud.io/project/issues?id=projectdiscovery_proxify&issues=AYMX7KMqlwq2_6qjpiNj&open=AYMX7KMqlwq2_6qjpiNj&pullRequest=159">SonarCloud</a></p>

<!--SONAR_ISSUE_KEY:AYMX7KMqlwq2_6qjpiNj-->Expect file permissions to be 0600 or less <p>See more on <a href="https://sonarcloud.io/project/issues?id=projectdiscovery_proxify&issues=AYMX7KMqlwq2_6qjpiNj&open=AYMX7KMqlwq2_6qjpiNj&pullRequest=159">SonarCloud</a></p>
cmd/swaggergen/swaggergen.go Fixed Show fixed Hide fixed
Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lint errors:

  • flag.Parse => err := flag.Parse
  • ioutil => io
  • Update docs if necessary

Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge conflicts

cmd/swaggergen/swaggergen.go Outdated Show resolved Hide resolved
cmd/swaggergen/swaggergen.go Outdated Show resolved Hide resolved
flagSet.SetDescription(`swaggergen generates a swagger specification from a directory of request/response logs`)

flagSet.StringVar(&options.logDir, "log-dir", "", "proxify output log directory")
flagSet.StringVarP(&options.api, "api-host", "api", "", "api host (example: api.example.com)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have some sort of validation on the api-host input

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any specific validation that you would suggest?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we expect a specific type of input, like a host, then we need to make sure that we only accept those and provide a descriptive error message to the user.

@Mzack9999 Mzack9999 dismissed stale reviews from forgedhallpass and themself October 28, 2022 15:40

new review requested

Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about moving the package into pkg/swaggergen and just instantiate it in cmd/swaggergen

@sonarcloud
Copy link

sonarcloud bot commented Nov 21, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability C 5 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Mzack9999 Mzack9999 removed the request for review from forgedhallpass November 21, 2022 13:05
@ehsandeep ehsandeep merged commit 22676b3 into dev Nov 21, 2022
@ehsandeep ehsandeep deleted the swagger-spec-gen branch November 21, 2022 13:16
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 this pull request may close these issues.

OpenAPI/Swagger descriptor generation
4 participants