feat(curl): generate curl cmd for request && example for curl cmd #794

ahuigo · 2024-05-11T10:07:36Z

I've extracted curl code from my previous PR(#734).
Could you review this code?

New feature: generate curl command

Example: https://github.com/ahuigo/resty/blob/curl/v2/examples/debug_curl_test.go

client := resty.New()
resp, err := client.R().EnableTrace().
    Get("https://httpbin.org/get")

// Explore curl command
curlCmdExecuted := resp.Request.GenerateCurlCommand()
fmt.Println("Curl Command:", curlCmdExecuted)

Output

$ go test -run '^TestDebugCurl$' github.com/go-resty/resty/v2/examples -v
=== RUN   TestDebugCurl
    debug_curl_test.go:29: curlCmdUnexecuted:  
    curl -X GET -H 'Content-Type: application/json' -H 'Cookie: count=1' -H 'User-Agent: go-resty/2.12.0 (https://github.com/go-resty/resty)' -d '{"name":"Alex"}' 'http://unexecuted-url'

    debug_curl_test.go:41: curlCmdExecuted:  
    curl -X POST -H 'Accept: application/json' -H 'Content-Type: application/json' -H 'Cookie: count=1; count=1' -H 'User-Agent: go-resty/2.12.0 (https://github.com/go-resty/resty)' -d '{"name":"Alex"}' http://127.0.0.1:56426/post
--- PASS: TestDebugCurl (0.00s)

luengwaiban · 2024-06-06T07:44:52Z

this PR is very needed！！

jeevatkm

@ahuigo - I'm sorry for the delayed response.

Thanks for extracting and creating a new PR. I have added my comments; could you update the PR? and then we can merge it.

jeevatkm · 2024-06-13T20:34:13Z

util_curl.go

