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

CP <--- CSMS GetConfig.req message Panic occurs when receiving unknown key #304

Open
libreleee opened this issue Oct 9, 2024 · 6 comments

Comments

@libreleee
Copy link

OCPP version:
[x] 1.6
[ ] 2.0.1

I'm submitting a ...

[X] bug report
[ ] feature request

Current behavior:

CP <--- CSMS[2,"9926ce5a-5090-4ea0-ad7b-0087d6fe4ec9","GetConfiguration",{"key":["AllowOfflineTxForUnknownId"]}]

file : example/1.6/cp/handler.go

func (handler *ChargePointHandler) OnGetConfiguration(request *core.GetConfigurationRequest) (confirmation *core.GetConfigurationConfirmation, err error) {
var resultKeys []core.ConfigurationKey
var unknownKeys []string
for _, key := range request.Key {
configKey, ok := handler.configuration[key]
if !ok {
unknownKeys = append(unknownKeys, *configKey.Value) <<<------------ This Line *configKey.Value = nil Panic
} else {
resultKeys = append(resultKeys, configKey)
}
}

...

It is a key that does not have a value, so of course it is nil.
and A panic occurs and the program terminates.
because *configKey.Value = *string.nil

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x0 pc=0x7ff7cdec87f1]

goroutine 27 [running]:
main.(*ChargePointHandler).OnGetConfiguration(0xc00018a2a0, 0xc000322168)
c:/Users/i/Documents/work/go/ocpp-go-master-vscode/example/1.6/cp/handler.go:492 +0x2f1
github.com/lorenzodonini/ocpp-go/ocpp1%2e6.(*chargePoint).handleIncomingRequest(0xc0002f4000, {0x7ff7ce14f820, 0xc000322168}, {0xc000378720, 0x24}, {0xc000094410, 0x10})
c:/Users/i/Documents/work/go/ocpp-go-master-vscode/ocpp1.6/charge_point.go:432 +0x20ac
github.com/lorenzodonini/ocpp-go/ocppj.(*Client).ocppMessageHandler(0xc00026fae0, {0xc0002f6400, 0x64, 0x200})
c:/Users/i/Documents/work/go/ocpp-go-master-vscode/ocppj/client.go:310 +0xaaf
github.com/lorenzodonini/ocpp-go/ws.(*Client).readPump(0xc0002f2000)
c:/Users/i/Documents/work/go/ocpp-go-master-vscode/ws/websocket.go:999 +0x4fa
created by github.com/lorenzodonini/ocpp-go/ws.(*Client).Start in goroutine 1
c:/Users/i/Documents/work/go/ocpp-go-master-vscode/ws/websocket.go:1153 +0xc19

Expected behavior:

CP <--- CSMS[2,"314dac3e-f639-438c-8e0a-48ae26bf23c4","GetConfiguration",{"key":["AllowOfflineTxForUnknownId"]}]
CP ---> CSMS [3,"314dac3e-f639-438c-8e0a-48ae26bf23c4",{"unknownKey":["AllowOfflineTxForUnknownId"]}]

Steps to reproduce:

Related code:

file : example/1.6/cp/handler.go

func (handler *ChargePointHandler) OnGetConfiguration(request *core.GetConfigurationRequest) (confirmation *core.GetConfigurationConfirmation, err error) {
var resultKeys []core.ConfigurationKey
var unknownKeys []string
for _, key := range request.Key {
configKey, ok := handler.configuration[key]
if !ok {
unknownKeys = append(unknownKeys, *configKey.Value) <<<------------ This Line *configKey.Value = nil Panic
} else {
resultKeys = append(resultKeys, configKey)
}
go-debugging
go-debugging

}

...
insert short code snippets here

Solution 1


I tried changing the code

...
for _, key := range request.Key {
configKey, ok := handler.configuration[key]
if !ok {
//unknownKeys = append(unknownKeys,*configKey.Value)
unknownKeys = append(unknownKeys, key) <<<-------------- *1 Code change
} else {
resultKeys = append(resultKeys, configKey)
}
}

...
unknownKeys = append(unknownKeys, *configKey.Value) -> unknownKeys = append(unknownKeys, key)

After changing, I was able to send a conf message without panic. as below
CSMS also confirmed normal reception.

CP log
Oct 9 17:06:53.338�[36m [INFO] [ocppj] �[0mrecv CP <--- CSMS[2,"643c4d80-e83a-44c2-b23a-5d2e37b08169","GetConfiguration",{"key":["AllowOfflineTxForUnknownId"]}]
Oct 9 17:06:55.794�[36m [INFO] [GetConfiguration] �[0mreturning configuration for requested keys: [AllowOfflineTxForUnknownId]
Oct 9 17:06:55.795�[36m [INFO] [ocppj] �[0msend2 CP ---> CSMS [3,"643c4d80-e83a-44c2-b23a-5d2e37b08169",{"unknownKey":["AllowOfflineTxForUnknownId"]}]

CSMS log
[INFO ] 2024-10-09 17:06:53,336 de.rwth.idsg.steve.ocpp.ws.WebSocketLogger (SteVe-Executor-0) - [chargeBoxId=CNU_TEST_CS1, sessionId=08c43155-6e89-ba36-ac28-c45e5bc58ef3] Sending: [2,"643c4d80-e83a-44c2-b23a-5d2e37b08169","GetConfiguration",{"key":["AllowOfflineTxForUnknownId"]}]
[INFO ] 2024-10-09 17:06:55,797 de.rwth.idsg.steve.ocpp.ws.WebSocketLogger (qtp351877391-31) - [chargeBoxId=CNU_TEST_CS1, sessionId=08c43155-6e89-ba36-ac28-c45e5bc58ef3] Received: [3,"643c4d80-e83a-44c2-b23a-5d2e37b08169",{"unknownKey":["AllowOfflineTxForUnknownId"]}]

CP <--- CSMS[2,"314dac3e-f639-438c-8e0a-48ae26bf23c4","GetConfiguration",{"key":["AllowOfflineTxForUnknownId"]}]
CP ---> CSMS [3,"314dac3e-f639-438c-8e0a-48ae26bf23c4",{"unknownKey":["AllowOfflineTxForUnknownId"]}]

Receipt was also confirmed on the OCTT Server.
In my case, this solved it.

I think the code needs to be modified.

Or is there another good solution?

Other information:

insert the output here
@xBlaz3kx
Copy link
Contributor

xBlaz3kx commented Dec 28, 2024

Could you wrap your code and logs into code blocks using ``` for better readability?

@xBlaz3kx
Copy link
Contributor

As far as I understand, this has nothing to do with the library, but with the example code, which has a bug.

The real problem is that we are using the ConfigurationKey.Value instead of Key itself.

@libreleee
Copy link
Author

I'm so happy to hear back from you.
I have summarized it again as requested.

----- code start, panic occurred -----

func (handler *ChargePointHandler) OnGetConfiguration(request *core.GetConfigurationRequest) (confirmation *core.GetConfigurationConfirmation, err error) {
var resultKeys []core.ConfigurationKey
var unknownKeys []string
for _, key := range request.Key {
configKey, ok := handler.configuration[key]
if !ok {
unknownKeys = append(unknownKeys, *configKey.Value) // panic occurred
//unknownKeys = append(unknownKeys, key)
} else {
resultKeys = append(resultKeys, configKey)
}
}
if len(request.Key) == 0 {
// Return config for all keys
for _, v := range handler.configuration {
resultKeys = append(resultKeys, v)
}
}
logDefault(request.GetFeatureName()).Infof("returning configuration for requested keys: %v", request.Key)
conf := core.NewGetConfigurationConfirmation(resultKeys)
conf.UnknownKey = unknownKeys
return conf, nil
}
----- code end -----

----- log start -----
'''
log - cp

[ocppj] recv CP <--- CSMS[2,"315b0eb2-f11e-4287-8fc7-6eeebfa28813","GetConfiguration",{"key":["AllowOfflineTxForUnknownId"]}]

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x0 pc=0x7ff66a4b7711]

goroutine 6 [running]:
main.(*ChargePointHandler).OnGetConfiguration(0xc000094180, 0xc0000083f0)
c:/Users/i/Documents/work/go/ocpp-go-master-vscode/example/1.6/cp/handler.go:550 +0x2f1
github.com/lorenzodonini/ocpp-go/ocpp1%2e6.(*chargePoint).handleIncomingRequest(0xc0000d8210, {0x7ff66a7420e0, 0xc0000083f0}, {0xc000224bd0, 0x24}, {0xc0001faff0, 0x10})
c:/Users/i/Documents/work/go/ocpp-go-master-vscode/ocpp1.6/charge_point.go:441 +0x20ac
github.com/lorenzodonini/ocpp-go/ocppj.(*Client).ocppMessageHandler(0xc0000ae820, {0xc00010ee00, 0x64, 0x200})
c:/Users/i/Documents/work/go/ocpp-go-master-vscode/ocppj/client.go:310 +0xaaf
github.com/lorenzodonini/ocpp-go/ws.(*Client).readPump(0xc000302000)
c:/Users/i/Documents/work/go/ocpp-go-master-vscode/ws/websocket.go:999 +0x4fa
created by github.com/lorenzodonini/ocpp-go/ws.(*Client).Start in goroutine 1
c:/Users/i/Documents/work/go/ocpp-go-master-vscode/ws/websocket.go:1153 +0xc19

'''
----- log end -----

@libreleee
Copy link
Author

----- code start, No panic occurred -----

func (handler *ChargePointHandler) OnGetConfiguration(request *core.GetConfigurationRequest) (confirmation *core.GetConfigurationConfirmation, err error) {
var resultKeys []core.ConfigurationKey
var unknownKeys []string
for _, key := range request.Key {
configKey, ok := handler.configuration[key]
if !ok {
//unknownKeys = append(unknownKeys, *configKey.Value)
unknownKeys = append(unknownKeys, key) //No panic occured
} else {
resultKeys = append(resultKeys, configKey)
}
}
if len(request.Key) == 0 {
// Return config for all keys
for _, v := range handler.configuration {
resultKeys = append(resultKeys, v)
}
}
logDefault(request.GetFeatureName()).Infof("returning configuration for requested keys: %v", request.Key)
conf := core.NewGetConfigurationConfirmation(resultKeys)
conf.UnknownKey = unknownKeys
return conf, nil
}

----- log start -----

'''
log - cp

Dec 28 19:54:30.517 [INFO] [ocppj] recv CP <--- CSMS[2,"64d2495c-88df-40b7-9147-387f2f08a7e6","GetConfiguration",{"key":["AllowOfflineTxForUnknownId"]}]
Dec 28 19:54:33.299 [INFO] [ocppj] send CP ---> CSMS[3,"64d2495c-88df-40b7-9147-387f2f08a7e6",{"unknownKey":["AllowOfflineTxForUnknownId"]}]

log - csms

INFO:ocpp: send CSMS ---> CP [2,"64d2495c-88df-40b7-9147-387f2f08a7e6","GetConfiguration",{"key":["AllowOfflineTxForUnknownId"]}]
INFO:ocpp: recv CSMS <--_ CP [3,"64d2495c-88df-40b7-9147-387f2f08a7e6",{"unknownKey":["AllowOfflineTxForUnknownId"]}]
'''
----- log end -----

@xBlaz3kx
Copy link
Contributor

I see, since you found the bug and the solution to fix it, why haven't you opened a PR?

@libreleee
Copy link
Author

libreleee commented Dec 29, 2024

First, I wanted to make sure there were other solutions and that I wasn't wrong

Over 20 years of program development experience but, I'm still not good at github activities because I'm a beginner.
My job is a programmer, but it is the most enjoyable hobby for me, This moment is fun. Thank you

I will prepare and PR.

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

No branches or pull requests

2 participants