Incorrect error handling for the JSON response

[arisetty]

When wrong credentials are given during arista node connection, HTTP response code indicates an error ( StatusCode:401, Status: "401 Unauthorized"), but, decodeEapiResponse does not set Error after decoding HTTP response. Instead, the function sets 'JSONRPCResponse.Error' to 'nil' (i.e success to the call)

Callers of 'HTTPSEapiConnection.send', HTTPSEapiConnection.Execute, EapiReqHandle.Call
misinterpret the response.

Have you noticed the issue? if so, can you please provide the fix?

Sample eapi.conf with wrong credentials

[LP-CARISE-OSX:~] $ cat ~/.eapi.conf
[connection:arista-dev]
host=192.168.27.230
username=eapi
password=Dankberg
enablepwd=Dankberg
transport=https
port=443

HTTP response

[LP-CARISE-OSX:~/projects/src/myproject] $ ./debug
======> &http.Response{Status:"401 Unauthorized", StatusCode:401, Proto:"HTTP/1.0", ProtoMajor:1, ProtoMinor:0, Header:http.Header{"Server":[]string{"BaseHTTP/0.3 Python/2.7"}, "Date":[]string{"Mon, 30 Jan 2017 21:54:12 GMT"}, "Cache-Control":[]string{"no-store", "no-cache", "must-revalidate", "max-age=0", "pre-check=0", "post-check=0"}, "Pragma":[]string{"no-cache"}, "Www-Authenticate":[]string{"Basic realm="COMMAND_API_AUTH""}, "Content-Type":[]string{"text/plain"}, "Content-Length":[]string{"62"}, "Connection":[]string{"close"}}, Body:(*http.cancelTimerBody)(0xc42012a100), ContentLength:62, TransferEncoding:[]string(nil), Close:true, Uncompressed:false, Trailer:http.Header(nil), Request:(*http.Request)(0xc4200a63c0), TLS:(*tls.ConnectionState)(0xc4200c04d0)}
2017/01/30 14:09:28 invalid character 'U' looking for beginning of value
chakri --> &goeapi.JSONRPCResponse{Jsonrpc:"", Result:[]map[string]interface {}(nil), ID:0, Error:(*goeapi.RespError)(nil)}
Version:
======> &http.Response{Status:"401 Unauthorized", StatusCode:401, Proto:"HTTP/1.0", ProtoMajor:1, ProtoMinor:0, Header:http.Header{"Cache-Control":[]string{"no-store", "no-cache", "must-revalidate", "max-age=0", "pre-check=0", "post-check=0"}, "Pragma":[]string{"no-cache"}, "Www-Authenticate":[]string{"Basic realm="COMMAND_API_AUTH""}, "Content-Type":[]string{"text/plain"}, "Content-Length":[]string{"62"}, "Connection":[]string{"close"}, "Server":[]string{"BaseHTTP/0.3 Python/2.7"}, "Date":[]string{"Mon, 30 Jan 2017 21:54:13 GMT"}}, Body:(*http.cancelTimerBody)(0xc42011b340), ContentLength:62, TransferEncoding:[]string(nil), Close:true, Uncompressed:false, Trailer:http.Header(nil), Request:(*http.Request)(0xc4200a6a50), TLS:(*tls.ConnectionState)(0xc4200c06e0)}
2017/01/30 14:09:29 invalid character 'U' looking for beginning of value
chakri --> &goeapi.JSONRPCResponse{Jsonrpc:"", Result:[]map[string]interface {}(nil), ID:0, Error:(*goeapi.RespError)(nil)}
Modelname :
Internal Version :
System MAC :
Serial Number :
Mem Total : 0
Bootup Timestamp : 0.00
Mem Free : 0
Version :
Architecture :
Internal Build ID :
Hardware Revision :

On the Same note, for the correct credentials, the below is the HTTP response

