mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joseph Wu" <jos...@mesosphere.io>
Subject Re: Review Request 39611: Remove binary `data` fields from state endpoints.
Date Mon, 02 Nov 2015 21:03:10 GMT


> On Nov. 2, 2015, 12:52 p.m., Neil Conway wrote:
> > How about backward compatibility? Adding a note to docs/upgrades.md seems a good
idea, at the very least. Are we pretty confident that no one else is looking at this data,
and/or we're happy to break anyone that is?
> 
> Kapil Arya wrote:
>     Good point about backward compatibility. However, if the `data` field contains random
bytes that might result into the entire json becoming "unrenderable", then it's broken anyways.
Also note that we don't expose `TaskInfo::data` and `TaskStatus::data` fields over state.json
either. We can use the dev/user mailing lists to get a feel for it.

Yeah, I emailed the dev list a while ago about this: http://www.mail-archive.com/dev@mesos.apache.org/msg33536.html

Note: The `TaskInfo::data` fields are part of the agent's state endpoint.


- Joseph


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39611/#review104789
-----------------------------------------------------------


On Nov. 2, 2015, 12:21 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39611/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2015, 12:21 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-3771
>     https://issues.apache.org/jira/browse/MESOS-3771
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> We do not encode or otherwise preprocess the raw binary in the ExecutorInfo or TaskInfo
`data` fields before outputting them to JSON via the state endpoints.  This means that the
outputted JSON may be invalid (i.e. non-UTF8) if `data` was filled in with arbitrary bytes.
> 
> The `data` fields are being removed because the state endpoints should not be writing
binary data in the first place.
> 
> 
> Diffs
> -----
> 
>   src/common/http.cpp f56d8a178e1f61d91adfeaad32b47718adbc4379 
>   src/slave/http.cpp d6df97fb64077fb051dd78fe35bf04e8be0331ff 
> 
> Diff: https://reviews.apache.org/r/39611/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> Apparently, none of our tests check these fields.  
> 
> (Discovered via manual tests) Our JSON parser (PicoJson) is not restrictive enough to
catch the invalid-JSON that results.  It will happily parse the binary blobs (roundtripping
successfully).
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message