resty icon indicating copy to clipboard operation
resty copied to clipboard

Encoded QueryStringParam shows !A(MISSING)

Open RafPe opened this issue 2 years ago • 14 comments

Hi ,

When working with resty & querystring params I noticed that when showing debug output and having special characters in queryParamString it shows them as !A(MISSING) altough query is properly formated and sent

2021/08/20 11:53:53.437251 DEBUG RESTY 
==============================================================================
GET  /api?paramSpecial=a+aa%!A(MISSING)sss&extended=false  HTTP/1.1

Its not a show stopper - but confuses end users when debugging.

RafPe avatar Aug 20 '21 10:08 RafPe

I think this line is the problem. debugLog should be escaped before passing to Debugf.

moorereason avatar Aug 20 '21 13:08 moorereason

@RafPe Do you mind helping with the tiny test cases or test request data? It will be helpful to replicate at my end and addressing an issue.

jeevatkm avatar Sep 12 '21 18:09 jeevatkm

@jeevatkm would this work https://github.com/apiheat/akamai-cli-netlist/issues/30#issuecomment-901991146 ? Its part of the refferenced issue here.

In short it happens when we have special characters in query string.

RafPe avatar Sep 13 '21 09:09 RafPe

@RafPe I have read the issue you have referred. It seems this param accountSwitchKey has a special character. Actually, I was trying to find out those special characters as a sample.

Right here Request URI is grabbed and composed for debug log https://github.com/go-resty/resty/blob/79cc4d44d50815b89104fec0cc8f363e7e8c7586/middleware.go#L277

I think applying Escape for the overall debug log might have side effects. So it's better to address at the URI processing end.

can you help to find out those special characters as best you can? So that it will be helpful.

jeevatkm avatar Sep 13 '21 18:09 jeevatkm

@RafPe Also after reading your referred issue. I think param accountSwitchKey value had a character set something like %A and then package fmt was unable to recognize, so it logged as %!A(MISSING). https://pkg.go.dev/fmt#hdr-Format_errors

fmt.Printf("%s hello %A", "welcome")

// output
// welcome hello %!A(MISSING)

fmt.Printf("accountSwitchKey=F-AC-XXXXXXX%AY-YYYY\n")

// output
// accountSwitchKey=F-AC-XXXXXXX%!A(MISSING)Y-YYYY

So, if apply the url.QueryEscape method, the value would be

F-AC-XXXXXXX%25AY-YYYY

Personally, I feel Resty will get confused more because the query string has an incorrect value that gets logged.

jeevatkm avatar Sep 13 '21 19:09 jeevatkm

@moorereason your thoughts

jeevatkm avatar Sep 13 '21 19:09 jeevatkm

@jeevatkm The param query string accountSwitchKey in deed was the one causing problem. The situation was caused by using the following characters F-AC-XXXXXXX:Y-YYYY

In my opinion the debug URI should reflect what is being actually sent to the end server as request ( imho )

RafPe avatar Sep 13 '21 19:09 RafPe

@RafPe Thanks for the info. Do you mean : caused this? I will look further at my end too.

jeevatkm avatar Sep 13 '21 19:09 jeevatkm

@jeevatkm I believe it did. Based on my tests there thats what I got

GET /network-list/v2/network-lists?accountSwitchKey=F-AC-XXXXXXX%!A(MISSING)Y-YYYY

RafPe avatar Sep 13 '21 19:09 RafPe

Somewhere along the way, the : is probably being uri-encoded as %3A. This would give the %!A(MISSING) message seen.

However, I'm unable to reproduce this issue with a simple test:

package main

import "github.com/go-resty/resty/v2"

func main() {
        _, err := resty.New().SetDebug(true).R().Get("http://httpbingo.org/get?foo=XX:YY")
        if err != nil {
                panic(err)
        }
}

I don't see anywhere in resty where we're misusing a printf-style function which would cause this problem.

@RafPe, can you make sure you're using the latest resty release? How exactly is your project using resty?

moorereason avatar Sep 13 '21 20:09 moorereason

@moorereason Resty is being used as http client within my apis base client which then in response is used in specific multiple service implementations.

The behaviour happens when I use the functionality of setting query string param - so not directly via URL.

For the current version I will have to check - but I'm sure I was updating it locally while debugging the referenced issue

RafPe avatar Sep 13 '21 20:09 RafPe

It looks to me like you're using v2.0.0. v2.0.0 had a misuse of log.Debugf, which was fixed and shipped with v2.3.0.

moorereason avatar Sep 13 '21 21:09 moorereason

@moorereason I will double check that tomorrow and confirm to you what is the precise version I get this behaviour

RafPe avatar Sep 13 '21 21:09 RafPe

@RafPe Yeah, it seems upstream lib go-edgegrid using Resty v2.0.0 https://github.com/apiheat/go-edgegrid/blob/4fd7bca6dc93d2e9376a44291f6a2a14a06f4b2c/go.mod#L8

Can you try the latest version and share feedback?

jeevatkm avatar Sep 14 '21 00:09 jeevatkm

Upgrading to Resty v2.3.0 or later would be appropriate to overcome this debug log issue.

jeevatkm avatar Oct 01 '23 03:10 jeevatkm