[LP-CARISE-OSX:~/projects/src/myproject] $ ./debug
======> &http.Response{Status:"200 OK", StatusCode:200, Proto:"HTTP/1.0", ProtoMajor:1, ProtoMinor:0, Header:http.Header{"Date":[]string{"Mon, 30 Jan 2017 21:53:45 GMT"}, "Cache-Control":[]string{"no-store", "no-cache", "must-revalidate", "max-age=0", "pre-check=0", "post-check=0"}, "Pragma":[]string{"no-cache"}, "Content-Type":[]string{"application/json"}, "Content-Length":[]string{"384"}, "Connection":[]string{"close"}, "Server":[]string{"BaseHTTP/0.3 Python/2.7"}}, Body:(*http.cancelTimerBody)(0xc4201280c0), ContentLength:384, TransferEncoding:[]string(nil), Close:true, Uncompressed:false, Trailer:http.Header(nil), Request:(*http.Request)(0xc42009a3c0), TLS:(*tls.ConnectionState)(0xc42001a630)}
chakri --> &goeapi.JSONRPCResponse{Jsonrpc:"2.0", Result:[]map[string]interface {}{map[string]interface {}{}, map[string]interface {}{"modelName":"vEOS", "internalVersion":"4.13.14M-2777234.41314M.1", "systemMacAddress":"12:20:96:30:92:82", "serialNumber":"", "version":"4.13.14M", "hardwareRevision":"", "memTotal":4.024932e+06, "bootupTimestamp":1.48519117951e+09, "memFree":2.581584e+06, "architecture":"i386", "internalBuildId":"b4c6e98e-0021-4681-abeb-fbfa1f39d02a"}}, ID:14670, Error:(*goeapi.RespError)(nil)}
Version: 4.13.14M
======> &http.Response{Status:"200 OK", StatusCode:200, Proto:"HTTP/1.0", ProtoMajor:1, ProtoMinor:0, Header:http.Header{"Cache-Control":[]string{"no-store", "no-cache", "must-revalidate", "max-age=0", "pre-check=0", "post-check=0"}, "Pragma":[]string{"no-cache"}, "Content-Type":[]string{"application/json"}, "Content-Length":[]string{"384"}, "Connection":[]string{"close"}, "Server":[]string{"BaseHTTP/0.3 Python/2.7"}, "Date":[]string{"Mon, 30 Jan 2017 21:53:46 GMT"}}, Body:(*http.cancelTimerBody)(0xc4201640c0), ContentLength:384, TransferEncoding:[]string(nil), Close:true, Uncompressed:false, Trailer:http.Header(nil), Request:(*http.Request)(0xc42009aa50), TLS:(*tls.ConnectionState)(0xc42001a840)}
chakri --> &goeapi.JSONRPCResponse{Jsonrpc:"2.0", Result:[]map[string]interface {}{map[string]interface {}{}, map[string]interface {}{"systemMacAddress":"12:20:96:30:92:82", "serialNumber":"", "memTotal":4.024932e+06, "memFree":2.581584e+06, "architecture":"i386", "internalBuildId":"b4c6e98e-0021-4681-abeb-fbfa1f39d02a", "hardwareRevision":"", "modelName":"vEOS", "bootupTimestamp":1.48519117951e+09, "version":"4.13.14M", "internalVersion":"4.13.14M-2777234.41314M.1"}}, ID:14670, *Error:(goeapi.RespError)(nil)}
Modelname : vEOS
Internal Version : 4.13.14M-2777234.41314M.1
System MAC : 12:20:96:30:92:82
Serial Number :
Mem Total : 4024932
Bootup Timestamp : 1485191179.51
Mem Free : 2581584
Version : 4.13.14M
Architecture : i386
Internal Build ID : b4c6e98e-0021-4681-abeb-fbfa1f39d02a
Hardware Revision :

=========================================
Call flow for reference

func (handle *EapiReqHandle) Call() error {
....
jsonrsp, err := handle.node.conn.Execute(commands, handle.encoding)
if err != nil {
return err
}

....

}

func (conn *HTTPSEapiConnection) Execute(commands []interface{},
encoding string) (*JSONRPCResponse, error) {
if conn == nil {
return &JSONRPCResponse{}, fmt.Errorf("No connection")
}
conn.ClearError()
data, err := buildJSONRequest(commands, encoding, os.Getpid())
if err != nil {
conn.SetError(err)
return &JSONRPCResponse{}, err
}
return conn.send(data)
}

// decodeEapiResponse [private] Used to decode JSON Response into
// structure format defined by type JSONRPCResponse
func decodeEapiResponse(resp *http.Response) *JSONRPCResponse {
dec := json.NewDecoder(resp.Body)
var v JSONRPCResponse
if err := dec.Decode(&v); err != nil {
log.Println(err)
}
return &v
}

func (conn *HTTPSEapiConnection) send(data []byte) (*JSONRPCResponse, error) {
{
...
...

resp, err := client.Post(url, "application/json", bytes.NewReader(data))
...

**# jsonRsp := decodeEapiResponse(resp)**

// check for errors in the JSON response
if jsonRsp.Error != nil {
	err := fmt.Errorf("JSON Error(%d): %s", jsonRsp.Error.Code,
		jsonRsp.Error.Message)
	conn.SetError(err)
	return jsonRsp, err
}

fmt.Printf("chakri --> %#v", jsonRsp)
return jsonRsp, nil

}

The issue is very easily reproducible with wrong credentials with show-version.go (arista sample code).
Thanks
Chakri

[arisetty]

The issue should be fixed either in decodeEapiResponse or check for HTTP.Response status code in HTTPSEapiConnection.send.

[cheynearista]

@arisetty you are correct. The basic status code is not being checked and should be. I'll be happy to fix this.

[arisetty]

@cheynearista Thank you so much. I appreciate if you could help with it. We would like to use the code from Arista as opposed to fixing ourselves.

If you would like, I've snippet of the code that might fix the issue. But, it does not handle all response codes. it just handles only status code (401 authentication error).

Here is my code in case

carisetty@chakri-vm:~/work/restart/Controller/src/github.com/aristanetworks/goeapi$ git diff eapilib.go
diff --git a/eapilib.go b/eapilib.go
index 6a6e957..3169576 100644
--- a/eapilib.go
+++ b/eapilib.go
@@ -540,7 +540,13 @@ func (conn *HTTPSEapiConnection) send(data []byte) (*JSONRPCResponse, error) {
}
}()

•   jsonRsp := decodeEapiResponse(resp)
  

• var jsonRsp *JSONRPCResponse
• if resp.StatusCode != 401 { /* status code: 401 authentication error */

•    jsonRsp = decodeEapiResponse(resp)
  

• } else {

•    jsonRsp = &JSONRPCResponse{ Error: &RespError{Code: resp.StatusCode,
  

•                                Message: resp.Status}}
  

• }

[cheynearista]

@arisetty I've committed a fix to the develop branch that should address this issue. Would it be possible for you to give it a try? Since the error is http related (and not related to JSON specifically), I've chosen to handle it slightly different from your recommendation above....but more inline with your original thought to possibly handling this in decodeEapiResponse(). As a side, I've opened issue #15 for tests related to this.

[arisetty]

@cheynearista Thank you so much. That is the best approach to fix the issue in decodeEapiResponse() instead of just handling 401. Yes, It works perfectly.
Though it is not important, one small recommendation is that in the example1.go, It might be better to move these lines

conf := node.RunningConfig()
fmt.Println(conf)

var showversion module.ShowVersion
handle, _ := node.GetHandle("json")
if err := handle.Enable(&showversion); err != nil {
panic(err)
}

Rest everything seems to be perfect. It is working as we expected.
I've tested different combinations and is working quite well.

Here is the output

carisetty@chakri-vm:~/work/restart/Controller/src$ ./example1

Version: 4.13.14M

Modelname : vEOS
Internal Version : 4.13.14M-2777234.41314M.1
System MAC : 12:20:96:30:92:82
Serial Number :
Mem Total : 4024932
Bootup Timestamp : 1485191179.50
Mem Free : 2578336
Version : 4.13.14M
Architecture : i386
Internal Build ID : b4c6e98e-0021-4681-abeb-fbfa1f39d02a
Hardware Revision :

Sysinfo: "Ladie"
carisetty@chakri-vm:/work/restart/Controller/src$ vi eapi.conf
carisetty@chakri-vm:/work/restart/Controller/src$ ./example1
panic: Http error: 401 Unauthorized

goroutine 1 [running]:
panic(0x7788c0, 0xc8200ec240)
/home/carisetty/go/src/runtime/panic.go:464 +0x3e6
main.main()
/home/carisetty/work/restart/Controller/src/example1.go:19 +0x17f