+	"github.com/go-resty/resty/v2/shellescape"
+)
+
+func BuildCurlRequest(req *http.Request, httpCookiejar http.CookieJar) (curl string) {


@ahuigo, please un-export this method to the package.

jeevatkm · 2024-06-13T20:40:45Z

request.go

@@ -333,6 +348,12 @@ func (r *Request) SetResult(res interface{}) *Request {
 	return r
 }

+// This method is to register curl cmd for request executed.
+func (r *Request) SetResultCurlCmd(curlCmd *string) *Request {


@ahuigo - please remove this method and the related implementation code.

Resty could provide a cURL command generation on-demand instead of implicit via -

resp, err := client.R(). EnableTrace(). Get("https://httpbin.org/get") curlCmd := resp.Request.GenerateCurlCommand()

I've done this as you said.

It needs to be explained that without SetResultCurlCmd, GenerateCurlCommand must rely on EnableTrace.

This is because if EnableTrace() is not called, GenerateCurlCommand cannot obtain the body.

jeevatkm · 2024-06-13T20:43:42Z

request.go

@@ -73,6 +74,20 @@ type Request struct {
 	retryConditions     []RetryConditionFunc
 }

+func (r *Request) GetCurlCmd() string {


@ahuigo Please refactor this method with the name GenerateCurlCommand and update this method logic to on-demand cURL command generation.

jeevatkm · 2024-06-13T20:44:34Z

middleware.go

@@ -307,6 +307,13 @@ func addCredentials(c *Client, r *Request) error {
 	return nil
 }

+func createCurlCmd(c *Client, r *Request) (err error) {


@ahuigo Please remove this middleware.

Deleting this middleware will cause GenerateCurlCommand to be unable to get the body (because resty will delete the body after executing the request)

// 3. Generate curl body if req.Body != nil { buf, _ := io.ReadAll(req.Body) req.Body = io.NopCloser(bytes.NewBuffer(buf)) // important!! curl += `-d ` + shellescape.Quote(string(buf)) }

Considering that I need to record body before the request is executed(only when EnableTrace() is called),
I need to read body in middleware.

func createCurlCmd(c *Client, r *Request) (err error) { if r.trace { if r.resultCurlCmd==nil{ r.resultCurlCmd = new(string) } *r.resultCurlCmd = buildCurlRequest(r.RawRequest, c.httpClient.Jar) } return nil }

I think the best time to record the body is when the middleware request occurs.

If it is before the middleware, parameters such as url and authorization will be lost.

If it is after the middleware, the body parameters will be lost.

Do you have any other better suggestions?

@ahuigo I understand. For now, keep this conditional execution for createCurlCmd method with r.trace enabled. I will see what could be improved in v3.

jeevatkm · 2024-06-13T20:44:53Z

client.go

@@ -1396,6 +1403,7 @@ func createClient(hc *http.Client) *Client {
 		parseRequestBody,
 		createHTTPRequest,
 		addCredentials,
+		createCurlCmd,


@ahuigo Please remove this middleware.

jeevatkm

@ahuigo Thanks for incorporating the review comments. I have added one new comment in the shellescape.go file. Please have a look.

jeevatkm · 2024-06-19T21:17:12Z

middleware.go

@@ -307,6 +307,13 @@ func addCredentials(c *Client, r *Request) error {
 	return nil
 }

+func createCurlCmd(c *Client, r *Request) (err error) {


@ahuigo I understand. For now, keep this conditional execution for createCurlCmd method with r.trace enabled. I will see what could be improved in v3.

jeevatkm · 2024-06-19T21:21:47Z

shellescape/shellescape.go

+The original Python package which this work was inspired by can be found
+at https://pypi.python.org/pypi/shellescape.
+*/
+package shellescape // "import gopkg.in/alessio/shellescape.v1"


@ahuigo Can we remove this line?
On the line, godoc has different meanings to it.

// "import gopkg.in/alessio/shellescape.v1"

If we learned something from this gopkg.in/alessio/shellescape.v1 library, please capture that acknowledgment in the above godoc instead of here.
Otherwise remove it.

jeevatkm

@ahuigo Thanks for updating PR and your contribution.

jeevatkm · 2024-06-28T03:36:24Z

@ahuigo It seems the PR check build failed at gofmt - can you have a look?
https://github.com/go-resty/resty/actions/runs/9594966759/job/26791514646?pr=794

1. refactor `GetCurlCommand` with the name `GenerateCurlCommand` 2. un-export this method `BuildCurlRequest` 3. remove SetResultCurlCmd

ahuigo · 2024-06-28T04:03:52Z

@ahuigo It seems the PR check build failed at gofmt - can you have a look? https://github.com/go-resty/resty/actions/runs/9594966759/job/26791514646?pr=794

@jeevatkm Fixed

codecov · 2024-06-28T05:04:36Z

Codecov Report

Attention: Patch coverage is 98.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.67%. Comparing base (baf7c12) to head (eeb7d8e).

Files	Patch %	Lines
client.go	90.90%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##               v2     #794      +/-   ##
==========================================
+ Coverage   96.53%   96.67%   +0.13%     
==========================================
  Files          12       14       +2     
  Lines        1673     1742      +69     
==========================================
+ Hits         1615     1684      +69     
  Misses         37       37              
  Partials       21       21

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jeevatkm · 2024-06-28T05:07:39Z

@ahuigo Please see - https://github.com/go-resty/resty/pull/794/checks?check_run_id=26793372890

ahuigo · 2024-06-28T07:38:16Z

@jeevatkm Fixed.
Since the unit test code of the /examples/* is in a different package from the source code, coverage does not calculate imported packages by default.

examples/debug_curl_test.go // import different package("github.com/go-resty/resty/v2")

In order to allow coverage to calculate imported packages, we need to add this parameter( -coverpkg=./...):

# go test ./... -race -coverprofile=coverage.txt
go test ./... -race -coverprofile=coverage.txt -coverpkg=./...

…kages that are imported in different packages

jeevatkm

Thanks, @ahuigo, for taking care of the test and CI argument.

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [github.com/go-resty/resty/v2](https://togithub.com/go-resty/resty) | `v2.13.1` -> `v2.14.0` | [![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fgo-resty%2fresty%2fv2/v2.14.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fgo-resty%2fresty%2fv2/v2.14.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fgo-resty%2fresty%2fv2/v2.13.1/v2.14.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fgo-resty%2fresty%2fv2/v2.13.1/v2.14.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>go-resty/resty (github.com/go-resty/resty/v2)</summary> ### [`v2.14.0`](https://togithub.com/go-resty/resty/releases/tag/v2.14.0) [Compare Source](https://togithub.com/go-resty/resty/compare/v2.13.1...v2.14.0) ### Release Notes #### New Features - feat(curl): generate curl cmd for request && example for curl cmd by [@ahuigo](https://togithub.com/ahuigo) in [go-resty/resty#794 #### Enhancements - build: update bazel config with new files by [@jeevatkm](https://togithub.com/jeevatkm) in [go-resty/resty#800 - chore: dependency and version update v2.14.0 by [@jeevatkm](https://togithub.com/jeevatkm) in [go-resty/resty#816 #### Upstream Fixes - update net package for vuln CVE-2023-45288 by [@shedyfreak](https://togithub.com/shedyfreak) in [go-resty/resty#804 #### Test Cases - fix(examples): wrongly stderr written as stdout by [@ahuigo](https://togithub.com/ahuigo) in [go-resty/resty#801 #### Documentation - fix: change resty.GET to resty.MethodGet in doc comment by [@autopp](https://togithub.com/autopp) in [go-resty/resty#803 - resty dev version number and year update by [@jeevatkm](https://togithub.com/jeevatkm) in [go-resty/resty#799 #### New Contributors - [@ahuigo](https://togithub.com/ahuigo) made their first contribution in [go-resty/resty#794 - [@autopp](https://togithub.com/autopp) made their first contribution in [go-resty/resty#803 - [@shedyfreak](https://togithub.com/shedyfreak) made their first contribution in [go-resty/resty#804 **Full Changelog**: go-resty/resty@v2.13.1...v2.14.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/Michsior14/transmission-gluetun-port-update).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

ahuigo force-pushed the curl/v2 branch from cbe8748 to 31f6221 Compare May 11, 2024 10:16

jeevatkm requested changes Jun 13, 2024

View reviewed changes

feat(curl): generate curl cmd for request && example for curl cmd

ef8bb46

ahuigo force-pushed the curl/v2 branch 2 times, most recently from 8de5faf to b2969a3 Compare June 19, 2024 07:07

jeevatkm requested changes Jun 19, 2024

View reviewed changes

ahuigo force-pushed the curl/v2 branch from b2969a3 to 01c7a55 Compare June 20, 2024 09:24

jeevatkm previously approved these changes Jun 28, 2024

View reviewed changes

refactor(curl): Simplified code

5090915

1. refactor `GetCurlCommand` with the name `GenerateCurlCommand` 2. un-export this method `BuildCurlRequest` 3. remove SetResultCurlCmd

ahuigo dismissed jeevatkm’s stale review via 5090915 June 28, 2024 04:00

ahuigo force-pushed the curl/v2 branch from 01c7a55 to 5090915 Compare June 28, 2024 04:00

jeevatkm previously approved these changes Jun 28, 2024

View reviewed changes

ahuigo dismissed jeevatkm’s stale review via 439ffa2 June 28, 2024 12:00

cicd(test): add "-coverpkg=./..." to measure the test coverage of pac…

eeb7d8e

…kages that are imported in different packages

ahuigo force-pushed the curl/v2 branch from 439ffa2 to eeb7d8e Compare June 28, 2024 12:37

jeevatkm approved these changes Jun 28, 2024

View reviewed changes

jeevatkm merged commit 855d418 into go-resty:v2 Jun 28, 2024
3 checks passed

nagius mentioned this pull request Aug 27, 2024

New createCurlCmd feature leaks secrets in log when tracing is enabled #828

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(curl): generate curl cmd for request && example for curl cmd #794

feat(curl): generate curl cmd for request && example for curl cmd #794

ahuigo commented May 11, 2024 •

edited

Loading

luengwaiban commented Jun 6, 2024

jeevatkm left a comment

jeevatkm Jun 13, 2024

ahuigo Jun 14, 2024

jeevatkm Jun 13, 2024

ahuigo Jun 14, 2024 •

edited

Loading

jeevatkm Jun 13, 2024

ahuigo Jun 14, 2024

jeevatkm Jun 13, 2024

ahuigo Jun 14, 2024 •

edited

Loading

jeevatkm Jun 19, 2024

jeevatkm Jun 13, 2024

jeevatkm left a comment

jeevatkm Jun 19, 2024

jeevatkm Jun 19, 2024

jeevatkm left a comment

jeevatkm commented Jun 28, 2024

ahuigo commented Jun 28, 2024

codecov bot commented Jun 28, 2024 •

edited

Loading

jeevatkm commented Jun 28, 2024

ahuigo commented Jun 28, 2024 •

edited

Loading

jeevatkm left a comment

feat(curl): generate curl cmd for request && example for curl cmd #794

feat(curl): generate curl cmd for request && example for curl cmd #794

Conversation

ahuigo commented May 11, 2024 • edited Loading

New feature: generate curl command

Output

luengwaiban commented Jun 6, 2024

jeevatkm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahuigo Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahuigo Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeevatkm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeevatkm left a comment

Choose a reason for hiding this comment

jeevatkm commented Jun 28, 2024

ahuigo commented Jun 28, 2024

codecov bot commented Jun 28, 2024 • edited Loading

Codecov Report

jeevatkm commented Jun 28, 2024

ahuigo commented Jun 28, 2024 • edited Loading

jeevatkm left a comment

Choose a reason for hiding this comment

ahuigo commented May 11, 2024 •

edited

Loading

ahuigo Jun 14, 2024 •

edited

Loading

ahuigo Jun 14, 2024 •

edited

Loading

codecov bot commented Jun 28, 2024 •

edited

Loading

ahuigo commented Jun 28, 2024 •

edited

Loading