search

Spacebrew / spacebrew

A dynamic re-routable software toolkit for choreographing interactive spaces.
MIT License
222 stars 50 forks source link

added json schema validation to all incoming non-binary messages #71

Closed quinkennedy closed 8 years ago

quinkennedy commented 8 years ago

addresses #70 and probably many other latent errors that may arise from malformed JSON.

The schema definition is somewhat relaxed since it does not specify "additionalProperties":false anywhere, which would further restrict what comes in. I focused on specifying required and common properties.

I don't think I'm missing anything, so I am open to adding "additionalProperties":false to everything except for value (in messages) and possibly options objects (in client config).

quinkennedy commented 8 years ago

also, for documentation purposes:

I decided to use AJV because it was the most performant and complete as documented on this completely un-verified webpage.

robotconscience commented 8 years ago

Overall looks good to me! Just to confirm, we're not forcing 'value' to conform to any type?

My only other comment was to add a 'catch' for JSON.parse so we can log that error.

robotconscience commented 8 years ago

I see it in the fold! Merge it!

quinkennedy commented 8 years ago

for the "value" parameter, I don't provide any specifics, so it should match anything (number, string, boolean, object...)

I have only tested against a couple clients (boolean, string) and hand-crafted bad config messages. I guess until we get a testing harness integrated (WIP on https://github.com/quinkennedy/spacebrew/tree/test-driven) going live is our best test :fearful:

robotconscience commented 8 years ago

PR looks good to me with the new fix. We could: a) merge this into master and test on sandbox b) merge into a branch and test on sandbox

I say we go wild and do a), but up to you

robotconscience commented 8 years ago

Logging into sandbox right now to test.