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

Issue 308 #321

Merged
merged 3 commits into from
Feb 12, 2021
Merged

Issue 308 #321

merged 3 commits into from
Feb 12, 2021

Conversation

iain-anderson
Copy link
Member

@iain-anderson iain-anderson commented Feb 12, 2021

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/device-sdk-c/blob/master/.github/CONTRIBUTING.md

What is the current behavior?

  • default values of CheckInterval and ProfilesDir result in run-time errors
  • data type names are inconsistent with the definitions in go-mod-core-contracts
  • arbitrarily large http requests may be made resulting in potential DoS

Issue Number: #303 #308

What is the new behavior?

Checks added for CheckInterval, ProfilesDir being unset
Changed type names
Added MaxRequestSize configuration option and enforced in http processing

Does this PR introduce a breaking change?

  • Yes
  • No
    Type names in device profiles are now case-sensitive (in practice this is a breaking change in core-metadata where they will be validated should not be an issue as they are normalised in core-metadata)

New Imports

  • Yes
  • No

Specific Instructions

Other information

fix: don't attempt to transform floats that are Inf or NaNs
fix: check type for strings in toml config

Signed-off-by: Iain Anderson <iain@iotechsys.com>
Signed-off-by: Iain Anderson <iain@iotechsys.com>
Signed-off-by: Iain Anderson <iain@iotechsys.com>
Copy link
Member

@SteveOss SteveOss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iain-anderson In edgex_propertytype_tostring have replaced strcasecmp with strcmp, does this mean type names are now case sensitive ?

@iain-anderson
Copy link
Member Author

@iain-anderson In edgex_propertytype_tostring have replaced strcasecmp with strcmp, does this mean type names are now case sensitive ?

Yes but they should be normalized during validation in core-metadata (code here)

@SteveOss
Copy link
Member

@iain-anderson And this is the only path to this function ?

@iain-anderson
Copy link
Member Author

@SteveOss it will only be used when receiving a device profile update notification from metadata, or when requesting a device profile from metadata (when a device is instantiated for which we don't yet have the profile definition)

Copy link
Member

@SteveOss SteveOss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

None yet

2 participants