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

Correctly process APPLICATION/SYSTEM DESCRIBE requests #1226

Merged
merged 1 commit into from
Jan 13, 2017

Conversation

avtolstoy
Copy link
Member

@avtolstoy avtolstoy commented Jan 1, 2017

Fixes #1181

NOTE: Device currently receives incorrect DESCRIBE request with flags=0x09 (see https://github.com/spark/firmware/blob/dfc5e68866cdd794d17ab51640ca565ae3a757ab/communication/src/protocol_defs.h#L91). The length of the request also seems to be wrong (judging by #1181), and instead of 9 bytes, the device receives 16 bytes, all extra bytes are 0x09:

Photon:                        41016b7d02b16409 09 09090909090909
Electron DESCRIBE_SYSTEM:      4101305d01b16441 01
Electron DESCRIBE_APPLICATION: 4101305e02b16441 02

The PR accounts for this by assuming that the server actually requests DESCRIBE_ALL (0x03), if describe flags byte >= DESCRIBE_ALL (0x03)


Doneness:

  • Contributor has signed CLA
  • Problem and Solution clearly stated
  • Code peer reviewed
  • API tests compiled
  • Run unit/integration/application tests on device
  • Add documentation (N/A)
  • Add to CHANGELOG.md after merging (add links to docs and issues)

Bugfix

  • [PR #1226] [Fixes #1181] [Photon/P1/Core] Process TCP DESCRIBE properly and return only one response, SYSTEM, APPLICATION, or COMBINED (ALL) describe message. Was sending separate SYSTEM and APPLICATION previously.

@avtolstoy avtolstoy added this to the 0.7.x milestone Jan 1, 2017
@technobly technobly added the bug label Jan 3, 2017
@brycekahle
Copy link
Contributor

I confirmed the TCP server does not set any flags.

The extra bytes sound like AES padding, which will always be applied to get to multiples of 16 bytes. It appears that whatever handles the AES decryption isn't adjusting the length appropriately.

@technobly technobly modified the milestones: 0.6.1, 0.7.0 Jan 12, 2017
@technobly technobly merged commit a8a17ee into develop Jan 13, 2017
@technobly technobly deleted the feature/describe-response branch January 13, 2017 18:40
@avtolstoy avtolstoy mentioned this pull request May 27, 2017
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants