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

Tr1d1um Panic: device status code used as the response status code can cause an unrecoverable panic #354

Closed
denopink opened this issue Jan 10, 2023 · 2 comments · Fixed by #367
Assignees
Labels

Comments

@denopink
Copy link
Contributor

denopink commented Jan 10, 2023

Tr1d1um panics when a device status code is either 100 or is an invalid http status code.

Currently, we don't reuse the following device status codes as our response status code: 0, 500.

Related code:

if errUnmarshall := json.Unmarshal(wrpModel.Payload, &deviceResponseModel); errUnmarshall == nil {
if deviceResponseModel.StatusCode != 0 && deviceResponseModel.StatusCode != http.StatusInternalServerError {
w.WriteHeader(deviceResponseModel.StatusCode)
}
}
_, err = w.Write(wrpModel.Payload)

Our current documentation @ https://github.com/xmidt-org/xmidt/wiki/XMiDT-API states the following:

Responses

Status Description
200 Request successful
403 Forbidden
404 Device Not Found
500 The XMiDT cloud had an issue
503 A XMiDT cloud component did not respond in the allowed time
504 The device did not respond in the allowed time
599 If XMiDT was able to transfer the request and response but the device reported a non-2xx status (see WRP.status_code in message or header)

Questions:

  • Which device status codes and ranges do we want tr1d1um to reuse as its own response status code?
@JC000
Copy link

JC000 commented Jan 14, 2023

If we are to change this behavior (return a status code other than what is documented), we need to remember to update that page.

If memory serves, my conversation with Wes had us agreeing that anything between 520 and 599 (inclusive) would be usable by RDK, and that we can use that value directly in the Tridium response. We need to verify that this was communicated to RDK and that their implementations uses this range.

@schmidtw
Copy link
Member

schmidtw commented Jan 17, 2023

Yes, that's the right range (520-599 inclusive). Here is the code that produces the error codes for webpa:
https://github.com/xmidt-org/wdmp-c/blob/317d79cadf2fd61031b7b553d5b5548c25d03f9d/src/wdmp_internal.h#L32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants